Add functionality to refresh tag form definitions#2
Conversation
…initions - Updated `NetboxPluginReloaderConfig` in `__init__.py` to refresh tag form definitions alongside custom field definitions. - Simplified `_refresh_form_field` method to handle both features dynamically. - Enhanced docstrings for better clarity and documentation. - Updated `README.md` to reflect changes, including the ability to reload custom field choices and tag choices. - Improved compatibility matrix format in `README.md`. - Updated `setup.py` metadata to ensure compatibility with Python 3.10+ and Django 5.0. This update ensures better integration of plugin models with NetBox's feature system.
|
""" WalkthroughThe updates include a refactor of the plugin's initialization logic to generalize and simplify model registration and form field refreshing, extending support for tag forms alongside custom fields. Documentation was updated to reflect these changes and clarify version compatibility. The plugin version was incremented, and Python 3.9 support was removed from the setup configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant NetBox
participant PluginReloader
participant PluginRegistry
participant FormClass
NetBox->>PluginReloader: Initialize (ready)
PluginReloader->>PluginRegistry: Register missing plugin models
PluginReloader->>FormClass: Refresh custom field form definitions
PluginReloader->>FormClass: Refresh tag form definitions
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (1)
79-79: Format the URL as a proper markdown link.The static analysis tool correctly identified a bare URL that should be formatted as a proper markdown link.
-- Based on https://github.com/netbox-community/netbox/discussions/17836 +- Based on [NetBox Community Discussion](https://github.com/netbox-community/netbox/discussions/17836)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
79-79: Bare URL used
null(MD034, no-bare-urls)
netbox_plugin_reloader/__init__.py (3)
48-50: Consider using keyword arguments for better readability.The method signature has 6 parameters using positional arguments, which reduces readability and maintainability. Consider using keyword arguments for some parameters.
- def _register_missing_plugin_models( - self, plugin_list, app_registry, netbox_registry, feature_mixins_map, model_register_function - ): + def _register_missing_plugin_models( + self, plugin_list, *, app_registry, netbox_registry, feature_mixins_map, model_register_function + ):And update the call site accordingly:
- self._register_missing_plugin_models(settings.PLUGINS, apps, registry, FEATURES_MAP, register_models) + self._register_missing_plugin_models( + settings.PLUGINS, + app_registry=apps, + netbox_registry=registry, + feature_mixins_map=FEATURES_MAP, + model_register_function=register_models + )🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 48-48: Too many arguments (6/5)
(R0913)
[refactor] 48-48: Too many positional arguments (6/5)
(R0917)
73-77: Remove unnecessary.keys()call.The static analysis tool correctly identified that checking
key in dict.keys()is less efficient thankey in dict.- return any( - app_label in registry["model_features"][feature_name] - and model_name in registry["model_features"][feature_name][app_label] - for feature_name in feature_mixins_map.keys() - ) + return any( + app_label in registry["model_features"][feature_name] + and model_name in registry["model_features"][feature_name][app_label] + for feature_name in feature_mixins_map + )🧰 Tools
🪛 Ruff (0.11.9)
76-76: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
79-94: LGTM! Excellent refactoring that eliminates code duplication.The generic
_refresh_form_fieldmethod is well-designed and successfully eliminates code duplication while supporting multiple form types. The implementation correctly handles both custom fields and tags with appropriate labels and help text.However, consider reducing the number of parameters to improve maintainability:
- def _refresh_form_field(self, form_class, feature_name, object_type_class, field_class, translation_function): + def _refresh_form_field(self, form_class, feature_name):And move the dependencies inside the method:
+ from core.models import ObjectType + from django.utils.translation import gettext_lazy as _ + from utilities.forms.fields import ContentTypeMultipleChoiceField + field_labels = { "custom_fields": ("Object types", "The type(s) of object that have this custom field"), "tags": ("Object types", "The type(s) of object that can have this tag"), } label, help_text = field_labels[feature_name] - object_types_field = field_class( - label=translation_function(label), - queryset=object_type_class.objects.with_feature(feature_name), - help_text=translation_function(help_text), + object_types_field = ContentTypeMultipleChoiceField( + label=_(label), + queryset=ObjectType.objects.with_feature(feature_name), + help_text=_(help_text), )🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 79-79: Too many arguments (6/5)
(R0913)
[refactor] 79-79: Too many positional arguments (6/5)
(R0917)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(4 hunks)netbox_plugin_reloader/__init__.py(2 hunks)netbox_plugin_reloader/version.py(1 hunks)setup.py(0 hunks)
💤 Files with no reviewable changes (1)
- setup.py
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
79-79: Bare URL used
null
(MD034, no-bare-urls)
🪛 Ruff (0.11.9)
netbox_plugin_reloader/__init__.py
76-76: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
🪛 Pylint (3.3.7)
netbox_plugin_reloader/__init__.py
[refactor] 48-48: Too many arguments (6/5)
(R0913)
[refactor] 48-48: Too many positional arguments (6/5)
(R0917)
[refactor] 79-79: Too many arguments (6/5)
(R0913)
[refactor] 79-79: Too many positional arguments (6/5)
(R0917)
🔇 Additional comments (6)
netbox_plugin_reloader/version.py (1)
3-3: LGTM! Appropriate version bump for the feature enhancement.The version increment from 4.3.0 to 4.3.1 correctly reflects the addition of tag form definition refresh functionality.
README.md (3)
9-9: LGTM! Documentation accurately reflects the enhanced functionality.The addition of tag form definitions refresh is properly documented and aligns with the PR objectives.
18-19: LGTM! Compatibility matrix correctly updated.The version mapping for NetBox 4.3.x with plugin version 4.3.x is accurate and follows the established pattern.
61-64: LGTM! Clear explanation of the enhanced functionality.The updated description accurately explains that the plugin now refreshes both custom field and tag form definitions.
netbox_plugin_reloader/__init__.py (2)
36-36: LGTM! Correct import for tag form functionality.The TagForm import is necessary for the enhanced functionality to refresh tag form definitions.
42-46: LGTM! Enhanced functionality successfully implements PR objectives.The refactored code now refreshes both custom field and tag form definitions, which aligns perfectly with the PR objectives. The generic approach reduces code duplication.
Docstrings generation was requested by @Kani999. * #2 (comment) The following files were modified: * `netbox_plugin_reloader/__init__.py`
|
Note Generated docstrings for this pull request at #3 |
…Y3nix25i3 📝 Add docstrings to `reloading_tags`
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
netbox_plugin_reloader/__init__.py (3)
51-51: Consider refactoring to reduce parameter count.The signature change to positional arguments is consistent with the call site. However, the method now has 6 parameters, which may impact readability and maintainability.
Consider grouping related parameters into a configuration object or using dependency injection to reduce the parameter count.
84-88: Improve performance by removing unnecessary.keys()call.The refactoring to use
any()is excellent and more pythonic. However, the.keys()call is unnecessary and impacts performance.Apply this diff to remove the unnecessary
.keys()call:return any( app_label in registry["model_features"][feature_name] and model_name in registry["model_features"][feature_name][app_label] - for feature_name in feature_mixins_map.keys() + for feature_name in feature_mixins_map )🧰 Tools
🪛 Ruff (0.11.9)
87-87: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
90-114: Excellent generalization with room for future improvement.The new method effectively generalizes form field refreshing for multiple features. The implementation is clean and the hardcoded labels dictionary is appropriate for the current scope.
Consider reducing the parameter count in future iterations by:
- Creating a configuration dataclass for the method parameters
- Using dependency injection for the classes
- Moving the field labels to a class-level constant
This would improve maintainability and make the method easier to test.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 90-90: Too many arguments (6/5)
(R0913)
[refactor] 90-90: Too many positional arguments (6/5)
(R0917)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netbox_plugin_reloader/__init__.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
netbox_plugin_reloader/__init__.py
87-87: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
🪛 Pylint (3.3.7)
netbox_plugin_reloader/__init__.py
[refactor] 50-50: Too many arguments (6/5)
(R0913)
[refactor] 50-50: Too many positional arguments (6/5)
(R0917)
[refactor] 90-90: Too many arguments (6/5)
(R0913)
[refactor] 90-90: Too many positional arguments (6/5)
(R0917)
🔇 Additional comments (5)
netbox_plugin_reloader/__init__.py (5)
28-30: LGTM! Clear documentation of expanded functionality.The updated docstring accurately reflects the new capability to refresh both custom field and tag form definitions.
38-38: LGTM! Necessary import for tag form functionality.The TagForm import is correctly added to support the new tag form refresh capability.
44-48: LGTM! Clean refactoring to generalized approach.The replacement of the specific method with generalized calls for both CustomFieldForm and TagForm improves code maintainability and follows DRY principles.
54-56: LGTM! Clear and concise documentation.The updated docstring effectively describes the method's functionality while being more concise.
26-114: Well-executed refactoring that achieves the PR objectives.The changes successfully add tag form definition refresh capability while improving code maintainability through generalization. The refactoring maintains existing functionality and follows good coding practices.
🧰 Tools
🪛 Ruff (0.11.9)
87-87: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
🪛 Pylint (3.3.7)
[refactor] 50-50: Too many arguments (6/5)
(R0913)
[refactor] 50-50: Too many positional arguments (6/5)
(R0917)
[refactor] 90-90: Too many arguments (6/5)
(R0913)
[refactor] 90-90: Too many positional arguments (6/5)
(R0917)
1 participant
NetboxPluginReloaderConfigin__init__.pyto refresh tag form definitions alongside custom field definitions.This update ensures better integration of plugin models with NetBox's feature system.
Summary by CodeRabbit
Documentation
Refactor
Chores