Conversation
|
Warning Review limit reached
More reviews will be available in 59 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 (22)
📝 WalkthroughWalkthroughThis PR implements optional primary IP auto-assignment from LibreNMS management IP during sync, introduces HTMX endpoints and reusable form templates for platform/device-type mapping, refactors the validation modal to use shared partials, updates frontend controls and settings, and ships comprehensive documentation for Module Sync and Mapping Rules configuration. ChangesPrimary IP Sync, Mapping Forms, and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 list entry text in docs/usage_tips/README.md that
currently reads "Multi Server Configuration" to the hyphenated form
"Multi-Server Configuration" so the compound modifier is correctly formatted;
change the link text and visible title in the markdown list item (the string
"Multi Server Configuration") to "Multi-Server Configuration".
In `@netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html`:
- Around line 190-200: The toggle input import_form.auto_create_ipam_default is
not included in the form-dirty detection so flipping it doesn’t enable the "Save
Import Settings" button; update the dirty-state wiring to listen for
change/input events on the checkbox/input for
import_form.auto_create_ipam_default (use its id_for_label or DOM id) and call
the existing dirty-state handler (the function that currently toggles the "Save
Import Settings" button) when it changes, ensuring the save button becomes
enabled when only this field is modified.
In `@netbox_librenms_plugin/utils.py`:
- Around line 293-295: The code uses bool(pref) which treats any non-empty
string like "false" as True; update the logic in the block that reads
get_user_pref(request, "plugins.netbox_librenms_plugin.auto_create_ipam") to
explicitly coerce string and numeric values: if pref is already a bool return
it, otherwise normalize str(pref).strip().lower() and return True for accepted
truthy tokens ("true","1","yes","on") and False for falsy tokens
("false","0","no","off"); ensure this logic is applied where pref is inspected
so auto_create_ipam reflects the actual configured value.
In `@netbox_librenms_plugin/views/imports/list.py`:
- Around line 165-168: The _auto_create_ipam value returned from get_user_pref
should be normalized to a boolean because string values like "false" are truthy
in templates; update the handling around where _auto_create_ipam is set (and the
similar spots at the occurrences around lines referenced) to detect string types
and convert common truthy/falsey strings (e.g. "true","1","yes" -> True;
"false","0","no" -> False) otherwise fall back to the settings_obj
auto_create_ipam_default or False; ensure normalization happens immediately
after calling get_user_pref so templates and subsequent logic use a real bool.
🪄 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: c8f364a0-4739-443b-b69c-843c7ea673be
⛔ 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 (27)
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/forms.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/models.pynetbox_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/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/forms.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/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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,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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.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/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/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/**/settings.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/settings.html:settings.htmluses a split-form pattern with two separate Django forms (ServerConfigForm+ImportSettingsForm) sharing one page, differentiated by a hiddenform_typefield.
The settings page test-connection button should be an HTMX POST toTestLibreNMSConnectionView, returning an inline alert fragment.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.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
**/import_utils/__init__.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
import_utils/is a package; the__init__.pymust re-export key functions so callers can usefrom import_utils import ...for functions fromfilters.py,device_operations.py,vm_operations.py,cache.py,permissions.py, andvirtual_chassis.py
Files:
netbox_librenms_plugin/import_utils/__init__.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/list.pynetbox_librenms_plugin/views/imports/actions.py
**/views/imports/list.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/list.py: Import page (LibreNMSImportViewinviews/imports/list.py) supports two modes: (1) Synchronous — callsprocess_device_filters()directly, renders results inline. (2) Background — enqueuesFilterDevicesJob, returnsJsonResponsewithjob_id/job_pk/poll_url. Frontend polls and redirects to?job_id={pk}on completion
Result loading in import views:_load_job_results(job_id)readsjob.data["device_ids"], reconstructs devices from per-device cache usingget_validated_device_cache_key()
Files:
netbox_librenms_plugin/views/imports/list.py
**/import_utils/vm_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
vm_operations.pymust export:bulk_import_vms(vm_imports, user, ...)
Files:
netbox_librenms_plugin/import_utils/vm_operations.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
**/import_utils/device_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
device_operations.pymust export:validate_device_for_import(device, ...)andbulk_import_devices_shared(devices, user, ...)
Files:
netbox_librenms_plugin/import_utils/device_operations.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 (21)
📚 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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/forms.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/forms.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/forms.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/list.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/list.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/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.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-03-08T14:17:28.826Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/import_utils/virtual_chassis.py:73-80
Timestamp: 2026-03-08T14:17:28.826Z
Learning: In Python code, when a lookup returns None (including API failures), implement negative caching by storing an empty result with a configurable TTL (default 5 minutes). Document the TTL and ensure a force_refresh=True bypasses the cache for manual re-fetch actions. Do not treat caching None/empty results on API failure as a bug if this mirrors existing patterns (e.g., get_device_with_server caching None on not-found). Apply this guidance to files within netbox_librenms_plugin/import_utils where similar inventory/API lookups occur.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/device_operations.py
📚 Learning: 2026-03-09T20:10:48.502Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/import_utils/device_operations.py:496-575
Timestamp: 2026-03-09T20:10:48.502Z
Learning: In netbox_librenms_plugin/import_utils/device_operations.py, in validate_device_for_import(), ensure both the cluster-required blocker (VM path) and the device_role-required blocker (device path) are guarded with if not result.get('existing_device') to ensure create-time prerequisites are only appended for new imports, not for link/update flows; keep available_roles and available_clusters populated for both new and existing-device cases so UI dropdowns function correctly on update views.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/device_operations.py
📚 Learning: 2026-05-22T19:37:46.167Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/tests/test_import_utils.py:1763-1766
Timestamp: 2026-05-22T19:37:46.167Z
Learning: In `validate_device_for_import` (device operations / serial-diff name-resolution logic), preserve the following result contract so the UI/test expectations remain stable:
- If `serial_action` is determined via a serial match AND the existing device has NO OOB/LibreNMS link, set `serial_action` to `"oob_candidate"` and ensure `promote_to_host` is NOT present in the returned dict.
- Populate `promote_to_host` only when the existing device already has an OOB/LibreNMS link (i.e., a host id is available to inherit from); otherwise omit the key.
- Always include `serial_role_choice_available` in the returned dict, defaulting to `False` (baseline) even when other resolution outcomes do not enable it.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/device_operations.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-08T11:35:34.876Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 23
File: netbox_librenms_plugin/import_utils/vm_operations.py:61-69
Timestamp: 2026-03-08T11:35:34.876Z
Learning: In netbox_librenms_plugin/import_utils/vm_operations.py, do not flag the bare-int librenms_id in create_vm_from_librenms as a multi-server bug on the pr/librenms-pre-id-multi-server branch. This file is on a staging branch where per-server scoping migration (set_librenms_device_id(vm, device_id, server_key)) will occur in the next multi-server PR. Treat this as a branch-specific exception and plan to enforce the per-server convention only in the subsequent PR, not on this branch.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.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_ip_helpers.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_ip_helpers.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_ip_helpers.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] 1910-1910: 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] 1917-1917: 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 (22)
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-25: LGTM!Also applies to: 27-27, 33-33
docs/usage_tips/README.md (1)
14-17: LGTM!Also applies to: 23-28
docs/usage_tips/mapping_rules.md (1)
1-230: LGTM!docs/usage_tips/module_sync.md (1)
1-57: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
1-110: 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/htmx/_dt_mapping_form.html (1)
1-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, 366-372
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html (1)
1006-1012: LGTM!netbox_librenms_plugin/forms.py (1)
165-174: LGTM!Also applies to: 182-182
netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py (1)
1-23: LGTM!netbox_librenms_plugin/models.py (1)
82-89: LGTM!netbox_librenms_plugin/import_utils/ip_helpers.py (1)
29-43: LGTM!Also applies to: 46-111
netbox_librenms_plugin/tests/test_ip_helpers.py (1)
10-174: LGTM!netbox_librenms_plugin/import_utils/__init__.py (1)
53-53: LGTM!netbox_librenms_plugin/import_utils/device_operations.py (1)
752-754: LGTM!Also applies to: 758-758, 770-770, 793-793, 826-826, 835-835, 844-844, 904-921, 951-951, 962-962
netbox_librenms_plugin/import_utils/bulk_import.py (1)
203-203: LGTM!netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (2)
352-357: LGTM!
1204-1211: LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
netbox_librenms_plugin/views/imports/list.py (1)
165-167:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize
auto_create_ipampreference to a real boolean before storing/rendering.At Line 165 and then reused at Line 171/Line 357, string prefs like
"false"remain truthy in templates, so the toggle can render enabled incorrectly.Suggested minimal fix
_auto_create_ipam = get_user_pref(request, "plugins.netbox_librenms_plugin.auto_create_ipam") + if isinstance(_auto_create_ipam, str): + _auto_create_ipam = _auto_create_ipam.strip().lower() in {"1", "true", "on", "yes"} if _auto_create_ipam is None: _auto_create_ipam = getattr(settings_obj, "auto_create_ipam_default", False) if settings_obj else FalseAlso applies to: 171-171, 357-357
🤖 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/imports/list.py` around lines 165 - 167, The user preference _auto_create_ipam retrieved via get_user_pref(request, "plugins.netbox_librenms_plugin.auto_create_ipam") must be normalized to a real boolean before use: update the code that sets _auto_create_ipam (and the other occurrences at the same symbol usage) to treat string values like "true"/"false" (and common variants "1"/"0", "yes"/"no") by lowercasing and comparing to truthy tokens, keep the fallback to getattr(settings_obj, "auto_create_ipam_default", False) when get_user_pref returns None, and ensure the final _auto_create_ipam variable is always a Python bool before storing or passing to templates/renderer.
🤖 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.
Duplicate comments:
In `@netbox_librenms_plugin/views/imports/list.py`:
- Around line 165-167: The user preference _auto_create_ipam retrieved via
get_user_pref(request, "plugins.netbox_librenms_plugin.auto_create_ipam") must
be normalized to a real boolean before use: update the code that sets
_auto_create_ipam (and the other occurrences at the same symbol usage) to treat
string values like "true"/"false" (and common variants "1"/"0", "yes"/"no") by
lowercasing and comparing to truthy tokens, keep the fallback to
getattr(settings_obj, "auto_create_ipam_default", False) when get_user_pref
returns None, and ensure the final _auto_create_ipam variable is always a Python
bool before storing or passing to templates/renderer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: a858c932-fac9-4d8c-80d7-622d6c739d6b
📒 Files selected for processing (23)
docs/usage_tips/README.mddocs/usage_tips/module_sync.mdnetbox_librenms_plugin/forms.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/models.pynetbox_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/templates/netbox_librenms_plugin/settings.htmlnetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/import_utils/device_operations.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/settings.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/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/settings.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/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/settings.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/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html
netbox_librenms_plugin/templates/**/settings.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/settings.html:settings.htmluses a split-form pattern with two separate Django forms (ServerConfigForm+ImportSettingsForm) sharing one page, differentiated by a hiddenform_typefield.
The settings page test-connection button should be an HTMX POST toTestLibreNMSConnectionView, returning an inline alert fragment.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.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
**/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/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
**/import_utils/vm_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
vm_operations.pymust export:bulk_import_vms(vm_imports, user, ...)
Files:
netbox_librenms_plugin/import_utils/vm_operations.py
**/import_utils/__init__.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
import_utils/is a package; the__init__.pymust re-export key functions so callers can usefrom import_utils import ...for functions fromfilters.py,device_operations.py,vm_operations.py,cache.py,permissions.py, andvirtual_chassis.py
Files:
netbox_librenms_plugin/import_utils/__init__.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/list.pynetbox_librenms_plugin/views/imports/actions.py
**/views/imports/list.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/list.py: Import page (LibreNMSImportViewinviews/imports/list.py) supports two modes: (1) Synchronous — callsprocess_device_filters()directly, renders results inline. (2) Background — enqueuesFilterDevicesJob, returnsJsonResponsewithjob_id/job_pk/poll_url. Frontend polls and redirects to?job_id={pk}on completion
Result loading in import views:_load_job_results(job_id)readsjob.data["device_ids"], reconstructs devices from per-device cache usingget_validated_device_cache_key()
Files:
netbox_librenms_plugin/views/imports/list.py
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
**/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
**/import_utils/device_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
device_operations.pymust export:validate_device_for_import(device, ...)andbulk_import_devices_shared(devices, user, ...)
Files:
netbox_librenms_plugin/import_utils/device_operations.py
🧠 Learnings (21)
📚 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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/import_utils/device_operations.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/import_utils/device_operations.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/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/import_utils/device_operations.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/settings.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/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/settings.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/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/settings.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/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.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/list.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/list.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-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_ip_helpers.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_ip_helpers.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_ip_helpers.py
📚 Learning: 2026-03-08T11:35:34.876Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 23
File: netbox_librenms_plugin/import_utils/vm_operations.py:61-69
Timestamp: 2026-03-08T11:35:34.876Z
Learning: In netbox_librenms_plugin/import_utils/vm_operations.py, do not flag the bare-int librenms_id in create_vm_from_librenms as a multi-server bug on the pr/librenms-pre-id-multi-server branch. This file is on a staging branch where per-server scoping migration (set_librenms_device_id(vm, device_id, server_key)) will occur in the next multi-server PR. Treat this as a branch-specific exception and plan to enforce the per-server convention only in the subsequent PR, not on this branch.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-03-08T14:17:28.826Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/import_utils/virtual_chassis.py:73-80
Timestamp: 2026-03-08T14:17:28.826Z
Learning: In Python code, when a lookup returns None (including API failures), implement negative caching by storing an empty result with a configurable TTL (default 5 minutes). Document the TTL and ensure a force_refresh=True bypasses the cache for manual re-fetch actions. Do not treat caching None/empty results on API failure as a bug if this mirrors existing patterns (e.g., get_device_with_server caching None on not-found). Apply this guidance to files within netbox_librenms_plugin/import_utils where similar inventory/API lookups occur.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.py
📚 Learning: 2026-03-09T20:10:48.502Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/import_utils/device_operations.py:496-575
Timestamp: 2026-03-09T20:10:48.502Z
Learning: In netbox_librenms_plugin/import_utils/device_operations.py, in validate_device_for_import(), ensure both the cluster-required blocker (VM path) and the device_role-required blocker (device path) are guarded with if not result.get('existing_device') to ensure create-time prerequisites are only appended for new imports, not for link/update flows; keep available_roles and available_clusters populated for both new and existing-device cases so UI dropdowns function correctly on update views.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.py
📚 Learning: 2026-05-22T19:37:46.167Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/tests/test_import_utils.py:1763-1766
Timestamp: 2026-05-22T19:37:46.167Z
Learning: In `validate_device_for_import` (device operations / serial-diff name-resolution logic), preserve the following result contract so the UI/test expectations remain stable:
- If `serial_action` is determined via a serial match AND the existing device has NO OOB/LibreNMS link, set `serial_action` to `"oob_candidate"` and ensure `promote_to_host` is NOT present in the returned dict.
- Populate `promote_to_host` only when the existing device already has an OOB/LibreNMS link (i.e., a host id is available to inherit from); otherwise omit the key.
- Always include `serial_role_choice_available` in the returned dict, defaulting to `False` (baseline) even when other resolution outcomes do not enable it.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.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-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] 1911-1911: 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] 1918-1918: 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 (22)
docs/usage_tips/README.md (1)
18-18: LGTM!docs/usage_tips/module_sync.md (1)
38-38: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
10-109: 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/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, 366-371
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_sync_base.html (1)
1006-1006: LGTM!Also applies to: 1011-1012
netbox_librenms_plugin/forms.py (1)
165-174: LGTM!Also applies to: 182-182
netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py (1)
1-23: LGTM!netbox_librenms_plugin/models.py (1)
82-89: LGTM!netbox_librenms_plugin/import_utils/ip_helpers.py (1)
1-111: LGTM!netbox_librenms_plugin/import_utils/__init__.py (1)
53-53: LGTM!netbox_librenms_plugin/tests/test_ip_helpers.py (1)
1-174: LGTM!netbox_librenms_plugin/import_utils/device_operations.py (1)
752-754: LGTM!Also applies to: 758-758, 770-770, 793-793, 826-826, 835-835, 844-844, 904-921, 951-951, 962-962
netbox_librenms_plugin/import_utils/bulk_import.py (1)
203-203: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html (1)
190-200: New import toggle isn’t included in import-settings dirty-state detection.Changing
auto_create_ipam_defaultalone still won’t enable Save Import Settings because the dirty-state logic does not read this checkbox (see Line 334/Line 344 in the same file).netbox_librenms_plugin/utils.py (1)
293-295: Preference coercion at fallback still flips string falsy values toTrue.At Line 295,
bool(pref)treats"false"/"0"as truthy because they are non-empty strings, which can incorrectly enable auto-create IPAM.netbox_librenms_plugin/import_utils/vm_operations.py (1)
25-25: LGTM!Also applies to: 88-106, 198-199, 257-264
netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html (1)
372-382: LGTM!Also applies to: 388-388
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (1)
352-357: LGTM!Also applies to: 1204-1212
netbox_librenms_plugin/views/imports/actions.py (1)
567-577: LGTM!Also applies to: 759-776, 1796-1801, 1820-1922
- utils.py: resolve_auto_create_ipam now checks isinstance(pref, str) before bool(pref) so stored strings like 'false' are not coerced to True - views/imports/list.py: normalize _auto_create_ipam from get_user_pref to a real bool immediately, preventing string-truthy rendering bugs - settings.html: wire id_auto_create_ipam_default into dirty-state JS so toggling it alone enables the Save Import Settings button Addresses CodeRabbit review comments on PR #84.
- utils.py: resolve_auto_create_ipam now checks isinstance(pref, str) before bool(pref) so stored strings like 'false' are not coerced to True - views/imports/list.py: normalize _auto_create_ipam from get_user_pref to a real bool immediately, preventing string-truthy rendering bugs - settings.html: wire id_auto_create_ipam_default into dirty-state JS so toggling it alone enables the Save Import Settings button Addresses CodeRabbit review comments on PR #84.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in IPAM auto-create feature so missing LibreNMS-known IPs become unassigned global /32 (IPv4) or /128 (IPv6) IPAM records on import, OOB attach, and promote-to-host actions. It also adds a PlatformMapping creation flow from the import validation modal (mirroring the existing AddDeviceTypeMappingView), refactors the inline DeviceType typeahead into a reusable include, and adds Module Sync / Mapping Rules documentation.
Changes:
- New
auto_create_ipam_defaultsetting (model + migration + form + settings UI + per-request toggle + user preference) feeding newip_helpers.get_or_create_global_ip/auto_create_ipam_enabled, wired into both Device and VM import paths with an aggregated post-import toast. - New
AddPlatformMappingView(URL + view export + reusable_platform_mapping_form.html) and extraction of the existing DeviceType mapping form into_dt_mapping_form.html. - Fix in
librenms_import.jsouter-modal dismiss handler so nested-modal dismiss buttons aren't intercepted; new docs for Module Sync and Mapping Rules.
Reviewed changes
Copilot reviewed 26 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| netbox_librenms_plugin/models.py | Adds auto_create_ipam_default field on LibreNMSSettings. |
| netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py | Migration for new field. |
| netbox_librenms_plugin/forms.py | Adds auto_create_ipam_default to ImportSettingsForm. |
| netbox_librenms_plugin/utils.py | New resolve_auto_create_ipam with POST/GET/user-pref/settings cascade. |
| netbox_librenms_plugin/import_utils/ip_helpers.py | New module: get_or_create_global_ip and auto_create_ipam_enabled. |
| netbox_librenms_plugin/import_utils/init.py | Re-exports new IP helpers. |
| netbox_librenms_plugin/import_utils/device_operations.py | Calls IP helper after device save, tracks created_ips. |
| netbox_librenms_plugin/import_utils/bulk_import.py | Propagates created_ips into success entries. |
| netbox_librenms_plugin/import_utils/vm_operations.py | Calls IP helper after VM save, stores transient _librenms_created_ips. |
| netbox_librenms_plugin/views/imports/list.py | Resolves and exposes auto_create_ipam in context. |
| netbox_librenms_plugin/views/imports/actions.py | Adds IPAM resolution + aggregated toast in BulkImportDevicesView; adds AddPlatformMappingView. |
| netbox_librenms_plugin/views/init.py | Exports AddPlatformMappingView. |
| netbox_librenms_plugin/urls.py | Registers add_platform_mapping URL. |
| netbox_librenms_plugin/templates/.../settings.html | Renders the new auto-create-IPAM toggle + JS change tracking. |
| netbox_librenms_plugin/templates/.../librenms_import.html | Wraps toggles in <span> with hidden-off inputs; adds IPAM toggle; updates hx-include. |
| netbox_librenms_plugin/templates/.../htmx/device_validation_details.html | Replaces inline DT form/JS with _dt_mapping_form.html include; adds platform mapping include. |
| netbox_librenms_plugin/templates/.../htmx/_dt_mapping_form.html | Reusable DeviceType mapping form (with optional preselect). |
| netbox_librenms_plugin/templates/.../htmx/_platform_mapping_form.html | Reusable Platform mapping form. |
| netbox_librenms_plugin/static/.../librenms_import.js | Switches toggle handlers to new checkbox IDs; restricts outer-modal dismiss to top-level modal. |
| netbox_librenms_plugin/tests/test_ip_helpers.py | New tests for IP helper behaviors (mocked DB). |
| docs/* | New Module Sync / Mapping Rules guides and cross-links. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
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)
355-368:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose the platform-mapping partial in the existing-device no-match branches too.
Right now
_platform_mapping_form.htmlis only included in the final "new import/no platform" branch. If an existing device has no assigned platform or LibreNMS OS is unmapped, the earlier branches still stop at "Create Platform", so the new reusable mapping flow is unreachable from the validation modal for that path.🤖 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 355 - 368, The template currently only includes the partial "_platform_mapping_form.html" in the final "new import/no platform" branch, so the reusable platform-mapping flow is not reachable from the earlier existing-device/no-match branches; update the conditional logic around the "Create Platform" button (the blocks referencing libre_device.os and libre_device.os != "-" and the Create Platform button/hx-get URL) to also include the partial "_platform_mapping_form.html" in those branches (i.e., render {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" %} wherever the Create Platform button is shown) so the mapping form is exposed for existing-device no-match paths as well.
🤖 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/import_utils/device_operations.py`:
- Around line 913-918: The code incorrectly coerces
sync_options["auto_create_ipam"] with bare bool() which treats non-empty
false-like strings as True; update the logic around _opts/_auto_create so that
if "auto_create_ipam" is present you accept actual booleans or canonicalize
strings (e.g. lowercased) and treat "false", "off", "0", "" as False and
"true","on","1" as True (otherwise fall back to auto_create_ipam_enabled()),
then pass that boolean into get_or_create_global_ip(primary_ip,
auto_create=_auto_create).
In `@netbox_librenms_plugin/import_utils/vm_operations.py`:
- Line 198: Resolve the auto_create_ipam option once before iterating the VMs
instead of leaving auto_create_ipam_opt as None so create_vm_from_librenms()
repeatedly calls auto_create_ipam_enabled() for each VM; read
sync_options.get("auto_create_ipam") (or compute its boolean fallback once) and
pass that resolved boolean into each create_vm_from_librenms(...) invocation (or
set it on the per-batch context) so that auto_create_ipam_enabled() is not
called N times during the bulk import.
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html`:
- Around line 52-70: The input handler for searchEl doesn't advance requestSeq
when clearing or returning early for short queries, so in-flight fetches can
still populate dropdownEl with stale results; fix by incrementing requestSeq
(e.g., ++requestSeq) immediately when the input is cleared or when q.length < 2
(before returning) so any outstanding responses are ignored, and ensure
hiddenEl.value and submitBtn state updates remain as before in the
searchEl.addEventListener callback.
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html`:
- Around line 50-68: When the input is edited (searchEl input handler) and
q.length < 2 you must invalidate any in-flight lookups so stale responses cannot
re-open the dropdown; increment requestSeq (or otherwise mark/invalidate pending
requests) before returning and also ensure dropdownEl is cleared and submitBtn
disabled as already done. Specifically, in the input handler around the check
for q.length < 2 (where searchEl, timer, requestSeq, seq, dropdownEl, submitBtn
are used), advance requestSeq (e.g., ++requestSeq) when short-circuiting so any
outstanding fetch then ignores its response, and keep clearing dropdownEl and
disabling submitBtn.
In `@netbox_librenms_plugin/views/imports/actions.py`:
- Around line 759-776: The summary toast created by messages.info(...) (see
created_ips_all, unique_ips variables and the messages.info call) never appears
in the HTMX bulk-import path because the view returns only OOB row swaps plus
closeModal; update the view to explicitly include the created-IP message in the
HTMX response so it will render in that flow: after building unique_ips, instead
of only calling messages.info(request, ...), render the messages fragment (or
the same message text) into an OOB swap payload returned to the client (e.g.
render the messages partial/template to a string and attach it as an OOB swap or
include it in the JSON/HTML response that also contains closeModal), or push the
message into the same OOB response channel your closeModal uses so the client
will display it during the HTMX import; keep the existing messages.info call for
non-HTMX flows but ensure the HTMX path also returns the message content to be
swapped into the page.
In `@netbox_librenms_plugin/views/imports/list.py`:
- Around line 165-169: The code reads the persisted preference via get_user_pref
and settings_obj but skips the request-aware resolver, causing the import page
toggle to show stale state; import and call resolve_auto_create_ipam(request)
(alongside get_user_pref) and use its return value as the authoritative
_auto_create_ipam to ensure the UI reflects the effective request-driven setting
(fall back to existing get_user_pref/settings_obj logic only if
resolve_auto_create_ipam returns None or undefined).
---
Outside diff comments:
In
`@netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html`:
- Around line 355-368: The template currently only includes the partial
"_platform_mapping_form.html" in the final "new import/no platform" branch, so
the reusable platform-mapping flow is not reachable from the earlier
existing-device/no-match branches; update the conditional logic around the
"Create Platform" button (the blocks referencing libre_device.os and
libre_device.os != "-" and the Create Platform button/hx-get URL) to also
include the partial "_platform_mapping_form.html" in those branches (i.e.,
render {% include "netbox_librenms_plugin/htmx/_platform_mapping_form.html" %}
wherever the Create Platform button is shown) so the mapping form is exposed for
existing-device no-match paths as well.
🪄 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: 13b10ad0-34ad-4cf4-ad15-994802154482
⛔ 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 (26)
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/forms.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/models.pynetbox_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/settings.htmlnetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**/*.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/import_utils/bulk_import.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/utils.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/settings.htmlnetbox_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/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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/settings.htmlnetbox_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/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html
netbox_librenms_plugin/templates/**/settings.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/settings.html:settings.htmluses a split-form pattern with two separate Django forms (ServerConfigForm+ImportSettingsForm) sharing one page, differentiated by a hiddenform_typefield.
The settings page test-connection button should be an HTMX POST toTestLibreNMSConnectionView, returning an inline alert fragment.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.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
**/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/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
**/import_utils/__init__.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
import_utils/is a package; the__init__.pymust re-export key functions so callers can usefrom import_utils import ...for functions fromfilters.py,device_operations.py,vm_operations.py,cache.py,permissions.py, andvirtual_chassis.py
Files:
netbox_librenms_plugin/import_utils/__init__.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/list.pynetbox_librenms_plugin/views/imports/actions.py
**/views/imports/list.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/list.py: Import page (LibreNMSImportViewinviews/imports/list.py) supports two modes: (1) Synchronous — callsprocess_device_filters()directly, renders results inline. (2) Background — enqueuesFilterDevicesJob, returnsJsonResponsewithjob_id/job_pk/poll_url. Frontend polls and redirects to?job_id={pk}on completion
Result loading in import views:_load_job_results(job_id)readsjob.data["device_ids"], reconstructs devices from per-device cache usingget_validated_device_cache_key()
Files:
netbox_librenms_plugin/views/imports/list.py
**/import_utils/device_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
device_operations.pymust export:validate_device_for_import(device, ...)andbulk_import_devices_shared(devices, user, ...)
Files:
netbox_librenms_plugin/import_utils/device_operations.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
**/import_utils/vm_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
vm_operations.pymust export:bulk_import_vms(vm_imports, user, ...)
Files:
netbox_librenms_plugin/import_utils/vm_operations.py
🧠 Learnings (21)
📚 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/import_utils/bulk_import.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/utils.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/import_utils/bulk_import.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/utils.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/import_utils/bulk_import.pynetbox_librenms_plugin/models.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/utils.py
📚 Learning: 2026-03-08T14:17:28.826Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/import_utils/virtual_chassis.py:73-80
Timestamp: 2026-03-08T14:17:28.826Z
Learning: In Python code, when a lookup returns None (including API failures), implement negative caching by storing an empty result with a configurable TTL (default 5 minutes). Document the TTL and ensure a force_refresh=True bypasses the cache for manual re-fetch actions. Do not treat caching None/empty results on API failure as a bug if this mirrors existing patterns (e.g., get_device_with_server caching None on not-found). Apply this guidance to files within netbox_librenms_plugin/import_utils where similar inventory/API lookups occur.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-03-09T20:10:48.502Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/import_utils/device_operations.py:496-575
Timestamp: 2026-03-09T20:10:48.502Z
Learning: In netbox_librenms_plugin/import_utils/device_operations.py, in validate_device_for_import(), ensure both the cluster-required blocker (VM path) and the device_role-required blocker (device path) are guarded with if not result.get('existing_device') to ensure create-time prerequisites are only appended for new imports, not for link/update flows; keep available_roles and available_clusters populated for both new and existing-device cases so UI dropdowns function correctly on update views.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-05-22T19:37:46.167Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/tests/test_import_utils.py:1763-1766
Timestamp: 2026-05-22T19:37:46.167Z
Learning: In `validate_device_for_import` (device operations / serial-diff name-resolution logic), preserve the following result contract so the UI/test expectations remain stable:
- If `serial_action` is determined via a serial match AND the existing device has NO OOB/LibreNMS link, set `serial_action` to `"oob_candidate"` and ensure `promote_to_host` is NOT present in the returned dict.
- Populate `promote_to_host` only when the existing device already has an OOB/LibreNMS link (i.e., a host id is available to inherit from); otherwise omit the key.
- Always include `serial_role_choice_available` in the returned dict, defaulting to `False` (baseline) even when other resolution outcomes do not enable it.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.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/settings.htmlnetbox_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/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/settings.htmlnetbox_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/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/settings.htmlnetbox_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/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/list.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/list.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
📚 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_ip_helpers.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_ip_helpers.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_ip_helpers.py
📚 Learning: 2026-03-08T11:35:34.876Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 23
File: netbox_librenms_plugin/import_utils/vm_operations.py:61-69
Timestamp: 2026-03-08T11:35:34.876Z
Learning: In netbox_librenms_plugin/import_utils/vm_operations.py, do not flag the bare-int librenms_id in create_vm_from_librenms as a multi-server bug on the pr/librenms-pre-id-multi-server branch. This file is on a staging branch where per-server scoping migration (set_librenms_device_id(vm, device_id, server_key)) will occur in the next multi-server PR. Treat this as a branch-specific exception and plan to enforce the per-server convention only in the subsequent PR, not on this branch.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.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
🪛 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] 1911-1911: 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] 1918-1918: 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 (19)
docs/README.md (2)
25-34: LGTM!
42-43: LGTM!docs/feature_list.md (2)
6-11: LGTM!
13-32: LGTM!docs/librenms_import/validation.md (2)
25-28: LGTM!
33-33: LGTM!docs/usage_tips/README.md (1)
14-27: LGTM!docs/usage_tips/mapping_rules.md (4)
1-32: LGTM!
33-83: LGTM!
84-159: LGTM!
160-230: LGTM!docs/usage_tips/module_sync.md (1)
1-57: LGTM!netbox_librenms_plugin/urls.py (1)
433-437: LGTM!netbox_librenms_plugin/views/__init__.py (1)
113-113: LGTM!netbox_librenms_plugin/import_utils/ip_helpers.py (1)
29-111: LGTM!netbox_librenms_plugin/import_utils/__init__.py (1)
53-54: LGTM!netbox_librenms_plugin/tests/test_ip_helpers.py (1)
10-174: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html (1)
190-200: LGTM!Also applies to: 314-348
netbox_librenms_plugin/utils.py (1)
366-416: LGTM!
- device_operations.py: fix bool() coercion on auto_create_ipam option;
string 'false' now correctly evaluates to False (CodeRabbit #3328922289)
- vm_operations.py: remove 'yes' from truthy set to align with canonical
{1,true,on}; resolve auto_create_ipam/use_sysname/strip_domain options
once before the VM loop instead of per-iteration (CodeRabbit #3328922290)
- list.py: replace 5-line manual pref cascade with resolve_auto_create_ipam()
(CodeRabbit #3328922298, Copilot #3328732877)
- actions.py: render auto-created IPAM entries as OOB toast in HTMX bulk-
import path; the messages queue is consumed by device_import_row.html
before reaching the browser (CodeRabbit #3328922296)
- _dt_mapping_form.html / _platform_mapping_form.html: increment requestSeq
before the early-return guard so in-flight requests from a prior query are
always cancelled when user clears the field (CodeRabbit #3328922291/#3328922294)
There was a problem hiding this comment.
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/imports/actions.py (1)
754-777:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBatch result toasts are still dropped in the HTMX import flow.
This fixes the auto-created-IP toast, but
messages.success(),messages.error(), andmessages.warning()still never render here.device_import_row.htmlconsumes the Djangomessagesqueue into an empty OOB#django-messagescontainer, and whenimported_device_idsis empty this response is justcloseModalwith no visible feedback. Skipped/failed-only imports can therefore complete silently.Please send the success/failed/skipped summaries through the same explicit OOB channel as
ip_toast_msgfor HTMX requests instead of relying on the standardmessagesqueue in this branch.Also applies to: 789-872
🤖 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/imports/actions.py` around lines 754 - 777, The success/error/warning toasts are still being added to Django's messages queue (messages.success/messages.error/messages.warning) which gets swallowed by device_import_row.html in HTMX flows; change the branch where ip_toast_msg is constructed to also build explicit OOB toast payloads for the success/failed/skipped summaries and send them via the same OOB channel used for ip_toast_msg (the explicit OOB response fragment targeting `#django-messages`) when the request is an HTMX request; keep using messages.* only for non-HTMX flows. Locate the code that reads device_result and vm_result and the block that computes ip_toast_msg and extend it to aggregate success_count/skipped/failed summaries into strings, then emit those strings the same way you emit ip_toast_msg for HTMX responses (so device_import_row.html will render them), while preserving the existing messages.* calls for normal requests.
♻️ Duplicate comments (1)
netbox_librenms_plugin/import_utils/vm_operations.py (1)
174-177:⚠️ Potential issue | 🟠 MajorResolve
auto_create_ipamto a boolean before entering the VM loop.
auto_create_ipam_optstill staysNonewhenbulk_import_vms()is called with its defaultsync_options=Noneor without that key, so everycreate_vm_from_librenms()call falls back toauto_create_ipam_enabled()and re-reads plugin settings once per VM. That keeps the N-query behavior on the default bulk path.Suggested fix
use_sysname_opt = sync_options.get("use_sysname", True) if sync_options else True strip_domain_opt = sync_options.get("strip_domain", False) if sync_options else False auto_create_ipam_opt = sync_options.get("auto_create_ipam") if sync_options else None + if auto_create_ipam_opt is None: + from .ip_helpers import auto_create_ipam_enabled + + auto_create_ipam_opt = auto_create_ipam_enabled() + elif isinstance(auto_create_ipam_opt, str): + auto_create_ipam_opt = auto_create_ipam_opt.strip().lower() in {"1", "true", "on"} + else: + auto_create_ipam_opt = bool(auto_create_ipam_opt)🤖 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/import_utils/vm_operations.py` around lines 174 - 177, Resolve auto_create_ipam to a boolean once before the VM loop: if sync_options includes the "auto_create_ipam" key, convert that value to a boolean and assign it to auto_create_ipam_opt; otherwise call auto_create_ipam_enabled() once and store its boolean result in auto_create_ipam_opt so create_vm_from_librenms() doesn't re-query settings per-VM (refer to bulk_import_vms(), auto_create_ipam_opt, create_vm_from_librenms(), and auto_create_ipam_enabled()).
🤖 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.
Outside diff comments:
In `@netbox_librenms_plugin/views/imports/actions.py`:
- Around line 754-777: The success/error/warning toasts are still being added to
Django's messages queue (messages.success/messages.error/messages.warning) which
gets swallowed by device_import_row.html in HTMX flows; change the branch where
ip_toast_msg is constructed to also build explicit OOB toast payloads for the
success/failed/skipped summaries and send them via the same OOB channel used for
ip_toast_msg (the explicit OOB response fragment targeting `#django-messages`)
when the request is an HTMX request; keep using messages.* only for non-HTMX
flows. Locate the code that reads device_result and vm_result and the block that
computes ip_toast_msg and extend it to aggregate success_count/skipped/failed
summaries into strings, then emit those strings the same way you emit
ip_toast_msg for HTMX responses (so device_import_row.html will render them),
while preserving the existing messages.* calls for normal requests.
---
Duplicate comments:
In `@netbox_librenms_plugin/import_utils/vm_operations.py`:
- Around line 174-177: Resolve auto_create_ipam to a boolean once before the VM
loop: if sync_options includes the "auto_create_ipam" key, convert that value to
a boolean and assign it to auto_create_ipam_opt; otherwise call
auto_create_ipam_enabled() once and store its boolean result in
auto_create_ipam_opt so create_vm_from_librenms() doesn't re-query settings
per-VM (refer to bulk_import_vms(), auto_create_ipam_opt,
create_vm_from_librenms(), and auto_create_ipam_enabled()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89ad8fcd-e067-45cc-81e2-7241facf669a
📒 Files selected for processing (6)
netbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.htmlnetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.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.14)
- GitHub Check: test-netbox (3.13)
- GitHub Check: test-netbox (3.12)
🧰 Additional context used
📓 Path-based instructions (10)
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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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.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/list.pynetbox_librenms_plugin/views/imports/actions.py
**/views/imports/list.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/list.py: Import page (LibreNMSImportViewinviews/imports/list.py) supports two modes: (1) Synchronous — callsprocess_device_filters()directly, renders results inline. (2) Background — enqueuesFilterDevicesJob, returnsJsonResponsewithjob_id/job_pk/poll_url. Frontend polls and redirects to?job_id={pk}on completion
Result loading in import views:_load_job_results(job_id)readsjob.data["device_ids"], reconstructs devices from per-device cache usingget_validated_device_cache_key()
Files:
netbox_librenms_plugin/views/imports/list.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/list.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/views/imports/actions.py
**/import_utils/device_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
device_operations.pymust export:validate_device_for_import(device, ...)andbulk_import_devices_shared(devices, user, ...)
Files:
netbox_librenms_plugin/import_utils/device_operations.py
**/import_utils/vm_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
vm_operations.pymust export:bulk_import_vms(vm_imports, user, ...)
Files:
netbox_librenms_plugin/import_utils/vm_operations.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 (14)
📚 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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.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.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/list.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.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/imports/list.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.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/imports/list.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.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/imports/list.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/imports/list.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T14:17:28.826Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/import_utils/virtual_chassis.py:73-80
Timestamp: 2026-03-08T14:17:28.826Z
Learning: In Python code, when a lookup returns None (including API failures), implement negative caching by storing an empty result with a configurable TTL (default 5 minutes). Document the TTL and ensure a force_refresh=True bypasses the cache for manual re-fetch actions. Do not treat caching None/empty results on API failure as a bug if this mirrors existing patterns (e.g., get_device_with_server caching None on not-found). Apply this guidance to files within netbox_librenms_plugin/import_utils where similar inventory/API lookups occur.
Applied to files:
netbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-03-09T20:10:48.502Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/import_utils/device_operations.py:496-575
Timestamp: 2026-03-09T20:10:48.502Z
Learning: In netbox_librenms_plugin/import_utils/device_operations.py, in validate_device_for_import(), ensure both the cluster-required blocker (VM path) and the device_role-required blocker (device path) are guarded with if not result.get('existing_device') to ensure create-time prerequisites are only appended for new imports, not for link/update flows; keep available_roles and available_clusters populated for both new and existing-device cases so UI dropdowns function correctly on update views.
Applied to files:
netbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-05-22T19:37:46.167Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/tests/test_import_utils.py:1763-1766
Timestamp: 2026-05-22T19:37:46.167Z
Learning: In `validate_device_for_import` (device operations / serial-diff name-resolution logic), preserve the following result contract so the UI/test expectations remain stable:
- If `serial_action` is determined via a serial match AND the existing device has NO OOB/LibreNMS link, set `serial_action` to `"oob_candidate"` and ensure `promote_to_host` is NOT present in the returned dict.
- Populate `promote_to_host` only when the existing device already has an OOB/LibreNMS link (i.e., a host id is available to inherit from); otherwise omit the key.
- Always include `serial_role_choice_available` in the returned dict, defaulting to `False` (baseline) even when other resolution outcomes do not enable it.
Applied to files:
netbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-03-08T11:35:34.876Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 23
File: netbox_librenms_plugin/import_utils/vm_operations.py:61-69
Timestamp: 2026-03-08T11:35:34.876Z
Learning: In netbox_librenms_plugin/import_utils/vm_operations.py, do not flag the bare-int librenms_id in create_vm_from_librenms as a multi-server bug on the pr/librenms-pre-id-multi-server branch. This file is on a staging branch where per-server scoping migration (set_librenms_device_id(vm, device_id, server_key)) will occur in the next multi-server PR. Treat this as a branch-specific exception and plan to enforce the per-server convention only in the subsequent PR, not on this branch.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.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
🔇 Additional comments (5)
netbox_librenms_plugin/import_utils/device_operations.py (1)
915-916: LGTM!netbox_librenms_plugin/import_utils/vm_operations.py (1)
97-103: LGTM!netbox_librenms_plugin/views/imports/list.py (1)
18-18: LGTM!Also applies to: 165-165
netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
55-56: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html (1)
57-58: LGTM!
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
netbox_librenms_plugin/views/imports/actions.py (1)
752-780:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMirror earlier queued messages into the HTMX OOB toast payload.
htmx_toastsonly captures the summaries assembled in this block. Anymessages.*added earlier in the request is still lost on the HTMX path because the row renders consume Django’s message storage before the browser sees it. The concrete regression here is the fallback warning at Line 685 when background import was requested but no RQ workers are available: HTMX users never see that the import actually ran synchronously.Either append those earlier warnings/errors to the same HTMX toast list at the call site, or render the remaining message storage into the final OOB fragment after the row renders.
Also applies to: 849-880
🤖 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/mapping_rules.md`:
- Around line 126-152: The docs claim `device_type` normalization is active but
the import path still passes the raw LibreNMS hardware string into
match_librenms_hardware_to_device_type() which never calls
apply_normalization_rules(); either update the documentation to state
`device_type` normalization is not yet applied, or modify the import code so the
LibreNMS hardware string is first passed through apply_normalization_rules()
(using the same normalization rule structures as
ModuleTypeMapping/ModuleBayMapping) before calling
match_librenms_hardware_to_device_type(), and ensure the DeviceTypeMapping
lookup uses that normalized value.
In `@netbox_librenms_plugin/import_utils/ip_helpers.py`:
- Around line 93-104: The IntegrityError recovery block in
get_or_create_global_ip currently runs inside the outer transaction and can mark
it as rollback-only; wrap the race-prone create call in a nested savepoint by
using Django's transaction.atomic() (savepoint) around
IPAddress.objects.create(address=f"{ip_str}{mask}", status="active") so that the
IntegrityError is contained, then catch IntegrityError and re-query with
IPAddress.objects.filter(address__net_host=ip_str, vrf__isnull=True).first() as
before; ensure transaction is imported (from django.db import transaction) and
only the create is inside the inner atomic block so the outer transaction is not
poisoned.
🪄 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: 4d6cb80f-4656-4505-ad42-cc4af10cc68c
⛔ 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 (26)
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/forms.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/models.pynetbox_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/settings.htmlnetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/views/__init__.pynetbox_librenms_plugin/views/imports/actions.pynetbox_librenms_plugin/views/imports/list.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.jsnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
netbox_librenms_plugin/templates/**/settings.html
📄 CodeRabbit inference engine (.github/instructions/frontend.instructions.md)
netbox_librenms_plugin/templates/**/settings.html:settings.htmluses a split-form pattern with two separate Django forms (ServerConfigForm+ImportSettingsForm) sharing one page, differentiated by a hiddenform_typefield.
The settings page test-connection button should be an HTMX POST toTestLibreNMSConnectionView, returning an inline alert fragment.
Files:
netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.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/models.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tests/test_ip_helpers.pynetbox_librenms_plugin/views/imports/actions.py
**/import_utils/__init__.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
import_utils/is a package; the__init__.pymust re-export key functions so callers can usefrom import_utils import ...for functions fromfilters.py,device_operations.py,vm_operations.py,cache.py,permissions.py, andvirtual_chassis.py
Files:
netbox_librenms_plugin/import_utils/__init__.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/list.pynetbox_librenms_plugin/views/imports/actions.py
**/views/imports/list.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
**/views/imports/list.py: Import page (LibreNMSImportViewinviews/imports/list.py) supports two modes: (1) Synchronous — callsprocess_device_filters()directly, renders results inline. (2) Background — enqueuesFilterDevicesJob, returnsJsonResponsewithjob_id/job_pk/poll_url. Frontend polls and redirects to?job_id={pk}on completion
Result loading in import views:_load_job_results(job_id)readsjob.data["device_ids"], reconstructs devices from per-device cache usingget_validated_device_cache_key()
Files:
netbox_librenms_plugin/views/imports/list.py
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/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/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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
**/import_utils/device_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
device_operations.pymust export:validate_device_for_import(device, ...)andbulk_import_devices_shared(devices, user, ...)
Files:
netbox_librenms_plugin/import_utils/device_operations.py
**/import_utils/vm_operations.py
📄 CodeRabbit inference engine (.github/instructions/background-jobs.instructions.md)
vm_operations.pymust export:bulk_import_vms(vm_imports, user, ...)
Files:
netbox_librenms_plugin/import_utils/vm_operations.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 (21)
📚 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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/settings.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.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/models.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tests/test_ip_helpers.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/models.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tests/test_ip_helpers.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/models.pynetbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/views/imports/list.pynetbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.pynetbox_librenms_plugin/forms.pynetbox_librenms_plugin/utils.pynetbox_librenms_plugin/urls.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.pynetbox_librenms_plugin/tests/test_ip_helpers.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/list.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/list.pynetbox_librenms_plugin/views/imports/actions.py
📚 Learning: 2026-03-08T14:17:28.826Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 22
File: netbox_librenms_plugin/import_utils/virtual_chassis.py:73-80
Timestamp: 2026-03-08T14:17:28.826Z
Learning: In Python code, when a lookup returns None (including API failures), implement negative caching by storing an empty result with a configurable TTL (default 5 minutes). Document the TTL and ensure a force_refresh=True bypasses the cache for manual re-fetch actions. Do not treat caching None/empty results on API failure as a bug if this mirrors existing patterns (e.g., get_device_with_server caching None on not-found). Apply this guidance to files within netbox_librenms_plugin/import_utils where similar inventory/API lookups occur.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-03-09T20:10:48.502Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 25
File: netbox_librenms_plugin/import_utils/device_operations.py:496-575
Timestamp: 2026-03-09T20:10:48.502Z
Learning: In netbox_librenms_plugin/import_utils/device_operations.py, in validate_device_for_import(), ensure both the cluster-required blocker (VM path) and the device_role-required blocker (device path) are guarded with if not result.get('existing_device') to ensure create-time prerequisites are only appended for new imports, not for link/update flows; keep available_roles and available_clusters populated for both new and existing-device cases so UI dropdowns function correctly on update views.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.py
📚 Learning: 2026-05-22T19:37:46.167Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 79
File: netbox_librenms_plugin/tests/test_import_utils.py:1763-1766
Timestamp: 2026-05-22T19:37:46.167Z
Learning: In `validate_device_for_import` (device operations / serial-diff name-resolution logic), preserve the following result contract so the UI/test expectations remain stable:
- If `serial_action` is determined via a serial match AND the existing device has NO OOB/LibreNMS link, set `serial_action` to `"oob_candidate"` and ensure `promote_to_host` is NOT present in the returned dict.
- Populate `promote_to_host` only when the existing device already has an OOB/LibreNMS link (i.e., a host id is available to inherit from); otherwise omit the key.
- Always include `serial_role_choice_available` in the returned dict, defaulting to `False` (baseline) even when other resolution outcomes do not enable it.
Applied to files:
netbox_librenms_plugin/import_utils/bulk_import.pynetbox_librenms_plugin/import_utils/__init__.pynetbox_librenms_plugin/import_utils/ip_helpers.pynetbox_librenms_plugin/import_utils/device_operations.pynetbox_librenms_plugin/import_utils/vm_operations.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/device_validation_details.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.htmlnetbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html
📚 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-08T11:35:34.876Z
Learnt from: marcinpsk
Repo: marcinpsk/netbox-librenms-plugin PR: 23
File: netbox_librenms_plugin/import_utils/vm_operations.py:61-69
Timestamp: 2026-03-08T11:35:34.876Z
Learning: In netbox_librenms_plugin/import_utils/vm_operations.py, do not flag the bare-int librenms_id in create_vm_from_librenms as a multi-server bug on the pr/librenms-pre-id-multi-server branch. This file is on a staging branch where per-server scoping migration (set_librenms_device_id(vm, device_id, server_key)) will occur in the next multi-server PR. Treat this as a branch-specific exception and plan to enforce the per-server convention only in the subsequent PR, not on this branch.
Applied to files:
netbox_librenms_plugin/import_utils/vm_operations.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_ip_helpers.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_ip_helpers.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_ip_helpers.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/_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)
🪛 OpenGrep (1.22.0)
netbox_librenms_plugin/views/imports/actions.py
[WARNING] 853-872: 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] 1944-1944: 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] 1951-1951: 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 (25)
docs/usage_tips/README.md (1)
14-18: LGTM!Also applies to: 23-27
netbox_librenms_plugin/templates/netbox_librenms_plugin/settings.html (1)
190-200: LGTM!Also applies to: 314-315, 321-322, 336-338, 348-349
docs/usage_tips/module_sync.md (1)
1-57: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/librenms_import.html (1)
351-358: LGTM!Also applies to: 362-370, 372-382, 388-388
netbox_librenms_plugin/views/__init__.py (1)
113-113: LGTM!docs/librenms_import/validation.md (1)
25-27: LGTM!Also applies to: 33-33
netbox_librenms_plugin/models.py (1)
82-90: LGTM!netbox_librenms_plugin/import_utils/bulk_import.py (1)
203-203: LGTM!docs/feature_list.md (1)
6-32: LGTM!netbox_librenms_plugin/import_utils/__init__.py (1)
53-54: LGTM!netbox_librenms_plugin/views/imports/list.py (1)
18-18: LGTM!Also applies to: 165-165, 169-169, 355-355
netbox_librenms_plugin/static/netbox_librenms_plugin/js/librenms_import.js (2)
352-357: LGTM!
1204-1213: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/device_validation_details.html (1)
178-180: LGTM!Also applies to: 195-200, 213-213, 366-371
docs/README.md (1)
25-44: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_dt_mapping_form.html (1)
1-113: LGTM!netbox_librenms_plugin/migrations/0011_librenmssettings_auto_create_ipam_default.py (1)
1-23: LGTM!netbox_librenms_plugin/forms.py (1)
165-183: LGTM!netbox_librenms_plugin/utils.py (1)
366-416: LGTM!netbox_librenms_plugin/templates/netbox_librenms_plugin/htmx/_platform_mapping_form.html (1)
10-39: LGTM!Also applies to: 40-111
netbox_librenms_plugin/urls.py (1)
16-17: LGTM!Also applies to: 433-437
netbox_librenms_plugin/import_utils/device_operations.py (1)
758-770: LGTM!Also applies to: 786-845, 946-963
netbox_librenms_plugin/import_utils/vm_operations.py (1)
25-25: LGTM!Also applies to: 174-183, 271-271
netbox_librenms_plugin/tests/test_ip_helpers.py (1)
10-174: LGTM!netbox_librenms_plugin/views/imports/actions.py (1)
1853-1955: LGTM!
|
@coderabbitai full review |
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.
- Add auto_create_ipam option to device/VM import to automatically create IP addresses in NetBox from LibreNMS data - Fix auto_create_ipam bool coercion and dirty-state wiring - Address PR #84 review comments: resolve auto_create_ipam once before VM loop instead of per-VM query; surface all HTMX-path toasts via OOB swap instead of Django messages queue - Fix UnboundLocalError caused by inner mark_safe import shadowing
… outer transaction IntegrityError caught without a nested savepoint leaves the outer transaction.atomic() marked as needing rollback, causing TransactionManagementError on any subsequent ORM query in the same block. Wrapping create() in transaction.atomic() creates a savepoint so the IntegrityError is absorbed without affecting the outer transaction. Also update docs to remove device_type normalization scope from mapping_rules.md since apply_normalization_rules() is not yet called by match_librenms_hardware_to_device_type(). Deferred to issue #90.
…k result AddPlatformMappingView now fails closed (error response) when more than one PlatformMapping row exists for the same librenms_os string, both before the permission check (fast path) and inside the transaction.atomic() block after SELECT FOR UPDATE (to catch concurrent duplicates). bulk_import_vms now includes 'created_ips' in each success entry, populated from vm._librenms_created_ips, mirroring bulk_import_devices_shared so callers receive consistent IPAM creation feedback for VMs.
…stale-lookup guards, simplify auto_create_ipam coercion - Add hidden server_key input to device-type and platform mapping forms so multi-server POSTs resolve against the correct LibreNMS API/cache namespace (AddDeviceTypeMappingView/AddPlatformMappingView read request.POST[server_key]) - Guard the typeahead .catch handlers with seq !== requestSeq so a stale failed lookup can't hide the dropdown from a newer successful one - Drop redundant string truthy-parsing of auto_create_ipam in bulk_import_vms and import_single_device; the flag is already normalised to bool by resolve_auto_create_ipam() at the request boundary
Drops the unassigned-global-/32 pre-create behaviour from device/VM import. It added busy import side-effects, was stuck in the default VRF, and never actually set primary_ip (NetBox requires the IP be interface-assigned), so it only parked records for manual wiring. Replaced (next commit) by Primary-IP auto-match on the IP-sync tab. Removed: ip_helpers.py + tests, migration 0011 + the auto_create_ipam_default model/form field, resolve_auto_create_ipam, the import-utils re-export; the pre-create blocks + created_ips plumbing in device_operations/vm_operations/ bulk_import; the import-page and settings-page toggles + their JS; and Kept: PR #84 server_key inputs + .catch stale-lookup guards, the PlatformMapping duplicate guard, and the htmx_toasts success/failed/skipped mechanism. oob-sync still references the removed helper for OOB/promote; that is reworked to interface-assigned IPs in a later commit on feat/oob-sync.
Replaces the removed import-time auto-create with an interface-assigned model. When the 'Set as Primary IP' toggle is on, SyncIPAddressesView matches the synced IP against the device/VM's LibreNMS management IP (from get_device_info) and, if that IP ends up assigned to one of the object's interfaces, sets it as primary_ip4/primary_ip6. This satisfies NetBox's interface-assignment constraint for primary_ip and creates no unassigned global records. - utils.resolve_set_primary_ip: POST/GET toggle -> user pref -> False cascade - SaveUserPrefView: allow the set_primary_ip pref - SyncIPAddressesView: get_management_ip / _same_host / _set_primary_ip helpers; process_ip_sync sets primary + reports it; display_sync_results surfaces it - IP-sync tab: 'Set as Primary IP' toggle, pre-checked from pref, persisted via save_user_pref - tests for the cascade and the process_ip_sync primary-set behaviour
…s on Flags the IP-sync row whose IP equals the device/VM's LibreNMS management IP (enrich_ip_data -> data-mgmt-ip row attribute) and auto-ticks its select box when the 'Set Primary IP' toggle is enabled, so syncing it sets the Primary IP in one action. Mirrors the parent-child row-auto-select construct (data-* attribute + JS), keyed off the LibreNMS-mapped interface. - utils.same_host: shared host-equality helper (SyncIPAddressesView._same_host now delegates to it) - BaseIPAddressTableView._flag_management_ip: marks the matching enriched entry - IPAddressTable: data-mgmt-ip row attribute - _ipaddress_sync_content.html: toggle JS now also auto-checks the mgmt row - tests for _flag_management_ip
…erface
- Wrap each IP in its own savepoint so one bad address rolls back only itself
instead of poisoning the whole batch; log the exception and include the
reason in the failure toast (previously a bare 'Failed: <ip>').
- When 'Set Primary IP' is on and the management IP has no NetBox interface,
emit a clear warning ('Sync interfaces first, then re-run') instead of
silently skipping the primary assignment.
…e pref A 403/500 from save_user_pref was silently swallowed (fetch only rejects on network errors), so the toggle looked persisted when it wasn't. Addresses CodeRabbit review on PR #84.
filter(address=...) alone can match the same address in a different VRF and then rewrite its VRF on save, hijacking an unrelated IP. Scope the lookup to the selected VRF so a new IP is created in the target VRF instead. Addresses PR #79 CodeRabbit.
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
Documentation
New Features
UI / UX
Tests