Skip to content

Conversation

@asmodehn
Copy link
Contributor

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

Use a class attribute instead of hardcoding config_app_label in every url.
To ease extensions test maintainance.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

This PR introduces a new class-level attribute config_app_label = "config" to the SeleniumTestMixin class in the test file. The attribute is used to dynamically construct admin URLs throughout the test methods. Hardcoded admin URL references (e.g., "admin:config_device_change") are replaced with dynamic equivalents that leverage the new attribute via reverse(f"admin:{self.config_app_label}_<entity>"). This refactoring affects multiple test navigation methods for device, devicegroup, VPN, and other entities.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • nemesifier
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description adequately explains the change's purpose (using class attribute instead of hardcoding to ease extension test maintenance) but is missing reference to existing issue and does not check all required checklist items.
Title check ✅ Passed The title accurately describes the main change: introducing config_app_label as a class field in SeleniumTestMixin to replace hardcoded app label references in config selenium tests.

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


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coveralls
Copy link

Coverage Status

coverage: 98.641%. remained the same
when pulling c763979 on asmodehn:config_app_label_in_selenium_tests
into 6697a3a on openwisp:master.

@nemesifier nemesifier changed the title [change] Use config_app_label as class field in config selenium tests [tests:fix] Use config_app_label as class field in config selenium tests Jan 23, 2026
@nemesifier nemesifier changed the title [tests:fix] Use config_app_label as class field in config selenium tests [fix:tests] Use config_app_label as class field in config selenium tests Jan 23, 2026
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thanks @asmodehn

@nemesifier nemesifier merged commit cbdb3c6 into openwisp:master Jan 23, 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