Better UI for Device Group admin (v1) #395
Conversation
feat: create+select for subject-type feat: fixed up Subject Type autocomplete. feat: update search feature in destinations search. feat: consolidate Device Group page. feat: navigation from inbound to device group feat: disable changing the default device group for an inbound integration. feat: add search to bridge integration list. feat: tabbed view on Device Group page.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces significant enhancements to device integration and management forms, focusing on improved search capabilities, user-friendly widgets, and consolidated device group management. The changes streamline the user experience with advanced selection interfaces and more intuitive navigation.
Key changes include:
- Advanced searchable multiselect widgets for destinations and devices with real-time filtering
- Subject type autocomplete field with create-new functionality and API integration
- Consolidated device group management with tabbed interface and improved navigation flow
- Enhanced search filters across device and bridge integration lists
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| cdip_admin/integrations/widgets.py | Implements custom widgets including searchable multiselect, subject type autocomplete, and device group selection widgets |
| cdip_admin/integrations/views.py | Adds device group management views, subject type creation API, and updates existing views with enhanced form handling |
| cdip_admin/integrations/forms.py | Integrates new custom widgets into device group and integration forms with improved field configurations |
| cdip_admin/integrations/filters.py | Adds comprehensive search functionality to device and bridge integration filters |
| cdip_admin/integrations/urls.py | Updates URL patterns for new device group management views and API endpoints |
| cdip_admin/integrations/templates/ | Updates templates with tabbed interfaces, improved layouts, and enhanced device management views |
| cdip_admin/integrations/static/integrations/js/ | Adds JavaScript implementations for custom widgets and interaction handling |
Comments suppressed due to low confidence (1)
cdip_admin/integrations/widgets.py:1
- This condition uses truthiness check which will fail if type is '0' or other falsy values. Use
is not Noneor!= ''for more reliable comparison.
from django import forms
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
chrisdoehring
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: chrisdoehring <chrisdoehring@gmail.com>
…to gundi-4725-v1-devicegroups
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| "search", | ||
| ) | ||
|
|
||
| def filter_search(self, queryset, name, value): |
There was a problem hiding this comment.
Alternative implementation: Use rest_framework.DjangoFilterBackend + SearchFilter and set a list of search fields at the view level (more examples in the v2 API views)
|
|
||
| def get_unique_together_validators(self): | ||
| # Overriden to disable unique together check as it's handled in the create method | ||
| # Overridden to disable unique together check as it's handled in the create method |
There was a problem hiding this comment.
Thank you for fixing those
| import json | ||
| from django.utils.safestring import mark_safe | ||
|
|
||
| class SearchableMultiSelectWidget(forms.Widget): |
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| def form_valid(self, form): | ||
| form.save() | ||
| return redirect("device_group", device_group_id=str(self.object.id)) |
There was a problem hiding this comment.
Bug: URL Reversal Errors Due to Positional Arguments
Several URL reversals, including redirect calls in DeviceGroupAddView and {% url %} tags in templates, are passing arguments positionally. This causes NoReverseMatch errors because the URL patterns expect device_group_id as a keyword argument.
I added some UI friendliness for navigating between Inbound Integrations, Device Groups and Outbound Destinations.
This is primarily to prevent errors like what's described in https://allenai.atlassian.net/browse/GUNDI-4725. This is where a user probably visited the Device Groups page and accidentally un-selected the desired outbound connection.
feat: create+select for subject-type
feat: fixed up Subject Type autocomplete.
feat: update search feature in destinations search.
feat: consolidate Device Group page.
feat: navigation from inbound to device group
feat: disable changing the default device group for an inbound integration.
feat: add search to bridge integration list.
feat: tabbed view on Device Group page.
Copilot's summary
This pull request introduces significant enhancements to the device integration and management forms, focusing on improved search/filter capabilities and more user-friendly widgets. It also includes minor corrections to documentation and help texts for clarity. The most important changes are grouped below.
Search and Filtering Improvements:
searchfilter to bothDeviceFilterandBridgeIntegrationFilter, allowing users to search across multiple related fields for devices and integrations, improving usability in large datasets. [1] [2]Form and Widget Enhancements:
SearchableMultiSelectField,SubjectTypeAutocompleteField, etc.) into forms for device groups and integrations, enabling advanced selection and autocomplete features for destinations and subject types. [1] [2] [3]InboundIntegrationConfigurationFormand related forms to provide context-aware filtering and display logic for device groups, including display-only widgets for existing integrations and auto-create messaging for new ones. [1] [2] [3]Documentation and Help Text Updates:
README.mddocumenting the searchable multiselect widget, its features, usage, and dependencies for frontend developers.Note
Adds searchable multi-select and autocomplete widgets, consolidates and enhances Device Group management (including new devices management view), expands search/filters, and wires supporting endpoints/routes and UI updates.
SearchableMultiSelectField,DeviceSearchableMultiSelectField,SubjectTypeAutocompleteField,DeviceGroupSelectWithLinkField,DeviceGroupAutoCreateField,DeviceGroupDisplayWidgetwith JS (searchable-multiselect.js,device-searchable-multiselect.js,subject-type-autocomplete.js).InboundIntegrationConfigurationAddFormand enhanceInboundIntegrationConfigurationForm(permission-aware device group querying, display-only default device group on edit, preserve on save).device_group_detail.html.DeviceGroupManagementFormand newDeviceGroupDevicesManagementForm+ page for selecting devices tied to the integration.devicesandbridge_integration_list.devicegroups/<id>toDeviceGroupManagementUpdateView; adddevicegroups/<id>/devices/manage(DeviceGroupDevicesManagementView).inboundconfigurations/add_type_modal,add_schema,add_dropdown_restore.requestto forms; add JS warning handler for type changes.DeviceFilter: owner field path fix (inbound_configuration__owner__name) and add full-textsearchacross device/config/owner/type/group.BridgeIntegrationFilter: addsearchacross owner/name/state/additional; update templates to expose it.additionalinstead ofstatewhen loading initial JSON.POST /integrations/api/create-subject-type/to createSubjectTypefor autocomplete.Written by Cursor Bugbot for commit 7e48d1c. This will update automatically on new commits. Configure here.