Feat/UI improvements - #83
Conversation
|
Warning Review limit reached
More reviews will be available in 6 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds comprehensive documentation for LibreNMS→NetBox mapping rules and module sync workflows, introduces reusable HTMX form components for inline platform and device-type mapping creation, refactors the device validation modal to use these shared components, updates import UI toggle handling and HTMX modal dismiss scoping, and implements a backend AddPlatformMappingView endpoint with transactional writes and HTMX OOB updates. ChangesPlatform Mapping and Module Sync Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/usage_tips/README.md`:
- Line 18: Update the step title text "Multi Server Configuration" to use a
hyphenated compound modifier: change the string "Multi Server Configuration" to
"Multi-Server Configuration" (i.e., replace the title text in README where the
list item reads [Multi Server Configuration](multi_server_configuration.md)).
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html`:
- Around line 366-368: The platform mapping include is never rendered in the
branch where validation.existing_device exists because the template checks
sync_info (which is not set there); update the conditional around the include in
device_validation_details.html to either check for sync_info's existence (e.g.,
"and sync_info") only when needed or remove the sync_info requirement so the
include runs when validation.existing_device is present and libre_device.os is
valid; specifically adjust the {% if ... %} that wraps the include of
"netbox_librenms_plugin/htmx/_platform_mapping_form.html" to reference
validation.existing_device (and its platform) or guard on sync_info being
defined (e.g., "if sync_info and sync_info.platform_info.platform_exists" vs
using "if validation.existing_device") so the
preselect_platform=validation.existing_device.platform path renders correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4fc949da-9dc3-432a-9295-d20fe892bb15
⛔ Files ignored due to path filters (13)
docs/img/Netbox-librenms-plugin-device-sync-fields.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-import-page.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-module-sync-tab.pngis excluded by!**/*.pngdocs/img/carrier_auto_install_rules/list.pngis excluded by!**/*.pngdocs/img/device_type_mappings/list.pngis excluded by!**/*.pngdocs/img/inventory_ignore_rules/list.pngis excluded by!**/*.pngdocs/img/module_bay_mappings/list.pngis excluded by!**/*.pngdocs/img/module_type_mappings/add.pngis excluded by!**/*.pngdocs/img/module_type_mappings/list.pngis excluded by!**/*.pngdocs/img/normalization_rules/add.pngis excluded by!**/*.pngdocs/img/normalization_rules/list.pngis excluded by!**/*.pngdocs/img/platform_mappings/add.pngis excluded by!**/*.pngdocs/img/platform_mappings/list.pngis excluded by!**/*.png
📒 Files selected for processing (15)
docs/README.mddocs/feature_list.mddocs/librenms_import/validation.mddocs/usage_tips/README.mddocs/usage_tips/mapping_rules.mddocs/usage_tips/module_sync.mdnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📜 Review details
⏰ 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). (3)
- GitHub Check: test-netbox (3.13)
- GitHub Check: test-netbox (3.14)
- GitHub Check: test-netbox (3.12)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building
HttpResponsefrom Django-template-rendered HTML in views, useformat_html()to compose the envelope andmark_safe()on the inner HTML to clear CodeQLpy/reflected-xssfalse positives. Example:format_html('<div id="target" hx-swap-oob="innerHTML">{}</div>', mark_safe(modal_html))
Files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
netbox_librenms_plugin/templates/**/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/*.html: HTMX 2.x is the primary async layer. Table row updates should return<tr hx-swap-oob="true">.
AvoidouterHTMLswaps in HTMX; use OOB or targetedinnerHTMLswaps to keep table layout intact.
Do not reintroducedata-bs-toggleor duplicate modal IDs in modal implementation.
Keep<select class="device-role-select">markup stable to preserve JavaScript hook-up for TomSelect decorators.
Do not re-addtable-responsivewrappers as their removal was deliberate to prevent dropdown clipping.
Templates live intemplates/netbox_librenms_plugin/; reuse and includes go underinc/subdirectory.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,js}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
All HTMX requests and
fetch()calls must include a CSRF token. Prefer extracting from hidden form input viadocument.querySelector('[name=csrfmiddlewaretoken]').valuerather than cookie-based approach.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,css}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Styling assumes Tabler defaults for the netbox_librenms_plugin frontend.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
HTMX fragments live in
templates/netbox_librenms_plugin/htmx/including:device_import_row.html,device_validation_details.html,device_vc_details.html,bulk_import_confirm.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/**/+(*_sync|*_sync_base).html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Sync pages should extend
librenms_sync_base.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html
netbox_librenms_plugin/static/**/*.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/*.js: Modals should try Bootstrap 5 native (bootstrap.Modal) first, falling back to manual DOM manipulation if unavailable. UseshowModal()/hideModal()helper functions.
UseModalManagerclass reference andfilterModalManagerinstance in fetch callbacks; do not use undefinedmodalInstancevariables.
Bind dismiss handlers (backdrop click,data-bs-dismissbuttons) once per element to prevent stacking on repeatedshowModal()calls.
Always checkresponse.okbefore processing fetch responses to catch HTTP errors.
In fetch catch blocks, showerror.messagefor debugging rather than generic messages.
The import filter form uses fetch withAccept: application/json, text/html—JSON for background jobs, HTML for synchronous mode.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
netbox_librenms_plugin/static/**/librenms_import.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/librenms_import.js:librenms_import.jsshould be wrapped in an IIFE withwindow.LibreNMSImportInitializedguard to prevent re-initialization during HTMX swaps.
ImplementModalManagerclass wrapping Bootstrap 5 modal show/hide with fallback in import page JavaScript.
ImplementpollJobStatus()function that polls/api/core/background-tasks/{jobId}/every 2s, updates progress messages, handles cancel button, and redirects on completion.
ImplementcaptureSelectionState()andrestoreSelectionState()functions to preserve checkbox state across HTMX content swaps.
ImplementcreateCacheCountdown()as a generic countdown timer for cache expiration display.
ImplementinitializeFilterForm()to intercept form submit, detect JSON response (background job), and start polling.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
**/urls.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use
<int:pk>(not<str:pk>) for numeric IDs in URL patterns to auto-validate and return 404 for non-integer values, eliminating URL-parameter taint that CodeQL flags
Files:
netbox_librenms_plugin/urls.py
**/views/imports/**
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/**: NetBox's/api/core/background-tasks/endpoint requires superuser (IsSuperuserinBaseRQViewSet). Non-superuser users cannot poll job status (403 Forbidden). Plugin automatically falls back to synchronous mode for non-superusers viashould_use_background_job()inlist.pyandactions.py
Import page filter fields:librenms_location,librenms_type,librenms_os,librenms_hostname,librenms_sysname,librenms_hardware,enable_vc_detection,show_disabled,exclude_existing
Files:
netbox_librenms_plugin/views/imports/actions.py
**/views/imports/actions.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/actions.py:DeviceImportHelperMixinprovidesget_validated_device_with_selections()andrender_device_row()for HTMX row rendering, shared by update views
BulkImportConfirmView(POST) — renders confirmation modal with selected device list viahtmx/bulk_import_confirm.html
BulkImportDevicesView(POST) — executes import. Background mode enqueuesImportDevicesJob; sync mode callsbulk_import_devices()+bulk_import_vms()and returns OOB row swaps withHX-Trigger: closeModal
DeviceValidationDetailsView(GET) — renders expandable validation details viahtmx/device_validation_details.html
DeviceVCDetailsView(GET) — renders VC member details viahtmx/device_vc_details.html
DeviceRoleUpdateView,DeviceClusterUpdateView,DeviceRackUpdateView(POST) — per-device dropdown updates. Apply selection to validation state and return re-rendered row viarender_device_row()
Files:
netbox_librenms_plugin/views/imports/actions.py
🧠 Learnings (9)
📚 Learning: 2026-03-07T22:46:57.537Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/tests/test_librenms_id.py:184-184
Timestamp: 2026-03-07T22:46:57.537Z
Learning: Do not flag or request style-only changes (such as removing redundant imports or cosmetic cleanup) in Python code reviews. Focus on issues that affect correctness, functionality, or security. This guideline applies to Python files across the repository, including tests (e.g., netbox_librenms_plugin/tests/test_librenms_id.py).
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T13:09:49.031Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/forms.py:75-79
Timestamp: 2026-03-08T13:09:49.031Z
Learning: In multi-server caching within the codebase, ensure poller group choices and similar cache discriminators use api.server_key as the cache key component (e.g., cache_key = f"librenms_poller_group_choices_{api.server_key}") rather than api.librenms_url. This aligns with the fixed approach seen in commit bf37f07 and with other modules. Apply this pattern consistently to Python files under netbox_librenms_plugin (and similar multi-server cache keys) to maintain correct cross-server caching behavior.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-05T09:51:15.707Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/base/modules_view.py:311-313
Timestamp: 2026-05-05T09:51:15.707Z
Learning: In this repository, if you see `timezone.timedelta` used from `django.utils.timezone`, do not request a diff to replace it with `datetime.timedelta` solely on style grounds. This usage is functionally correct because Django exposes `timedelta` via `django.utils.timezone`; treat this as intentional and avoid churn unless there is evidence of changed/incorrect behavior (e.g., `timezone` is not Django’s module or `timedelta` is unavailable).
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-04-15T12:38:49.280Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/views/base/modules_view.py:133-141
Timestamp: 2026-04-15T12:38:49.280Z
Learning: Do not require or flag an explicit `permission_required = PERM_VIEW_PLUGIN` on views that inherit from `LibreNMSPermissionMixin` (e.g., those ultimately including `BaseModuleTableView` in `views/base/modules_view.py`). `LibreNMSPermissionMixin` is the authoritative place where `permission_required` is set to `PERM_VIEW_PLUGIN`; inherited values are already enforced via the mixin, making a redundant override unnecessary.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-17T11:32:40.631Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 76
File: netbox_librenms_plugin/views/object_sync/devices.py:178-180
Timestamp: 2026-05-17T11:32:40.631Z
Learning: In this plugin’s JSON endpoint views (e.g., NetBox view classes/functions under netbox_librenms_plugin/views/**), do not require a try/except JSONDecodeError around `json.loads(request.body)` by default if the endpoint is already protected by CSRF + session authentication and has object-level permission gates. Treat missing JSONDecodeError handling as acceptable when the only expected downside is a noisy log line. If you identify any additional security or availability impact from invalid JSON (e.g., unhandled exceptions leading to user-visible 500s, potential DoS amplification, or lack of appropriate throttling/validation), then flag it and recommend adding guarded parsing/validation.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-13T11:16:36.294Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:136-137
Timestamp: 2026-03-13T11:16:36.294Z
Learning: In Django templates under netbox_librenms_plugin/templates/**/*.html, do not suggest adding explicit parentheses to {% if %} expressions for readability. The project favors compact expressions using implicit operator precedence (and binds tighter than or). Treat parentheses as cosmetic and avoid guidance to insert them for style reasons.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-01T08:25:06.260Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:245-246
Timestamp: 2026-05-01T08:25:06.260Z
Learning: In netbox_librenms_plugin template HTML/HTMX code, only require an X-CSRFToken header for state-changing requests made via fetch() or HTMX (POST, PUT, PATCH, DELETE). Do not require X-CSRFToken on read-only fetch() GET calls (e.g., autocomplete/lookup endpoints like dcim-api:devicetype-list); Django/DRF exempt GET requests from CSRF validation. Therefore, code reviews should not flag missing CSRF headers on GET fetch() calls used for lookups/autocomplete.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-13T20:03:16.435Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tests/test_coverage_api2.py:319-355
Timestamp: 2026-03-13T20:03:16.435Z
Learning: Do not propose replacing server_info with module_sync.server_key in the templates located under netbox_librenms_plugin/templates/netbox_librenms_plugin (specifically _module_sync.html and inc/_module_sync.html). These templates rely on server_info being present in the parent template context (librenms_sync_base.html) and server_key may be absent on initial load. Treat the correct usage of server_info for populating the value as the intended pattern; only flag issues if server_key is incorrectly used in these templates. This guideline applies to all files under the templates path for this plugin.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-08T09:30:45.499Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/views/imports/actions.py:1031-1035
Timestamp: 2026-03-08T09:30:45.499Z
Learning: In netbox_librenms_plugin/views/imports/actions.py, ensure that DeviceConflictActionView.post() explicitly rejects boolean values for librenms_id via isinstance(librenms_id, bool) before coercing to int, returning HTTP 400. Do not remove or consolidate this pre-coercion boolean check. This guard is intentional and consistent with the similar bool-guard pattern used in set_librenms_device_id, get_librenms_device_id, and find_by_librenms_id; preserve this behavior to avoid ambiguity and misinterpretation of truthy/falsey booleans.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
🧬 Code graph analysis (3)
netbox_librenms_plugin/views/__init__.py (1)
netbox_librenms_plugin/views/imports/actions.py (1)
AddPlatformMappingView(1794-1895)
netbox_librenms_plugin/urls.py (1)
netbox_librenms_plugin/views/imports/actions.py (1)
AddPlatformMappingView(1794-1895)
netbox_librenms_plugin/views/imports/actions.py (4)
netbox_librenms_plugin/import_utils/device_operations.py (1)
fetch_device_with_cache(963-1014)netbox_librenms_plugin/views/mixins.py (3)
LibreNMSPermissionMixin(42-97)NetBoxObjectPermissionMixin(112-221)LibreNMSAPIMixin(224-310)netbox_librenms_plugin/models.py (1)
PlatformMapping(708-752)netbox_librenms_plugin/import_utils/cache.py (1)
get_import_device_cache_key(193-212)
🪛 HTMLHint (1.9.2)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 22-22: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 27-27: Special characters must be escaped : [ > ].
(spec-char-escape)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 20-20: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 25-25: Special characters must be escaped : [ > ].
(spec-char-escape)
🪛 LanguageTool
docs/usage_tips/README.md
[grammar] ~18-~18: Use a hyphen to join words.
Context: ...during device import and sync 4. [Multi Server Configuration](multi_server_confi...
(QB_NEW_EN_HYPHEN)
🪛 OpenGrep (1.22.0)
netbox_librenms_plugin/views/imports/actions.py
[WARNING] 1884-1884: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
[WARNING] 1891-1891: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
🔇 Additional comments (14)
docs/README.md (1)
25-35: LGTM!Also applies to: 43-43
docs/feature_list.md (1)
6-7: LGTM!Also applies to: 13-32
docs/librenms_import/validation.md (1)
25-27: LGTM!Also applies to: 33-33
docs/usage_tips/mapping_rules.md (1)
1-229: LGTM!docs/usage_tips/module_sync.md (1)
1-57: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
10-109: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html (1)
12-112: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html (1)
178-180: LGTM!Also applies to: 195-200, 213-213
netbox_librenms_plugin/views/imports/actions.py (1)
1794-1895: LGTM!netbox_librenms_plugin/urls.py (1)
16-16: LGTM!Also applies to: 427-431
netbox_librenms_plugin/views/__init__.py (1)
112-112: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html (1)
351-368: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html (1)
1006-1012: LGTM!netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (1)
352-353: LGTM!Also applies to: 1202-1209
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@netbox_librenms_plugin/views/imports/actions.py`:
- Around line 1856-1868: The current get_or_create on PlatformMapping can race
with a concurrent delete and allow creation without proper "add" permission;
change the logic to follow the AddDeviceTypeMappingView pattern: first query
PlatformMapping.objects.select_for_update().filter(librenms_os=librenms_os.lower()).first()
inside the existing transaction.atomic() to lock the potential row, then if a
mapping is found treat it as existing_mapping, otherwise enforce "POST" add
permissions (set required_object_permissions = {"POST":[("add",
PlatformMapping)]} and call require_object_permissions("POST")) before creating
a new PlatformMapping; also keep the existing update path (check
mapping.netbox_platform_id vs platform_id and call
require_object_permissions("POST") for change when appropriate) and ensure
mapping.save() only after permissions checks so the TOCTOU concurrent-delete
case is handled the same way as AddDeviceTypeMappingView.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 710cc2d3-65bb-4b4b-9530-a45ef4caf407
⛔ Files ignored due to path filters (13)
docs/img/Netbox-librenms-plugin-device-sync-fields.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-import-page.pngis excluded by!**/*.pngdocs/img/Netbox-librenms-plugin-module-sync-tab.pngis excluded by!**/*.pngdocs/img/carrier_auto_install_rules/list.pngis excluded by!**/*.pngdocs/img/device_type_mappings/list.pngis excluded by!**/*.pngdocs/img/inventory_ignore_rules/list.pngis excluded by!**/*.pngdocs/img/module_bay_mappings/list.pngis excluded by!**/*.pngdocs/img/module_type_mappings/add.pngis excluded by!**/*.pngdocs/img/module_type_mappings/list.pngis excluded by!**/*.pngdocs/img/normalization_rules/add.pngis excluded by!**/*.pngdocs/img/normalization_rules/list.pngis excluded by!**/*.pngdocs/img/platform_mappings/add.pngis excluded by!**/*.pngdocs/img/platform_mappings/list.pngis excluded by!**/*.png
📒 Files selected for processing (15)
docs/README.mddocs/feature_list.mddocs/librenms_import/validation.mddocs/usage_tips/README.mddocs/usage_tips/mapping_rules.mddocs/usage_tips/module_sync.mdnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building
HttpResponsefrom Django-template-rendered HTML in views, useformat_html()to compose the envelope andmark_safe()on the inner HTML to clear CodeQLpy/reflected-xssfalse positives. Example:format_html('<div id="target" hx-swap-oob="innerHTML">{}</div>', mark_safe(modal_html))
Files:
netbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
**/urls.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use
<int:pk>(not<str:pk>) for numeric IDs in URL patterns to auto-validate and return 404 for non-integer values, eliminating URL-parameter taint that CodeQL flags
Files:
netbox_librenms_plugin/urls.py
netbox_librenms_plugin/templates/**/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/*.html: HTMX 2.x is the primary async layer. Table row updates should return<tr hx-swap-oob="true">.
AvoidouterHTMLswaps in HTMX; use OOB or targetedinnerHTMLswaps to keep table layout intact.
Do not reintroducedata-bs-toggleor duplicate modal IDs in modal implementation.
Keep<select class="device-role-select">markup stable to preserve JavaScript hook-up for TomSelect decorators.
Do not re-addtable-responsivewrappers as their removal was deliberate to prevent dropdown clipping.
Templates live intemplates/netbox_librenms_plugin/; reuse and includes go underinc/subdirectory.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,js}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
All HTMX requests and
fetch()calls must include a CSRF token. Prefer extracting from hidden form input viadocument.querySelector('[name=csrfmiddlewaretoken]').valuerather than cookie-based approach.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,css}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Styling assumes Tabler defaults for the netbox_librenms_plugin frontend.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
HTMX fragments live in
templates/netbox_librenms_plugin/htmx/including:device_import_row.html,device_validation_details.html,device_vc_details.html,bulk_import_confirm.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/**/+(*_sync|*_sync_base).html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Sync pages should extend
librenms_sync_base.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html
netbox_librenms_plugin/static/**/*.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/*.js: Modals should try Bootstrap 5 native (bootstrap.Modal) first, falling back to manual DOM manipulation if unavailable. UseshowModal()/hideModal()helper functions.
UseModalManagerclass reference andfilterModalManagerinstance in fetch callbacks; do not use undefinedmodalInstancevariables.
Bind dismiss handlers (backdrop click,data-bs-dismissbuttons) once per element to prevent stacking on repeatedshowModal()calls.
Always checkresponse.okbefore processing fetch responses to catch HTTP errors.
In fetch catch blocks, showerror.messagefor debugging rather than generic messages.
The import filter form uses fetch withAccept: application/json, text/html—JSON for background jobs, HTML for synchronous mode.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
netbox_librenms_plugin/static/**/librenms_import.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/librenms_import.js:librenms_import.jsshould be wrapped in an IIFE withwindow.LibreNMSImportInitializedguard to prevent re-initialization during HTMX swaps.
ImplementModalManagerclass wrapping Bootstrap 5 modal show/hide with fallback in import page JavaScript.
ImplementpollJobStatus()function that polls/api/core/background-tasks/{jobId}/every 2s, updates progress messages, handles cancel button, and redirects on completion.
ImplementcaptureSelectionState()andrestoreSelectionState()functions to preserve checkbox state across HTMX content swaps.
ImplementcreateCacheCountdown()as a generic countdown timer for cache expiration display.
ImplementinitializeFilterForm()to intercept form submit, detect JSON response (background job), and start polling.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
**/views/imports/**
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/**: NetBox's/api/core/background-tasks/endpoint requires superuser (IsSuperuserinBaseRQViewSet). Non-superuser users cannot poll job status (403 Forbidden). Plugin automatically falls back to synchronous mode for non-superusers viashould_use_background_job()inlist.pyandactions.py
Import page filter fields:librenms_location,librenms_type,librenms_os,librenms_hostname,librenms_sysname,librenms_hardware,enable_vc_detection,show_disabled,exclude_existing
Files:
netbox_librenms_plugin/views/imports/actions.py
**/views/imports/actions.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/actions.py:DeviceImportHelperMixinprovidesget_validated_device_with_selections()andrender_device_row()for HTMX row rendering, shared by update views
BulkImportConfirmView(POST) — renders confirmation modal with selected device list viahtmx/bulk_import_confirm.html
BulkImportDevicesView(POST) — executes import. Background mode enqueuesImportDevicesJob; sync mode callsbulk_import_devices()+bulk_import_vms()and returns OOB row swaps withHX-Trigger: closeModal
DeviceValidationDetailsView(GET) — renders expandable validation details viahtmx/device_validation_details.html
DeviceVCDetailsView(GET) — renders VC member details viahtmx/device_vc_details.html
DeviceRoleUpdateView,DeviceClusterUpdateView,DeviceRackUpdateView(POST) — per-device dropdown updates. Apply selection to validation state and return re-rendered row viarender_device_row()
Files:
netbox_librenms_plugin/views/imports/actions.py
🧠 Learnings (10)
📚 Learning: 2026-03-07T22:46:57.537Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/tests/test_librenms_id.py:184-184
Timestamp: 2026-03-07T22:46:57.537Z
Learning: Do not flag or request style-only changes (such as removing redundant imports or cosmetic cleanup) in Python code reviews. Focus on issues that affect correctness, functionality, or security. This guideline applies to Python files across the repository, including tests (e.g., netbox_librenms_plugin/tests/test_librenms_id.py).
Applied to files:
netbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T13:09:49.031Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/forms.py:75-79
Timestamp: 2026-03-08T13:09:49.031Z
Learning: In multi-server caching within the codebase, ensure poller group choices and similar cache discriminators use api.server_key as the cache key component (e.g., cache_key = f"librenms_poller_group_choices_{api.server_key}") rather than api.librenms_url. This aligns with the fixed approach seen in commit bf37f07 and with other modules. Apply this pattern consistently to Python files under netbox_librenms_plugin (and similar multi-server cache keys) to maintain correct cross-server caching behavior.
Applied to files:
netbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-05T09:51:15.707Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/base/modules_view.py:311-313
Timestamp: 2026-05-05T09:51:15.707Z
Learning: In this repository, if you see `timezone.timedelta` used from `django.utils.timezone`, do not request a diff to replace it with `datetime.timedelta` solely on style grounds. This usage is functionally correct because Django exposes `timedelta` via `django.utils.timezone`; treat this as intentional and avoid churn unless there is evidence of changed/incorrect behavior (e.g., `timezone` is not Django’s module or `timedelta` is unavailable).
Applied to files:
netbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-13T11:16:36.294Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:136-137
Timestamp: 2026-03-13T11:16:36.294Z
Learning: In Django templates under netbox_librenms_plugin/templates/**/*.html, do not suggest adding explicit parentheses to {% if %} expressions for readability. The project favors compact expressions using implicit operator precedence (and binds tighter than or). Treat parentheses as cosmetic and avoid guidance to insert them for style reasons.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-01T08:25:06.260Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:245-246
Timestamp: 2026-05-01T08:25:06.260Z
Learning: In netbox_librenms_plugin template HTML/HTMX code, only require an X-CSRFToken header for state-changing requests made via fetch() or HTMX (POST, PUT, PATCH, DELETE). Do not require X-CSRFToken on read-only fetch() GET calls (e.g., autocomplete/lookup endpoints like dcim-api:devicetype-list); Django/DRF exempt GET requests from CSRF validation. Therefore, code reviews should not flag missing CSRF headers on GET fetch() calls used for lookups/autocomplete.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-13T20:03:16.435Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tests/test_coverage_api2.py:319-355
Timestamp: 2026-03-13T20:03:16.435Z
Learning: Do not propose replacing server_info with module_sync.server_key in the templates located under netbox_librenms_plugin/templates/netbox_librenms_plugin (specifically _module_sync.html and inc/_module_sync.html). These templates rely on server_info being present in the parent template context (librenms_sync_base.html) and server_key may be absent on initial load. Treat the correct usage of server_info for populating the value as the intended pattern; only flag issues if server_key is incorrectly used in these templates. This guideline applies to all files under the templates path for this plugin.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-25T21:48:19.264Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html:12-12
Timestamp: 2026-05-25T21:48:19.264Z
Learning: In this plugin’s HTMX form templates, `hx-include` selectors that target toggle preference wrapper `<span>` element IDs (e.g., `#use-sysname-toggle`, `#strip-domain-toggle`, `#auto-create-ipam-toggle`) are intentional. Those wrapper spans contain both the hidden `off` fallback input and the checkbox; HTMX must include the wrapper so the correct value is serialized, including the unchecked/off state. Do not recommend changing `hx-include` to the checkbox IDs with the `-cb` suffix, since it would omit the hidden fallback and break unchecked/off state submission.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-04-15T12:38:49.280Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/views/base/modules_view.py:133-141
Timestamp: 2026-04-15T12:38:49.280Z
Learning: Do not require or flag an explicit `permission_required = PERM_VIEW_PLUGIN` on views that inherit from `LibreNMSPermissionMixin` (e.g., those ultimately including `BaseModuleTableView` in `views/base/modules_view.py`). `LibreNMSPermissionMixin` is the authoritative place where `permission_required` is set to `PERM_VIEW_PLUGIN`; inherited values are already enforced via the mixin, making a redundant override unnecessary.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-17T11:32:40.631Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 76
File: netbox_librenms_plugin/views/object_sync/devices.py:178-180
Timestamp: 2026-05-17T11:32:40.631Z
Learning: In this plugin’s JSON endpoint views (e.g., NetBox view classes/functions under netbox_librenms_plugin/views/**), do not require a try/except JSONDecodeError around `json.loads(request.body)` by default if the endpoint is already protected by CSRF + session authentication and has object-level permission gates. Treat missing JSONDecodeError handling as acceptable when the only expected downside is a noisy log line. If you identify any additional security or availability impact from invalid JSON (e.g., unhandled exceptions leading to user-visible 500s, potential DoS amplification, or lack of appropriate throttling/validation), then flag it and recommend adding guarded parsing/validation.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T09:30:45.499Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/views/imports/actions.py:1031-1035
Timestamp: 2026-03-08T09:30:45.499Z
Learning: In netbox_librenms_plugin/views/imports/actions.py, ensure that DeviceConflictActionView.post() explicitly rejects boolean values for librenms_id via isinstance(librenms_id, bool) before coercing to int, returning HTTP 400. Do not remove or consolidate this pre-coercion boolean check. This guard is intentional and consistent with the similar bool-guard pattern used in set_librenms_device_id, get_librenms_device_id, and find_by_librenms_id; preserve this behavior to avoid ambiguity and misinterpretation of truthy/falsey booleans.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
🪛 HTMLHint (1.9.2)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 20-20: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 25-25: Special characters must be escaped : [ > ].
(spec-char-escape)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 22-22: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 27-27: Special characters must be escaped : [ > ].
(spec-char-escape)
🪛 LanguageTool
docs/usage_tips/README.md
[grammar] ~18-~18: Use a hyphen to join words.
Context: ...during device import and sync 4. [Multi Server Configuration](multi_server_confi...
(QB_NEW_EN_HYPHEN)
🪛 OpenGrep (1.22.0)
netbox_librenms_plugin/views/imports/actions.py
[WARNING] 1884-1884: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
[WARNING] 1891-1891: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
🔇 Additional comments (16)
docs/README.md (1)
25-44: LGTM!docs/librenms_import/validation.md (1)
25-33: LGTM!docs/usage_tips/README.md (1)
14-27: LGTM!docs/usage_tips/module_sync.md (2)
1-57: Excellent documentation quality.This module sync guide is clear, comprehensive, and well-structured. The table format for match statuses is particularly helpful, and the coverage of Virtual Chassis support and carrier modules provides important context for users.
46-46: Confirm module sync screenshot is present and committed
docs/img/Netbox-librenms-plugin-module-sync-tab.pngexists and is tracked by git (committed), matching the reference indocs/usage_tips/module_sync.md.docs/usage_tips/mapping_rules.md (1)
29-221: Screenshots referenced bydocs/usage_tips/mapping_rules.mdexist — all seven referenceddocs/img/*/list.pngimages are present in the repository.docs/feature_list.md (1)
31-31: Contrib directory exists
The referencedcontrib/directory is present in the repository and containsREADME.mdplus multiple YAML rule files, so the concern aboutcontrib/being missing is already addressed.netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
10-39: LGTM!Also applies to: 41-109
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html (1)
12-41: LGTM!Also applies to: 43-111
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html (1)
178-180: LGTM!Also applies to: 195-200, 213-214, 366-372
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html (1)
351-357: LGTM!Also applies to: 362-369, 377-377
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html (1)
1006-1013: LGTM!netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (1)
352-355: LGTM!Also applies to: 1202-1210
netbox_librenms_plugin/views/imports/actions.py (1)
1876-1895: LGTM!netbox_librenms_plugin/urls.py (1)
16-16: LGTM!Also applies to: 427-431
netbox_librenms_plugin/views/__init__.py (1)
112-112: LGTM!
7ab981b to
0739d97
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@netbox_librenms_plugin/views/imports/actions.py`:
- Around line 1813-1847: Replace the raw HttpResponse(...) error branches in the
AddPlatformMappingView flow (the checks for not libre_device, empty librenms_os,
missing/invalid platform_id, and Platform.DoesNotExist after
Platform.objects.get) with calls to _htmx_error_response(...) using the same
HTML message and HTTP status code so HTMX toasts are shown consistently; locate
the error returns around the libre_device, librenms_os, platform_id
parsing/validation and the Platform.objects.get(...) block and swap each
HttpResponse(...) return for an equivalent _htmx_error_response(message,
status=...) call.
- Around line 1819-1824: The backend currently allows the placeholder OS value
"-" because librenms_os is only checked for emptiness; update the validation in
the same block where librenms_os is computed (the variable librenms_os in the
view handling mapping creation in imports/actions.py) to also reject the
sentinel "-" (after .strip()), returning the same 400 HttpResponse with the
"Device has no OS string -- cannot create mapping." message when librenms_os ==
"-" so the server enforces the same guard as the template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88e414f0-84a1-4ca9-95c6-bb000f09c8df
📒 Files selected for processing (11)
docs/usage_tips/README.mddocs/usage_tips/module_sync.mdnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
netbox_librenms_plugin/**/*.{html,js}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
All HTMX requests and
fetch()calls must include a CSRF token. Prefer extracting from hidden form input viadocument.querySelector('[name=csrfmiddlewaretoken]').valuerather than cookie-based approach.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/static/**/*.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/*.js: Modals should try Bootstrap 5 native (bootstrap.Modal) first, falling back to manual DOM manipulation if unavailable. UseshowModal()/hideModal()helper functions.
UseModalManagerclass reference andfilterModalManagerinstance in fetch callbacks; do not use undefinedmodalInstancevariables.
Bind dismiss handlers (backdrop click,data-bs-dismissbuttons) once per element to prevent stacking on repeatedshowModal()calls.
Always checkresponse.okbefore processing fetch responses to catch HTTP errors.
In fetch catch blocks, showerror.messagefor debugging rather than generic messages.
The import filter form uses fetch withAccept: application/json, text/html—JSON for background jobs, HTML for synchronous mode.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
netbox_librenms_plugin/static/**/librenms_import.js
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/static/**/librenms_import.js:librenms_import.jsshould be wrapped in an IIFE withwindow.LibreNMSImportInitializedguard to prevent re-initialization during HTMX swaps.
ImplementModalManagerclass wrapping Bootstrap 5 modal show/hide with fallback in import page JavaScript.
ImplementpollJobStatus()function that polls/api/core/background-tasks/{jobId}/every 2s, updates progress messages, handles cancel button, and redirects on completion.
ImplementcaptureSelectionState()andrestoreSelectionState()functions to preserve checkbox state across HTMX content swaps.
ImplementcreateCacheCountdown()as a generic countdown timer for cache expiration display.
ImplementinitializeFilterForm()to intercept form submit, detect JSON response (background job), and start polling.
Files:
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js
netbox_librenms_plugin/templates/**/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/*.html: HTMX 2.x is the primary async layer. Table row updates should return<tr hx-swap-oob="true">.
AvoidouterHTMLswaps in HTMX; use OOB or targetedinnerHTMLswaps to keep table layout intact.
Do not reintroducedata-bs-toggleor duplicate modal IDs in modal implementation.
Keep<select class="device-role-select">markup stable to preserve JavaScript hook-up for TomSelect decorators.
Do not re-addtable-responsivewrappers as their removal was deliberate to prevent dropdown clipping.
Templates live intemplates/netbox_librenms_plugin/; reuse and includes go underinc/subdirectory.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,css}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Styling assumes Tabler defaults for the netbox_librenms_plugin frontend.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/**/+(*_sync|*_sync_base).html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Sync pages should extend
librenms_sync_base.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building
HttpResponsefrom Django-template-rendered HTML in views, useformat_html()to compose the envelope andmark_safe()on the inner HTML to clear CodeQLpy/reflected-xssfalse positives. Example:format_html('<div id="target" hx-swap-oob="innerHTML">{}</div>', mark_safe(modal_html))
Files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
HTMX fragments live in
templates/netbox_librenms_plugin/htmx/including:device_import_row.html,device_validation_details.html,device_vc_details.html,bulk_import_confirm.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
**/urls.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use
<int:pk>(not<str:pk>) for numeric IDs in URL patterns to auto-validate and return 404 for non-integer values, eliminating URL-parameter taint that CodeQL flags
Files:
netbox_librenms_plugin/urls.py
**/views/imports/**
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/**: NetBox's/api/core/background-tasks/endpoint requires superuser (IsSuperuserinBaseRQViewSet). Non-superuser users cannot poll job status (403 Forbidden). Plugin automatically falls back to synchronous mode for non-superusers viashould_use_background_job()inlist.pyandactions.py
Import page filter fields:librenms_location,librenms_type,librenms_os,librenms_hostname,librenms_sysname,librenms_hardware,enable_vc_detection,show_disabled,exclude_existing
Files:
netbox_librenms_plugin/views/imports/actions.py
**/views/imports/actions.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/actions.py:DeviceImportHelperMixinprovidesget_validated_device_with_selections()andrender_device_row()for HTMX row rendering, shared by update views
BulkImportConfirmView(POST) — renders confirmation modal with selected device list viahtmx/bulk_import_confirm.html
BulkImportDevicesView(POST) — executes import. Background mode enqueuesImportDevicesJob; sync mode callsbulk_import_devices()+bulk_import_vms()and returns OOB row swaps withHX-Trigger: closeModal
DeviceValidationDetailsView(GET) — renders expandable validation details viahtmx/device_validation_details.html
DeviceVCDetailsView(GET) — renders VC member details viahtmx/device_vc_details.html
DeviceRoleUpdateView,DeviceClusterUpdateView,DeviceRackUpdateView(POST) — per-device dropdown updates. Apply selection to validation state and return re-rendered row viarender_device_row()
Files:
netbox_librenms_plugin/views/imports/actions.py
🧠 Learnings (10)
📚 Learning: 2026-03-13T11:16:36.294Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:136-137
Timestamp: 2026-03-13T11:16:36.294Z
Learning: In Django templates under netbox_librenms_plugin/templates/**/*.html, do not suggest adding explicit parentheses to {% if %} expressions for readability. The project favors compact expressions using implicit operator precedence (and binds tighter than or). Treat parentheses as cosmetic and avoid guidance to insert them for style reasons.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-01T08:25:06.260Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:245-246
Timestamp: 2026-05-01T08:25:06.260Z
Learning: In netbox_librenms_plugin template HTML/HTMX code, only require an X-CSRFToken header for state-changing requests made via fetch() or HTMX (POST, PUT, PATCH, DELETE). Do not require X-CSRFToken on read-only fetch() GET calls (e.g., autocomplete/lookup endpoints like dcim-api:devicetype-list); Django/DRF exempt GET requests from CSRF validation. Therefore, code reviews should not flag missing CSRF headers on GET fetch() calls used for lookups/autocomplete.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-13T20:03:16.435Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tests/test_coverage_api2.py:319-355
Timestamp: 2026-03-13T20:03:16.435Z
Learning: Do not propose replacing server_info with module_sync.server_key in the templates located under netbox_librenms_plugin/templates/netbox_librenms_plugin (specifically _module_sync.html and inc/_module_sync.html). These templates rely on server_info being present in the parent template context (librenms_sync_base.html) and server_key may be absent on initial load. Treat the correct usage of server_info for populating the value as the intended pattern; only flag issues if server_key is incorrectly used in these templates. This guideline applies to all files under the templates path for this plugin.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-07T22:46:57.537Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/tests/test_librenms_id.py:184-184
Timestamp: 2026-03-07T22:46:57.537Z
Learning: Do not flag or request style-only changes (such as removing redundant imports or cosmetic cleanup) in Python code reviews. Focus on issues that affect correctness, functionality, or security. This guideline applies to Python files across the repository, including tests (e.g., netbox_librenms_plugin/tests/test_librenms_id.py).
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T13:09:49.031Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/forms.py:75-79
Timestamp: 2026-03-08T13:09:49.031Z
Learning: In multi-server caching within the codebase, ensure poller group choices and similar cache discriminators use api.server_key as the cache key component (e.g., cache_key = f"librenms_poller_group_choices_{api.server_key}") rather than api.librenms_url. This aligns with the fixed approach seen in commit bf37f07 and with other modules. Apply this pattern consistently to Python files under netbox_librenms_plugin (and similar multi-server cache keys) to maintain correct cross-server caching behavior.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-05T09:51:15.707Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/base/modules_view.py:311-313
Timestamp: 2026-05-05T09:51:15.707Z
Learning: In this repository, if you see `timezone.timedelta` used from `django.utils.timezone`, do not request a diff to replace it with `datetime.timedelta` solely on style grounds. This usage is functionally correct because Django exposes `timedelta` via `django.utils.timezone`; treat this as intentional and avoid churn unless there is evidence of changed/incorrect behavior (e.g., `timezone` is not Django’s module or `timedelta` is unavailable).
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-04-15T12:38:49.280Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/views/base/modules_view.py:133-141
Timestamp: 2026-04-15T12:38:49.280Z
Learning: Do not require or flag an explicit `permission_required = PERM_VIEW_PLUGIN` on views that inherit from `LibreNMSPermissionMixin` (e.g., those ultimately including `BaseModuleTableView` in `views/base/modules_view.py`). `LibreNMSPermissionMixin` is the authoritative place where `permission_required` is set to `PERM_VIEW_PLUGIN`; inherited values are already enforced via the mixin, making a redundant override unnecessary.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-17T11:32:40.631Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 76
File: netbox_librenms_plugin/views/object_sync/devices.py:178-180
Timestamp: 2026-05-17T11:32:40.631Z
Learning: In this plugin’s JSON endpoint views (e.g., NetBox view classes/functions under netbox_librenms_plugin/views/**), do not require a try/except JSONDecodeError around `json.loads(request.body)` by default if the endpoint is already protected by CSRF + session authentication and has object-level permission gates. Treat missing JSONDecodeError handling as acceptable when the only expected downside is a noisy log line. If you identify any additional security or availability impact from invalid JSON (e.g., unhandled exceptions leading to user-visible 500s, potential DoS amplification, or lack of appropriate throttling/validation), then flag it and recommend adding guarded parsing/validation.
Applied to files:
netbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-25T21:48:19.264Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html:12-12
Timestamp: 2026-05-25T21:48:19.264Z
Learning: In this plugin’s HTMX form templates, `hx-include` selectors that target toggle preference wrapper `<span>` element IDs (e.g., `#use-sysname-toggle`, `#strip-domain-toggle`, `#auto-create-ipam-toggle`) are intentional. Those wrapper spans contain both the hidden `off` fallback input and the checkbox; HTMX must include the wrapper so the correct value is serialized, including the unchecked/off state. Do not recommend changing `hx-include` to the checkbox IDs with the `-cb` suffix, since it would omit the hidden fallback and break unchecked/off state submission.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-08T09:30:45.499Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/views/imports/actions.py:1031-1035
Timestamp: 2026-03-08T09:30:45.499Z
Learning: In netbox_librenms_plugin/views/imports/actions.py, ensure that DeviceConflictActionView.post() explicitly rejects boolean values for librenms_id via isinstance(librenms_id, bool) before coercing to int, returning HTTP 400. Do not remove or consolidate this pre-coercion boolean check. This guard is intentional and consistent with the similar bool-guard pattern used in set_librenms_device_id, get_librenms_device_id, and find_by_librenms_id; preserve this behavior to avoid ambiguity and misinterpretation of truthy/falsey booleans.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
🪛 HTMLHint (1.9.2)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 20-20: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 25-25: Special characters must be escaped : [ > ].
(spec-char-escape)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 22-22: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 27-27: Special characters must be escaped : [ > ].
(spec-char-escape)
🪛 OpenGrep (1.22.0)
netbox_librenms_plugin/views/imports/actions.py
[WARNING] 1900-1900: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
[WARNING] 1907-1907: Django mark_safe() with dynamic content can lead to XSS. Only use mark_safe() with trusted, pre-escaped content.
(coderabbit.xss.python-mark-safe)
🔇 Additional comments (4)
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html (1)
1006-1012: LGTM!netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (2)
352-355: LGTM!
1202-1211: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html (1)
351-377: ⚡ Quick winBackend resolves the toggle hidden+checkbox values and naming variants correctly
netbox_librenms_plugin/utils.py::resolve_naming_preferences()accepts both hyphenated and underscored POST keys (use-sysname-toggle/use_sysname-toggle/use_sysname,strip-domain-toggle/strip_domain-toggle/strip_domain), so backend/user-pref naming mismatches won’t break toggle state.- The function uses
request.POST.get(...)(POST overrides GET and user prefs). With duplicate keys from the hidden+checkbox pattern,request.POST.get()takes the last value; the template renders the hiddenoffinput before the checkbox, so a checked toggle should submitonas the effective value.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
netbox_librenms_plugin/views/sync/interfaces.py (1)
77-81:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop
server_keyfrom the post-sync redirect.This view now sends users back to
?tab=interfaces&...&server_key=..., but sync views in this repo are supposed to rely on the globally selected server after the POST completes. Keeping the query param here reintroduces a stale server hint into the URL.Suggested fix
redirect_url = ( reverse(url_name, kwargs={"pk": object_id}) + f"?tab=interfaces&interface_name_field={interface_name_field}" - + (f"&server_key={quote_plus(server_key)}" if server_key else "") )Based on learnings: "Across netbox_librenms_plugin/views/sync/*.py, verify redirects and URL rewrites do not append ?server_key=. The active server context is determined from the global setting (settings.selected_server) via BaseLibreNMSSyncView.get(), so request.GET server_key is not used to preserve context."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@netbox_librenms_plugin/views/sync/interfaces.py` around lines 77 - 81, The redirect_url construction is appending a server_key query param which reintroduces a stale server hint; remove the conditional f"&server_key={quote_plus(server_key)}" so redirect_url is only built from reverse(url_name, kwargs={"pk": object_id}) plus "?tab=interfaces&interface_name_field={interface_name_field}". Update the code that sets redirect_url (the expression using reverse, url_name, object_id, interface_name_field, and server_key) to stop including server_key and rely on the global server context (BaseLibreNMSSyncView.get / settings.selected_server) instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@netbox_librenms_plugin/tables/modules.py`:
- Around line 199-214: The current render_module_bay function turns module_bay
== "-" into a red "No matching bay" even for informational "Integrated" rows
created by BaseModuleTableView._build_row; update render_module_bay to skip the
error styling for those rows by checking record.get("status") (or an equivalent
flag set by _build_row) and, when status == "Integrated" and value == "-",
return the plain value (or the original placeholder) instead of the red span;
keep existing behavior for true missing/empty bays and the module_bay_url
linking logic.
In `@netbox_librenms_plugin/utils.py`:
- Around line 16-17: The regex _VC_MEMBER_INTERFACE_PATTERN currently disallows
hyphens in the interface family prefix so names like "xe-1/0/0" don't match;
update the pattern used by _VC_MEMBER_INTERFACE_PATTERN to allow hyphens in the
prefix (e.g. include '-' in the character class for the prefix group) while
preserving the member (\d+) and suffix ([/:].+) groups so existing member and
suffix capture behavior remains unchanged.
In `@netbox_librenms_plugin/views/base/modules_view.py`:
- Around line 191-203: The helper _count_adoptable_template_interfaces currently
does a DB count each time it is called from _build_row, causing N extra queries;
change it to cache results keyed by (device.pk, tuple(template_names)) on the
view instance (or use functools.lru_cache on a helper that accepts device id and
an immutable template names tuple) so repeated calls return the cached integer
instead of re-running Interface.objects.filter(device=device,
module__isnull=True, name__in=template_names).count(); update _build_row to call
the cached helper and ensure the cache key uses device id and an ordered/hashed
form of get_module_template_interface_names(device, module) to avoid collisions.
- Around line 2255-2259: The code sets row["can_update_interface_binding"] when
matched_type matches and _count_adoptable_template_interfaces(installed) > 0,
but that can be premature if _attach_interface_match() later fails to resolve a
unique interface; instead remove/stop setting can_update_interface_binding here
and either (a) let _attach_interface_match() be the sole setter of
row["can_update_interface_binding"] after it has successfully resolved and
assigned matched_interface_id, or (b) if you prefer to keep this check, require
that a verified matched_interface_id exists before setting the flag (i.e., only
set can_update_interface_binding when matched_interface_id is present); update
render_actions to rely on matched_interface_id rather than this early flag if
needed.
---
Outside diff comments:
In `@netbox_librenms_plugin/views/sync/interfaces.py`:
- Around line 77-81: The redirect_url construction is appending a server_key
query param which reintroduces a stale server hint; remove the conditional
f"&server_key={quote_plus(server_key)}" so redirect_url is only built from
reverse(url_name, kwargs={"pk": object_id}) plus
"?tab=interfaces&interface_name_field={interface_name_field}". Update the code
that sets redirect_url (the expression using reverse, url_name, object_id,
interface_name_field, and server_key) to stop including server_key and rely on
the global server context (BaseLibreNMSSyncView.get / settings.selected_server)
instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5aad836f-eff8-468e-b4cb-c832599c6ea3
📒 Files selected for processing (19)
netbox_librenms_plugin/librenms_api.pynetbox_librenms_plugin/tables/modules.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tests/test_modules_view.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/sync/modules.py
📜 Review details
⏰ 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). (3)
- GitHub Check: test-netbox (3.12)
- GitHub Check: test-netbox (3.14)
- GitHub Check: test-netbox (3.13)
🧰 Additional context used
📓 Path-based instructions (6)
**/views/object_sync/**/*.py
📄 CodeRabbit inference engine (.github/instructions/sync.instructions.md)
Object sync view methods must create instances of concrete table views, copy the
requestobject, and callget_context_data(). VMs must skip cables and VLANs by returningNonefrom thoseget_*_context()methods.
Files:
netbox_librenms_plugin/views/object_sync/devices.py
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building
HttpResponsefrom Django-template-rendered HTML in views, useformat_html()to compose the envelope andmark_safe()on the inner HTML to clear CodeQLpy/reflected-xssfalse positives. Example:format_html('<div id="target" hx-swap-oob="innerHTML">{}</div>', mark_safe(modal_html))
Files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/librenms_api.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tables/modules.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
**/views/sync/**/*.py
📄 CodeRabbit inference engine (.github/instructions/sync.instructions.md)
Sync action views must follow the pattern: check permissions with
LibreNMSPermissionMixinandNetBoxObjectPermissionMixin, read selected items fromrequest.POST.getlist('select'), load cached data usingCacheMixin.get_cache_key(), apply changes insidetransaction.atomic(), and redirect to the sync tab with?tab=<resource>.
Files:
netbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/sync/modules.py
**/urls.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use
<int:pk>(not<str:pk>) for numeric IDs in URL patterns to auto-validate and return 404 for non-integer values, eliminating URL-parameter taint that CodeQL flags
Files:
netbox_librenms_plugin/urls.py
**/views/base/**/*.py
📄 CodeRabbit inference engine (.github/instructions/sync.instructions.md)
**/views/base/**/*.py: Base view classes (BaseLibreNMSSyncView,BaseInterfaceTableView,BaseCableTableView,BaseIPAddressTableView,BaseVLANTableView) must implement the data pipeline pattern: fetch data from LibreNMS API, cache results withCacheMixinkeys likelibrenms_{data_type}_{model_name}_{pk}, compare against NetBox objects, and render a django-tables2 table in a partial template.
Base table view classes must implement resource-specific comparison logic: interface matching by name, IP matching by address/mask, VLAN matching by VID+group, and cables by matching remote devices and checking cable status.
VlanAssignmentMixinmust resolve VLAN group scope in order: Rack → Location → Site → SiteGroup → Region → Global, and must provide auto-selection of the most-specific VLAN group and lookup map building for interface and VLAN sync.
Files:
netbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/views/base/modules_view.py
**/tables/**/*.py
📄 CodeRabbit inference engine (.github/instructions/sync.instructions.md)
Table classes in
tables/must useToggleColumn(attrs={'input': {'name': 'select'}})for selection, accept contextual parameters in constructors (e.g.,device,interface_name_field,vlan_groups), setself.tabandself.prefixfor multi-table pagination, includedata-*attributes in row attrs, and VLAN columns must userender_vlans()with hidden inputs and JSON data.
Files:
netbox_librenms_plugin/tables/modules.py
🧠 Learnings (19)
📚 Learning: 2026-03-07T22:46:57.537Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/tests/test_librenms_id.py:184-184
Timestamp: 2026-03-07T22:46:57.537Z
Learning: Do not flag or request style-only changes (such as removing redundant imports or cosmetic cleanup) in Python code reviews. Focus on issues that affect correctness, functionality, or security. This guideline applies to Python files across the repository, including tests (e.g., netbox_librenms_plugin/tests/test_librenms_id.py).
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/librenms_api.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tables/modules.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-03-08T08:57:43.392Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/views/object_sync/devices.py:83-100
Timestamp: 2026-03-08T08:57:43.392Z
Learning: In views under netbox_librenms_plugin/views/object_sync, server_key values come from admin-controlled PLUGINS_CONFIG dict keys (e.g., "default", "production") and are not user input. Therefore URL-encoding them via urlencode() is unnecessary defensiveness. Do not flag direct string interpolation of server_key into query strings as a URL-injection or encoding issue. This guidance should apply to similar views in the same directory.
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.py
📚 Learning: 2026-03-08T13:09:49.031Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/forms.py:75-79
Timestamp: 2026-03-08T13:09:49.031Z
Learning: In multi-server caching within the codebase, ensure poller group choices and similar cache discriminators use api.server_key as the cache key component (e.g., cache_key = f"librenms_poller_group_choices_{api.server_key}") rather than api.librenms_url. This aligns with the fixed approach seen in commit bf37f07 and with other modules. Apply this pattern consistently to Python files under netbox_librenms_plugin (and similar multi-server cache keys) to maintain correct cross-server caching behavior.
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/librenms_api.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tables/modules.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-05-05T09:51:15.707Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/base/modules_view.py:311-313
Timestamp: 2026-05-05T09:51:15.707Z
Learning: In this repository, if you see `timezone.timedelta` used from `django.utils.timezone`, do not request a diff to replace it with `datetime.timedelta` solely on style grounds. This usage is functionally correct because Django exposes `timedelta` via `django.utils.timezone`; treat this as intentional and avoid churn unless there is evidence of changed/incorrect behavior (e.g., `timezone` is not Django’s module or `timedelta` is unavailable).
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/librenms_api.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tables/modules.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-04-15T12:38:49.280Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/views/base/modules_view.py:133-141
Timestamp: 2026-04-15T12:38:49.280Z
Learning: Do not require or flag an explicit `permission_required = PERM_VIEW_PLUGIN` on views that inherit from `LibreNMSPermissionMixin` (e.g., those ultimately including `BaseModuleTableView` in `views/base/modules_view.py`). `LibreNMSPermissionMixin` is the authoritative place where `permission_required` is set to `PERM_VIEW_PLUGIN`; inherited values are already enforced via the mixin, making a redundant override unnecessary.
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.py
📚 Learning: 2026-05-17T11:32:40.631Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 76
File: netbox_librenms_plugin/views/object_sync/devices.py:178-180
Timestamp: 2026-05-17T11:32:40.631Z
Learning: In this plugin’s JSON endpoint views (e.g., NetBox view classes/functions under netbox_librenms_plugin/views/**), do not require a try/except JSONDecodeError around `json.loads(request.body)` by default if the endpoint is already protected by CSRF + session authentication and has object-level permission gates. Treat missing JSONDecodeError handling as acceptable when the only expected downside is a noisy log line. If you identify any additional security or availability impact from invalid JSON (e.g., unhandled exceptions leading to user-visible 500s, potential DoS amplification, or lack of appropriate throttling/validation), then flag it and recommend adding guarded parsing/validation.
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.pynetbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/base/interfaces_view.pynetbox_librenms_plugin/views/base/modules_view.pynetbox_librenms_plugin/views/sync/modules.py
📚 Learning: 2026-05-05T09:58:50.179Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/object_sync/devices.py:57-75
Timestamp: 2026-05-05T09:58:50.179Z
Learning: In object_sync view classes that pass Django/NetBox `request` into child table context helpers (e.g., for interfaces/cables/IPs/vlans/modules), ensure the child view stores `copy.copy(request)` rather than the original `request` object. Apply this consistently across similar sync views (such as the pattern used in `VMLibreNMSSyncView` in `vms.py`) to prevent cross-view request mutation when the child view modifies the request.
Applied to files:
netbox_librenms_plugin/views/object_sync/devices.py
📚 Learning: 2026-03-12T20:27:53.873Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/librenms_api.py:703-709
Timestamp: 2026-03-12T20:27:53.873Z
Learning: In netbox_librenms_plugin/librenms_api.py, enforce that get_device_inventory() and get_inventory_filtered() always return a list of dicts. Validate as: inventory must be a list and every item must be a dict; if not, log a warning with the raw payload and return (False, error_message). Do not weaken the check to just verify a list type. This should prevent downstream AttributeError/TypeError when callers call .get() on items.
Applied to files:
netbox_librenms_plugin/librenms_api.py
📚 Learning: 2026-03-07T10:32:06.242Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/utils.py:107-117
Timestamp: 2026-03-07T10:32:06.242Z
Learning: In netbox_librenms_plugin/utils.py, keep the Priority 1 loop to guard against None and bool values when accessing raw_cf.get(server_key). Do not replace the Priority 1 condition with a full get_librenms_device_id call. The two-pass design is intentional: Priority 1 performs quick sanity checks, while Priority 2 handles string normalization and full validation by calling get_librenms_device_id(member, server_key, auto_save=False).
Applied to files:
netbox_librenms_plugin/utils.py
📚 Learning: 2026-03-07T22:38:43.110Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/utils.py:552-584
Timestamp: 2026-03-07T22:38:43.110Z
Learning: In netbox_librenms_plugin/utils.py, do not propose replacing 'obj.custom_field_data.get("librenms_id") or {}' with a None check. The code intentionally uses 'or {}' to handle falsey values; downstream type guards treat them equivalently since LibreNMS IDs start at 1, making 0 equivalent to 'not set'. Do not modify this logic; keep the existing behavior for all falsey values.
Applied to files:
netbox_librenms_plugin/utils.py
📚 Learning: 2026-03-08T08:55:46.317Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/utils.py:532-595
Timestamp: 2026-03-08T08:55:46.317Z
Learning: In netbox_librenms_plugin/utils.py, do not modify set_librenms_device_id to call obj.save(). It is mutator-only and should only update in-memory obj.custom_field_data[...] without persisting. Ensure callers perform persistence: after mutation, run full_clean() and then save() (as seen in device_operations.py around lines ~864-866) or explicit obj.save() after set_librenms_device_id (as in librenms_api.py around lines ~261-262). This pattern prevents coupling mutation with persistence and preserves validation in between.
Applied to files:
netbox_librenms_plugin/utils.py
📚 Learning: 2026-03-09T19:15:13.104Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/utils.py:249-267
Timestamp: 2026-03-09T19:15:13.104Z
Learning: In netbox_librenms_plugin/utils.py, ensure match_librenms_hardware_to_device_type returns None when DeviceTypeMapping.MultipleObjectsReturned is raised (fail-closed per inline comment). Callers must guard for result is None separately from the normal result check (e.g., if result is None: handle; elif result.get('matched'): ... ). Note that the success path uses match_type='mapping' (not 'exact'), distinguishing it from standard part_number/model exact lookups. Consider adding a unit test that asserts None is returned on MultipleObjectsReturned and that callers properly handle both None and dict results.
Applied to files:
netbox_librenms_plugin/utils.py
📚 Learning: 2026-03-07T10:40:38.106Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/views/sync/interfaces.py:241-244
Timestamp: 2026-03-07T10:40:38.106Z
Learning: In netbox_librenms_plugin/views/sync/interfaces.py, ensure that set_librenms_device_id does not apply the legacy bare-integer guard to Interface/VMInterface objects. The guard is only relevant for Device/VM objects with pre-existing bare integers from before multi-server support. Interfaces/VMInterfaces have librenms_id starting empty and their port_id is always written from the LibreNMS API JSON response, so there is no migration concern. Do not treat the warning-log path as a silent no-op for interfaces; keep appropriate logging/alerts active. Add or adjust tests to verify that interfaces paths write port_id correctly and do not trigger the legacy-bare-int logic, and document this distinction in code comments.
Applied to files:
netbox_librenms_plugin/views/sync/interfaces.py
📚 Learning: 2026-03-07T13:12:59.182Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/views/sync/device_fields.py:445-448
Timestamp: 2026-03-07T13:12:59.182Z
Learning: Across netbox_librenms_plugin/views/sync/*.py, verify redirects and URL rewrites do not append ?server_key=. The active server context is determined from the global setting (settings.selected_server) via BaseLibreNMSSyncView.get(), so request.GET server_key is not used to preserve context. Ensure RemoveServerMappingView and ConvertLegacyLibreNMSIdView (and similar views) rely on the global setting instead of propagating server_key in redirects.
Applied to files:
netbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/sync/modules.py
📚 Learning: 2026-03-07T17:17:04.217Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 20
File: netbox_librenms_plugin/views/sync/cables.py:160-165
Timestamp: 2026-03-07T17:17:04.217Z
Learning: In Python views under netbox_librenms_plugin/views/sync, when obtaining a server_key for cache namespace scoping, read it from request.POST with a fallback to self.librenms_api.server_key (e.g., server_key = request.POST.get("server_key") or self.librenms_api.server_key) and assign it to an attribute (e.g., self._post_server_key) used by get_cached_links_data to build the cache key. Do not flag or remove this POST-read pattern, as it ensures consistent, future-proof cache namespace scoping for link data lookups. Apply this guidance to similar Sync views in the same module where server_key-based cache scoping is used.
Applied to files:
netbox_librenms_plugin/views/sync/interfaces.pynetbox_librenms_plugin/views/sync/modules.py
📚 Learning: 2026-03-27T02:04:22.276Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tests/test_coverage_api.py:893-939
Timestamp: 2026-03-27T02:04:22.276Z
Learning: For unit tests in this repo (e.g., coverage API tests), when testing a happy-path call like `add_device()`, assert both the success flag and the expected success message (e.g., `assert ok is True` and `assert msg == "Device added successfully."`). This ensures the test fails if `add_device()` returns `(False, ...)`. If a related assertion is explicitly tracked as a known deferred follow-up for a prior PR, do not treat the missing `ok is True` assertion as a new review finding in subsequent reviews.
Applied to files:
netbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-04-01T15:55:42.180Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/tests/test_coverage_actions.py:88-171
Timestamp: 2026-04-01T15:55:42.180Z
Learning: When unit/integration testing actions that indirectly use a function imported at module import time, patch the function where it is *used* (the consumer’s import path), e.g. `netbox_librenms_plugin.views.imports.actions.resolve_naming_preferences`, rather than its original definition. For tests that target the function itself directly, patch the original dependency/definition (e.g. `netbox_librenms_plugin.utils.get_user_pref` or patch `resolve_naming_preferences` at `netbox_librenms_plugin.utils`) so the function under test sees the mocked behavior.
Applied to files:
netbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-05-05T09:46:17.700Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/tests/test_vm_operations.py:46-46
Timestamp: 2026-05-05T09:46:17.700Z
Learning: When the code under test performs *lazy imports* inside function bodies (i.e., the imported symbol is not bound at the module scope), mock/patch the *source module path that the function imports from*, not the consumer module path. The correct patch target is where the imported name is resolved at runtime (e.g., `virtualization.models.VirtualMachine`), because patching `netbox_librenms_plugin.import_utils.vm_operations.VirtualMachine` can fail with `AttributeError` since `VirtualMachine` is never a `vm_operations` module attribute.
Applied to files:
netbox_librenms_plugin/tests/test_tables_modules.pynetbox_librenms_plugin/tests/test_coverage_sync_interfaces.pynetbox_librenms_plugin/tests/test_utils.pynetbox_librenms_plugin/tests/test_coverage_base_views.pynetbox_librenms_plugin/tests/test_coverage_devices.pynetbox_librenms_plugin/tests/test_sync_interfaces.pynetbox_librenms_plugin/tests/test_librenms_api.pynetbox_librenms_plugin/tests/test_sync_modules.pynetbox_librenms_plugin/tests/test_modules_view.py
📚 Learning: 2026-03-08T14:23:14.395Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tables/modules.py:0-0
Timestamp: 2026-03-08T14:23:14.395Z
Learning: In Python/Django code, avoid wrapping a list already containing SafeString values (produced by format_html) with format_html("{}", mark_safe(...)). This is redundant and can raise Django 6.0 deprecation warnings. Instead, concatenate the strings directly and wrap once, e.g. use mark_safe("".join(str(b) for b in buttons)) and avoid nested format_html calls. Apply this pattern to files under netbox_librenms_plugin/tables/ (any .py files) to ensure SafeString handling remains explicit and compatible with Django 6.0.
Applied to files:
netbox_librenms_plugin/tables/modules.py
🔇 Additional comments (37)
netbox_librenms_plugin/views/object_sync/devices.py (1)
317-317: LGTM!Also applies to: 607-607
netbox_librenms_plugin/librenms_api.py (1)
173-198: LGTM!Also applies to: 220-229
netbox_librenms_plugin/views/sync/interfaces.py (1)
155-235: LGTM!Also applies to: 301-312
netbox_librenms_plugin/tests/test_tables_modules.py (1)
14-38: LGTM!Also applies to: 74-75, 551-587
netbox_librenms_plugin/tests/test_coverage_sync_interfaces.py (1)
445-498: LGTM!Also applies to: 525-558, 845-878
netbox_librenms_plugin/views/__init__.py (1)
21-21: LGTM!Also applies to: 113-113
netbox_librenms_plugin/urls.py (1)
59-59: LGTM!Also applies to: 188-192
netbox_librenms_plugin/tests/test_utils.py (1)
354-369: LGTM!Also applies to: 558-576
netbox_librenms_plugin/tests/test_coverage_base_views.py (1)
1083-1147: LGTM!netbox_librenms_plugin/views/base/interfaces_view.py (1)
10-10: LGTM!Also applies to: 73-102, 198-206, 221-230, 233-257
netbox_librenms_plugin/tests/test_coverage_devices.py (1)
229-230: LGTM!Also applies to: 378-478, 1006-1006, 1036-1036
netbox_librenms_plugin/tests/test_sync_interfaces.py (1)
153-155: LGTM!Also applies to: 216-233
netbox_librenms_plugin/tests/test_librenms_api.py (1)
526-569: LGTM!netbox_librenms_plugin/views/sync/modules.py (15)
14-23: LGTM!Also applies to: 60-77
89-123: LGTM!
140-223: LGTM!
226-266: LGTM!
269-361: LGTM!
363-441: LGTM!
444-487: LGTM!
489-589: LGTM!
592-689: LGTM!Also applies to: 797-915
1011-1068: LGTM!
1071-1213: LGTM!
1267-1306: LGTM!Also applies to: 1309-1328
1351-1355: LGTM!
1442-1644: LGTM!
1665-1669: LGTM!netbox_librenms_plugin/tests/test_sync_modules.py (1)
283-4922: LGTM!netbox_librenms_plugin/tests/test_modules_view.py (8)
105-338: LGTM!
1468-1520: LGTM!
2352-2476: LGTM!
2542-2550: LGTM!
3108-3170: LGTM!
3350-3356: LGTM!
3400-3579: LGTM!
4435-4500: LGTM!
| def render_module_bay(self, value, record): | ||
| """Render module bay with link if found in NetBox.""" | ||
| if not value or value == "-": | ||
| return format_html('<span class="text-danger">{}</span>', "No matching bay") | ||
| if url := record.get("module_bay_url"): | ||
| return format_html('<a href="{}">{}</a>', url, value) | ||
| return format_html("{}", value) | ||
| rendered_value = format_html('<span class="text-danger">{}</span>', "No matching bay") | ||
| elif url := record.get("module_bay_url"): | ||
| rendered_value = format_html('<a href="{}">{}</a>', url, value) | ||
| else: | ||
| rendered_value = value | ||
|
|
||
| # Mirror Name-column hierarchy marker in Module Bay for child rows. | ||
| depth = record.get("depth", 0) | ||
| if depth > 0: | ||
| padding_px = depth * 20 | ||
| return format_html('<span style="padding-left:{}px">└─ {}</span>', padding_px, rendered_value) | ||
|
|
||
| return rendered_value |
There was a problem hiding this comment.
Avoid rendering an error state for Integrated rows.
BaseModuleTableView._build_row() emits informational Integrated rows with module_bay == "-". This branch now turns those rows into a red “No matching bay” warning, which contradicts the status badge and makes a non-actionable duplicate look broken.
Suggested fix
def render_module_bay(self, value, record):
"""Render module bay with link if found in NetBox."""
- if not value or value == "-":
+ if record.get("status") == "Integrated":
+ rendered_value = "-"
+ elif not value or value == "-":
rendered_value = format_html('<span class="text-danger">{}</span>', "No matching bay")
elif url := record.get("module_bay_url"):
rendered_value = format_html('<a href="{}">{}</a>', url, value)
else:
rendered_value = value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def render_module_bay(self, value, record): | |
| """Render module bay with link if found in NetBox.""" | |
| if not value or value == "-": | |
| return format_html('<span class="text-danger">{}</span>', "No matching bay") | |
| if url := record.get("module_bay_url"): | |
| return format_html('<a href="{}">{}</a>', url, value) | |
| return format_html("{}", value) | |
| rendered_value = format_html('<span class="text-danger">{}</span>', "No matching bay") | |
| elif url := record.get("module_bay_url"): | |
| rendered_value = format_html('<a href="{}">{}</a>', url, value) | |
| else: | |
| rendered_value = value | |
| # Mirror Name-column hierarchy marker in Module Bay for child rows. | |
| depth = record.get("depth", 0) | |
| if depth > 0: | |
| padding_px = depth * 20 | |
| return format_html('<span style="padding-left:{}px">└─ {}</span>', padding_px, rendered_value) | |
| return rendered_value | |
| def render_module_bay(self, value, record): | |
| """Render module bay with link if found in NetBox.""" | |
| if record.get("status") == "Integrated": | |
| rendered_value = "-" | |
| elif not value or value == "-": | |
| rendered_value = format_html('<span class="text-danger">{}</span>', "No matching bay") | |
| elif url := record.get("module_bay_url"): | |
| rendered_value = format_html('<a href="{}">{}</a>', url, value) | |
| else: | |
| rendered_value = value | |
| # Mirror Name-column hierarchy marker in Module Bay for child rows. | |
| depth = record.get("depth", 0) | |
| if depth > 0: | |
| padding_px = depth * 20 | |
| return format_html('<span style="padding-left:{}px">└─ {}</span>', padding_px, rendered_value) | |
| return rendered_value |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbox_librenms_plugin/tables/modules.py` around lines 199 - 214, The current
render_module_bay function turns module_bay == "-" into a red "No matching bay"
even for informational "Integrated" rows created by
BaseModuleTableView._build_row; update render_module_bay to skip the error
styling for those rows by checking record.get("status") (or an equivalent flag
set by _build_row) and, when status == "Integrated" and value == "-", return the
plain value (or the original placeholder) instead of the red span; keep existing
behavior for true missing/empty bays and the module_bay_url linking logic.
| _VC_MEMBER_INTERFACE_PATTERN = re.compile(r"^(?P<prefix>[A-Za-z][A-Za-z0-9]*)(?P<member>\d+)(?P<suffix>[/:].+)$") | ||
|
|
There was a problem hiding this comment.
VC interface rewriting misses hyphenated interface families.
The new pattern only matches prefixes made of letters/digits, so names like xe-1/0/0, et-1/0/0, or ge-1/0/0 never rewrite to the selected member. On those platforms the module flow will keep the wrong member number and fail interface binding.
Suggested fix
-_VC_MEMBER_INTERFACE_PATTERN = re.compile(r"^(?P<prefix>[A-Za-z][A-Za-z0-9]*)(?P<member>\d+)(?P<suffix>[/:].+)$")
+_VC_MEMBER_INTERFACE_PATTERN = re.compile(
+ r"^(?P<prefix>[A-Za-z][A-Za-z0-9-]*)(?P<member>\d+)(?P<suffix>[/:].+)$"
+)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbox_librenms_plugin/utils.py` around lines 16 - 17, The regex
_VC_MEMBER_INTERFACE_PATTERN currently disallows hyphens in the interface family
prefix so names like "xe-1/0/0" don't match; update the pattern used by
_VC_MEMBER_INTERFACE_PATTERN to allow hyphens in the prefix (e.g. include '-' in
the character class for the prefix group) while preserving the member (\d+) and
suffix ([/:].+) groups so existing member and suffix capture behavior remains
unchanged.
| def _count_adoptable_template_interfaces(self, module): | ||
| """Count standalone interfaces that match an installed module's interface templates.""" | ||
| from dcim.models import Interface | ||
|
|
||
| device = getattr(module, "device", None) | ||
| if device is None: | ||
| return 0 | ||
|
|
||
| template_names = get_module_template_interface_names(device, module) | ||
| if not template_names: | ||
| return 0 | ||
|
|
||
| return Interface.objects.filter(device=device, module__isnull=True, name__in=template_names).count() |
There was a problem hiding this comment.
Cache adoptable-interface counts instead of querying once per row.
_build_row() calls this helper for every installed matched module, and each call does a fresh Interface.objects.filter(...).count(). On large module tables that adds one extra DB round-trip per row even though the result only depends on the device and the module's template interface names.
Suggested fix
def _count_adoptable_template_interfaces(self, module):
"""Count standalone interfaces that match an installed module's interface templates."""
from dcim.models import Interface
device = getattr(module, "device", None)
if device is None:
return 0
template_names = get_module_template_interface_names(device, module)
if not template_names:
return 0
- return Interface.objects.filter(device=device, module__isnull=True, name__in=template_names).count()
+ cache_key = (device.pk, tuple(sorted(template_names)))
+ cache = getattr(self, "_adoptable_interface_count_cache", {})
+ if cache_key in cache:
+ return cache[cache_key]
+
+ count = Interface.objects.filter(device=device, module__isnull=True, name__in=template_names).count()
+ cache[cache_key] = count
+ self._adoptable_interface_count_cache = cache
+ return count🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbox_librenms_plugin/views/base/modules_view.py` around lines 191 - 203,
The helper _count_adoptable_template_interfaces currently does a DB count each
time it is called from _build_row, causing N extra queries; change it to cache
results keyed by (device.pk, tuple(template_names)) on the view instance (or use
functools.lru_cache on a helper that accepts device id and an immutable template
names tuple) so repeated calls return the cached integer instead of re-running
Interface.objects.filter(device=device, module__isnull=True,
name__in=template_names).count(); update _build_row to call the cached helper
and ensure the cache key uses device id and an ordered/hashed form of
get_module_template_interface_names(device, module) to avoid collisions.
| if matched_type is not None and installed.module_type_id == matched_type.pk: | ||
| adoptable_interface_count = self._count_adoptable_template_interfaces(installed) | ||
| if adoptable_interface_count: | ||
| row["can_update_interface_binding"] = True | ||
| row["adoptable_interface_count"] = adoptable_interface_count |
There was a problem hiding this comment.
Only mark rows updateable after a concrete interface match exists.
This sets can_update_interface_binding as soon as the installed module type matches and there are adoptable template interfaces on the device. If _attach_interface_match() later fails to resolve a unique interface, the row still keeps that flag and render_actions() will show Update Interface without any verified matched_interface_id.
Suggested fix
if matched_type is not None and installed.module_type_id == matched_type.pk:
adoptable_interface_count = self._count_adoptable_template_interfaces(installed)
if adoptable_interface_count:
- row["can_update_interface_binding"] = True
row["adoptable_interface_count"] = adoptable_interface_countThen let _attach_interface_match() remain the only place that sets can_update_interface_binding, or make the table action also require matched_interface_id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbox_librenms_plugin/views/base/modules_view.py` around lines 2255 - 2259,
The code sets row["can_update_interface_binding"] when matched_type matches and
_count_adoptable_template_interfaces(installed) > 0, but that can be premature
if _attach_interface_match() later fails to resolve a unique interface; instead
remove/stop setting can_update_interface_binding here and either (a) let
_attach_interface_match() be the sole setter of
row["can_update_interface_binding"] after it has successfully resolved and
assigned matched_interface_id, or (b) if you prefer to keep this check, require
that a verified matched_interface_id exists before setting the flag (i.e., only
set can_update_interface_binding when matched_interface_id is present); update
render_actions to rely on matched_interface_id rather than this early flag if
needed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html (1)
179-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the selected LibreNMS server when posting device-type mappings.
These new
_dt_mapping_form.htmlincludes don't carryserver_key, butAddDeviceTypeMappingView.post()only rehydrates the correct API client fromrequest.POST["server_key"]. On multi-server imports, submitting one of these forms can therefore read/update mappings in the default server context instead of the server the validation modal was opened for. Please serializeserver_keyfrom the shared partial.Also applies to: 213-213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html` around lines 179 - 199, The device-type mapping forms render from the shared partial _dt_mapping_form.html but currently omit server_key, causing AddDeviceTypeMappingView.post() to pick up the wrong API client via request.POST["server_key"]; update the partial to accept and render a hidden input named "server_key" (e.g. <input type=\"hidden\" name=\"server_key\" value=\"{{ server_key }}\">) and update every include in device_validation_details.html (the two places that include _dt_mapping_form.html) to pass the current server_key into the include so the posted form always carries the correct server context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html`:
- Around line 20-24: The platform-mapping modal loses the server context because
the included template "netbox_librenms_plugin/htmx/_platform_mapping_form.html"
does not send server_key, so modify the form include in
create_platform_modal.html (or the shared _platform_mapping_form.html) to submit
the server_key: either add a hidden input named "server_key" populated with the
current server_key value or add hx-include targeting a hidden server_key element
in the surrounding modal so that AddPlatformMappingView.post() can read
request.POST["server_key"] and apply the mapping to the correct LibreNMS server.
---
Outside diff comments:
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html`:
- Around line 179-199: The device-type mapping forms render from the shared
partial _dt_mapping_form.html but currently omit server_key, causing
AddDeviceTypeMappingView.post() to pick up the wrong API client via
request.POST["server_key"]; update the partial to accept and render a hidden
input named "server_key" (e.g. <input type=\"hidden\" name=\"server_key\"
value=\"{{ server_key }}\">) and update every include in
device_validation_details.html (the two places that include
_dt_mapping_form.html) to pass the current server_key into the include so the
posted form always carries the correct server context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d378790-fdb9-4ddf-88bb-81c91db1db4d
📒 Files selected for processing (4)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/views/imports/actions.py
📜 Review details
⏰ 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). (3)
- GitHub Check: test-netbox (3.13)
- GitHub Check: test-netbox (3.14)
- GitHub Check: test-netbox (3.12)
🧰 Additional context used
📓 Path-based instructions (7)
netbox_librenms_plugin/templates/**/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/*.html: HTMX 2.x is the primary async layer. Table row updates should return<tr hx-swap-oob="true">.
AvoidouterHTMLswaps in HTMX; use OOB or targetedinnerHTMLswaps to keep table layout intact.
Do not reintroducedata-bs-toggleor duplicate modal IDs in modal implementation.
Keep<select class="device-role-select">markup stable to preserve JavaScript hook-up for TomSelect decorators.
Do not re-addtable-responsivewrappers as their removal was deliberate to prevent dropdown clipping.
Templates live intemplates/netbox_librenms_plugin/; reuse and includes go underinc/subdirectory.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,js}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
All HTMX requests and
fetch()calls must include a CSRF token. Prefer extracting from hidden form input viadocument.querySelector('[name=csrfmiddlewaretoken]').valuerather than cookie-based approach.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/**/*.{html,css}
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
Styling assumes Tabler defaults for the netbox_librenms_plugin frontend.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/*.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
HTMX fragments live in
templates/netbox_librenms_plugin/htmx/including:device_import_row.html,device_validation_details.html,device_vc_details.html,bulk_import_confirm.html.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
**/views/imports/**
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/**: NetBox's/api/core/background-tasks/endpoint requires superuser (IsSuperuserinBaseRQViewSet). Non-superuser users cannot poll job status (403 Forbidden). Plugin automatically falls back to synchronous mode for non-superusers viashould_use_background_job()inlist.pyandactions.py
Import page filter fields:librenms_location,librenms_type,librenms_os,librenms_hostname,librenms_sysname,librenms_hardware,enable_vc_detection,show_disabled,exclude_existing
Files:
netbox_librenms_plugin/views/imports/actions.py
**/views/imports/actions.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/actions.py:DeviceImportHelperMixinprovidesget_validated_device_with_selections()andrender_device_row()for HTMX row rendering, shared by update views
BulkImportConfirmView(POST) — renders confirmation modal with selected device list viahtmx/bulk_import_confirm.html
BulkImportDevicesView(POST) — executes import. Background mode enqueuesImportDevicesJob; sync mode callsbulk_import_devices()+bulk_import_vms()and returns OOB row swaps withHX-Trigger: closeModal
DeviceValidationDetailsView(GET) — renders expandable validation details viahtmx/device_validation_details.html
DeviceVCDetailsView(GET) — renders VC member details viahtmx/device_vc_details.html
DeviceRoleUpdateView,DeviceClusterUpdateView,DeviceRackUpdateView(POST) — per-device dropdown updates. Apply selection to validation state and return re-rendered row viarender_device_row()
Files:
netbox_librenms_plugin/views/imports/actions.py
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building
HttpResponsefrom Django-template-rendered HTML in views, useformat_html()to compose the envelope andmark_safe()on the inner HTML to clear CodeQLpy/reflected-xssfalse positives. Example:format_html('<div id="target" hx-swap-oob="innerHTML">{}</div>', mark_safe(modal_html))
Files:
netbox_librenms_plugin/views/imports/actions.py
🧠 Learnings (10)
📚 Learning: 2026-03-13T11:16:36.294Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:136-137
Timestamp: 2026-03-13T11:16:36.294Z
Learning: In Django templates under netbox_librenms_plugin/templates/**/*.html, do not suggest adding explicit parentheses to {% if %} expressions for readability. The project favors compact expressions using implicit operator precedence (and binds tighter than or). Treat parentheses as cosmetic and avoid guidance to insert them for style reasons.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-01T08:25:06.260Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html:245-246
Timestamp: 2026-05-01T08:25:06.260Z
Learning: In netbox_librenms_plugin template HTML/HTMX code, only require an X-CSRFToken header for state-changing requests made via fetch() or HTMX (POST, PUT, PATCH, DELETE). Do not require X-CSRFToken on read-only fetch() GET calls (e.g., autocomplete/lookup endpoints like dcim-api:devicetype-list); Django/DRF exempt GET requests from CSRF validation. Therefore, code reviews should not flag missing CSRF headers on GET fetch() calls used for lookups/autocomplete.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-13T20:03:16.435Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/tests/test_coverage_api2.py:319-355
Timestamp: 2026-03-13T20:03:16.435Z
Learning: Do not propose replacing server_info with module_sync.server_key in the templates located under netbox_librenms_plugin/templates/netbox_librenms_plugin (specifically _module_sync.html and inc/_module_sync.html). These templates rely on server_info being present in the parent template context (librenms_sync_base.html) and server_key may be absent on initial load. Treat the correct usage of server_info for populating the value as the intended pattern; only flag issues if server_key is incorrectly used in these templates. This guideline applies to all files under the templates path for this plugin.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-05-25T21:48:19.264Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html:12-12
Timestamp: 2026-05-25T21:48:19.264Z
Learning: In this plugin’s HTMX form templates, `hx-include` selectors that target toggle preference wrapper `<span>` element IDs (e.g., `#use-sysname-toggle`, `#strip-domain-toggle`, `#auto-create-ipam-toggle`) are intentional. Those wrapper spans contain both the hidden `off` fallback input and the checkbox; HTMX must include the wrapper so the correct value is serialized, including the unchecked/off state. Do not recommend changing `hx-include` to the checkbox IDs with the `-cb` suffix, since it would omit the hidden fallback and break unchecked/off state submission.
Applied to files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
📚 Learning: 2026-03-07T22:46:57.537Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/tests/test_librenms_id.py:184-184
Timestamp: 2026-03-07T22:46:57.537Z
Learning: Do not flag or request style-only changes (such as removing redundant imports or cosmetic cleanup) in Python code reviews. Focus on issues that affect correctness, functionality, or security. This guideline applies to Python files across the repository, including tests (e.g., netbox_librenms_plugin/tests/test_librenms_id.py).
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T09:30:45.499Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 21
File: netbox_librenms_plugin/views/imports/actions.py:1031-1035
Timestamp: 2026-03-08T09:30:45.499Z
Learning: In netbox_librenms_plugin/views/imports/actions.py, ensure that DeviceConflictActionView.post() explicitly rejects boolean values for librenms_id via isinstance(librenms_id, bool) before coercing to int, returning HTTP 400. Do not remove or consolidate this pre-coercion boolean check. This guard is intentional and consistent with the similar bool-guard pattern used in set_librenms_device_id, get_librenms_device_id, and find_by_librenms_id; preserve this behavior to avoid ambiguity and misinterpretation of truthy/falsey booleans.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T13:09:49.031Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/forms.py:75-79
Timestamp: 2026-03-08T13:09:49.031Z
Learning: In multi-server caching within the codebase, ensure poller group choices and similar cache discriminators use api.server_key as the cache key component (e.g., cache_key = f"librenms_poller_group_choices_{api.server_key}") rather than api.librenms_url. This aligns with the fixed approach seen in commit bf37f07 and with other modules. Apply this pattern consistently to Python files under netbox_librenms_plugin (and similar multi-server cache keys) to maintain correct cross-server caching behavior.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-05T09:51:15.707Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 58
File: netbox_librenms_plugin/views/base/modules_view.py:311-313
Timestamp: 2026-05-05T09:51:15.707Z
Learning: In this repository, if you see `timezone.timedelta` used from `django.utils.timezone`, do not request a diff to replace it with `datetime.timedelta` solely on style grounds. This usage is functionally correct because Django exposes `timedelta` via `django.utils.timezone`; treat this as intentional and avoid churn unless there is evidence of changed/incorrect behavior (e.g., `timezone` is not Django’s module or `timedelta` is unavailable).
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-04-15T12:38:49.280Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 50
File: netbox_librenms_plugin/views/base/modules_view.py:133-141
Timestamp: 2026-04-15T12:38:49.280Z
Learning: Do not require or flag an explicit `permission_required = PERM_VIEW_PLUGIN` on views that inherit from `LibreNMSPermissionMixin` (e.g., those ultimately including `BaseModuleTableView` in `views/base/modules_view.py`). `LibreNMSPermissionMixin` is the authoritative place where `permission_required` is set to `PERM_VIEW_PLUGIN`; inherited values are already enforced via the mixin, making a redundant override unnecessary.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-05-17T11:32:40.631Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 76
File: netbox_librenms_plugin/views/object_sync/devices.py:178-180
Timestamp: 2026-05-17T11:32:40.631Z
Learning: In this plugin’s JSON endpoint views (e.g., NetBox view classes/functions under netbox_librenms_plugin/views/**), do not require a try/except JSONDecodeError around `json.loads(request.body)` by default if the endpoint is already protected by CSRF + session authentication and has object-level permission gates. Treat missing JSONDecodeError handling as acceptable when the only expected downside is a noisy log line. If you identify any additional security or availability impact from invalid JSON (e.g., unhandled exceptions leading to user-visible 500s, potential DoS amplification, or lack of appropriate throttling/validation), then flag it and recommend adding guarded parsing/validation.
Applied to files:
netbox_librenms_plugin/views/imports/actions.py
🪛 HTMLHint (1.9.2)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_manage_icon.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/create_platform_modal.html
[error] 21-21: Special characters must be escaped : [ > ].
(spec-char-escape)
- Extract device type mapping form into reusable partial (_dt_mapping_form.html) - Extract platform mapping form into reusable partial (_platform_mapping_form.html) - Add AddPlatformMappingView for creating PlatformMapping from import modal - Register add_platform_mapping URL - device_validation_details.html: replace 100-line inline DT form with include; add DT mapping option in mismatch branch; add no-mapping elif with warning; add platform mapping form in platform row - librenms_sync_base.html: fix VM-without-cluster breadcrumb; add object_model_name guard on platform modal; rename _create_platform_url -> create_platform_url - librenms_import.html: wrap toggles with span+hidden-input so unchecked values submit; rename checkbox IDs to -cb suffix - librenms_import.js: rename toggle IDs to match -cb suffix; fix nested modal dismiss so inner modals (e.g. Promote-to-host picker) are not broken
… reachable
Outer {% if validation.existing_device and ... %} at line 293 was missing
its {% endif %}, breaking template compilation. The new-import branch's
platform mapping include was also guarded on sync_info, which is only set
when an existing device matches — so the form never rendered for new
imports. Drop the sync_info clause and the unreachable preselect_platform.
The upfront permission check at line 1849-1854 selects "add" or "change" based on whether a mapping exists, but the get_or_create that follows could race with a concurrent delete: a caller with only "change" permission could end up creating a new row. Mirror the established AddDeviceTypeMappingView pattern — lock the row with select_for_update, re-derive the required permission from the locked state, and surface IntegrityError on concurrent inserts.
Mirror the sibling AddDeviceTypeMappingView style for consistent HTMX toast behavior on validation errors, and reject the LibreNMS "-" placeholder OS that previously slipped past the empty-string check.
Existing devices whose LibreNMS OS has no PlatformMapping previously only got the 'Create Platform' path, nudging users toward duplicate Platform records. Mirror the device-type flow: also offer the platform-mapping form — preselected to the device's current platform when it has one, or unselected when it doesn't — so the LibreNMS OS can be mapped to an existing platform instead. Addresses CodeRabbit feedback surfaced on PR #84 (the file belongs to this branch / PR bonzo81#299).
… locking AddPlatformMappingView now rejects an ambiguous OS string up front, and inside the transaction materialises the candidate rows with select_for_update()[:2] instead of count(). count() silently drops the FOR UPDATE clause, so the rows were never actually locked before the create/update decision (race window). Addresses CodeRabbit feedback on PR bonzo81#303; the platform-mapping logic belongs to this branch / PR bonzo81#299.
…ed modal The platform cell in the import validation modal had become cluttered (inline mapping typeahead + Create Platform button + sync button all at once). Replace the inline forms/buttons with a single pencil icon that opens one modal offering BOTH options: - map the LibreNMS OS to an existing platform (typeahead), and - create a new platform. The icon also appears where a platform is already resolved (direct match or via mapping), so the mapping can be changed compactly. The combined modal reuses the existing create-platform modal plus the _platform_mapping_form partial, guarded so the device-sync page (create-only, no libre_device) is unaffected. - _platform_manage_icon.html: the compact pencil-icon trigger - create_platform_modal.html: adds a 'map to existing' section when libre_device is present; neutral alert wording - CreatePlatformFromImportView.get: passes libre_device + current_platform - device_validation_details.html: platform cell rewritten to use the icon; inline _platform_mapping_form includes + Create Platform buttons removed
… leaked as text)
Django's {# #} comment cannot span multiple lines, so the multi-line header
comment rendered literally in the platform cell. Switch to a {% comment %}
block.
Static scan of all plugin .html templates flagging any line with an unbalanced
{# / #} count (the signature of a multi-line {# #}, which Django renders as
literal text). Would have caught the _platform_manage_icon regression that
get_template()/template-compile checks missed.
- _dt_mapping_form / _platform_mapping_form: post the server_key hidden field
(inherited from the render context) so Add{DeviceType,Platform}MappingView run
the post-save re-validation + cache invalidation against the same LibreNMS
server the modal was rendered for, not the default — fixes wrong-server
context in multi-server setups.
- AddPlatformMappingView: logger.exception instead of logger.warning in the save
failure handler, to preserve the traceback and match AddDeviceTypeMappingView.
Addresses Copilot review on bonzo81#299.
ba2ea09 to
abf769b
Compare
Integrated rows are informational duplicates for a component fused into a parent module (BaseModuleTableView._build_row emits them with module_bay == "-" and status "Integrated"). render_status shows a muted badge and render_actions returns no actions for them, but render_module_bay still painted a red 'No matching bay' warning, which contradicts the badge and makes a non-actionable row look broken. Guard on status == "Integrated" (mirroring render_actions) and render a plain '-' instead. Addresses a CodeRabbit review comment on PR #83.
1 participant
Summary
Briefly describe what this PR does in plain English, and provide as much of the following information as possible.
Motivation / Problem
What issue does this solve?
Link any related issues if applicable.
Scope of Change
Delete items that don’t apply:
How Was This Tested?
Delete items that don’t apply and describe briefly.
Manual Test Steps (if applicable)
Risk Assessment
Explain briefly.
Backwards Compatibility
Other Notes
Anything the maintainer(s) should pay particular attention to?
Summary by CodeRabbit
Release Notes
New Features
Documentation
UI Improvements