Skip to content

[change:tests] Added config_app_label in BaseTestNotification#1192

Merged
nemesifier merged 1 commit intoopenwisp:masterfrom
asmodehn:patch-1
Jan 8, 2026
Merged

[change:tests] Added config_app_label in BaseTestNotification#1192
nemesifier merged 1 commit intoopenwisp:masterfrom
asmodehn:patch-1

Conversation

@asmodehn
Copy link
Copy Markdown
Member

@asmodehn asmodehn commented Jan 6, 2026

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Description of Changes

Makes the notifications tests in connection app simpler to extend by exposing the config_app_label at the class level

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 6, 2026

Walkthrough

A class attribute config_app_label was added to BaseTestNotification in openwisp_controller/connection/tests/test_notifications.py. The attribute evaluates to "config" when the SAMPLE_APP environment variable is not set and "sample_config" when it is. The _generic_notification_test method was updated to use config_app_label for constructing the admin device_change URL, and an unused local variable was removed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description is mostly complete but missing critical checklist items and reference to issue number. It explains the purpose of changes but lacks detail on testing and documentation.
Title check ✅ Passed The title clearly and specifically describes the main change: adding a config_app_label attribute to BaseTestNotification class to improve test code organization.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @openwisp_controller/connection/tests/test_notifications.py:
- Line 33: In _generic_notification_test, change the reference to the class
attribute config_app_label to use self.config_app_label so device_url_path =
reverse(f"admin:{self.config_app_label}_device_change", args=[self.d.id])
(ensure any other usages in that method also prefix config_app_label with self)
to avoid the NameError when the test runs.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6323bb and 2597187.

📒 Files selected for processing (1)
  • openwisp_controller/connection/tests/test_notifications.py
🧰 Additional context used
🪛 Ruff (0.14.10)
openwisp_controller/connection/tests/test_notifications.py

33-33: Undefined name config_app_label

(F821)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Python==3.10 | django~=5.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.10 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (1)
openwisp_controller/connection/tests/test_notifications.py (1)

20-20: LGTM! Good refactoring to centralize the app label logic.

This class attribute provides a single source of truth for the config app label, making the test code more maintainable and easier to extend.

Comment thread openwisp_controller/connection/tests/test_notifications.py Outdated
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 98.642% (+0.03%) from 98.612%
when pulling 3b6be7d on asmodehn:patch-1
into d6323bb on openwisp:master.

@nemesifier nemesifier changed the title [change] Add config_app_label in BaseTestNotification [change:tests] Added config_app_label in BaseTestNotification Jan 8, 2026
@nemesifier nemesifier merged commit d1569ab into openwisp:master Jan 8, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants