Conversation
|
Review changes with Changed Files
|
lucaspar
added
gateway
There was a problem hiding this comment.
Pull request overview
This pull request implements a "Quick Add Capture(s) to Dataset" feature that allows users to add one or multiple captures to a dataset through two workflows: (1) a selection mode in the captures table where users can select multiple captures via checkboxes, and (2) planned support for single-capture quick-add from dropdown menus (though the UI for this is not yet implemented in the templates).
Changes:
- Added backend API endpoints for quick-adding captures to datasets and retrieving eligible datasets
- Implemented frontend selection mode with checkbox-based multi-select in the captures table
- Created a modal interface for selecting the target dataset and confirming the operation
- Added support for automatic multi-channel capture grouping during addition
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| gateway/sds_gateway/users/views.py | Added QuickAddCapturesToDatasetView for adding captures and UserDatasetsForQuickAddView for listing eligible datasets with proper permission checks |
| gateway/sds_gateway/users/urls.py | Registered two new URL patterns for the quick-add API endpoints |
| gateway/sds_gateway/templates/users/partials/quick_add_to_dataset_modal.html | New modal template for dataset selection with accessible form controls |
| gateway/sds_gateway/templates/users/partials/captures_page_table.html | Added selection column with checkboxes (hidden by default, shown in selection mode) |
| gateway/sds_gateway/templates/users/file_list.html | Integrated selection mode buttons and quick-add modal into the file list page |
| gateway/sds_gateway/static/js/file-list.js | Implemented selection mode toggle, checkbox state management, and row click handling |
| gateway/sds_gateway/static/js/components.js | Updated row click handler to ignore clicks on selection checkboxes |
| gateway/sds_gateway/static/js/actions/QuickAddToDatasetManager.js | New manager class handling modal interactions, dataset loading, and API calls for single/multi capture addition |
| gateway/sds_gateway/static/css/file-list.css | Added CSS rules to show/hide selection column based on selection mode state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gateway/sds_gateway/static/js/actions/QuickAddToDatasetManager.js
Outdated
Show resolved
Hide resolved
gateway/sds_gateway/templates/users/partials/captures_page_table.html
Outdated
Show resolved
Hide resolved
gateway/sds_gateway/static/js/actions/QuickAddToDatasetManager.js
Outdated
Show resolved
Hide resolved
Changes to be committed: modified: gateway/sds_gateway/static/css/file-list.css new file: gateway/sds_gateway/static/js/actions/QuickAddToDatasetManager.js modified: gateway/sds_gateway/static/js/components.js modified: gateway/sds_gateway/static/js/file-list.js modified: gateway/sds_gateway/templates/users/file_list.html modified: gateway/sds_gateway/templates/users/partials/captures_page_table.html new file: gateway/sds_gateway/templates/users/partials/quick_add_to_dataset_modal.html modified: gateway/sds_gateway/users/urls.py modified: gateway/sds_gateway/users/views.py Changes to be committed: modified: gateway/sds_gateway/static/css/file-list.css new file: gateway/sds_gateway/static/js/actions/QuickAddToDatasetManager.js modified: gateway/sds_gateway/static/js/components.js modified: gateway/sds_gateway/static/js/file-list.js modified: gateway/sds_gateway/templates/users/file_list.html modified: gateway/sds_gateway/templates/users/partials/captures_page_table.html new file: gateway/sds_gateway/templates/users/partials/quick_add_to_dataset_modal.html modified: gateway/sds_gateway/users/urls.py modified: gateway/sds_gateway/users/views.py
LLKruczek
force-pushed
the
lk/quick-add-capture
branch
from
8c30df5 to
31278c8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| owned = ( | ||
| user.datasets.filter(is_deleted=False, is_public=False) | ||
| .order_by("name") | ||
| .values("uuid", "name") | ||
| ) | ||
| shared_uuids = list( | ||
| UserSharePermission.objects.filter( | ||
| shared_with=user, | ||
| item_type=ItemType.DATASET, | ||
| is_deleted=False, | ||
| is_enabled=True, | ||
| permission_level__in=[ | ||
| PermissionLevel.CO_OWNER, | ||
| PermissionLevel.CONTRIBUTOR, | ||
| ], | ||
| ).values_list("item_uuid", flat=True) | ||
| ) | ||
| shared = ( | ||
| Dataset.objects.filter( | ||
| uuid__in=shared_uuids, is_deleted=False, is_public=False | ||
| ) | ||
| .exclude(owner=user) | ||
| .order_by("name") | ||
| .values("uuid", "name") | ||
| ) |
There was a problem hiding this comment.
This dataset list is intended to populate the quick-add modal with datasets the user can actually add captures to. Right now it filters out deleted/public datasets but does not filter out status == DatasetStatus.FINAL, so published datasets can appear in the dropdown and then fail later with 403 when selected. Filter these querysets to exclude FINAL datasets (consistent with _get_dataset_editable_by_user).
| <!-- Captures Table --> | ||
| {% include "users/partials/captures_page_table.html" %} | ||
| </main> | ||
| </div> | ||
| </div> | ||
| <!-- Capture Details Modal --> | ||
| {% include "users/partials/capture_modal.html" %} | ||
| <!-- Quick Add to Dataset Modal (single or multiple captures) --> | ||
| {% include "users/partials/quick_add_to_dataset_modal.html" %} |
There was a problem hiding this comment.
The captures table still renders Share actions targeting #shareModal-<uuid>, but this page no longer includes the per-capture share_modal.html instances (previously rendered in a loop). That will make the Share button a no-op / open nothing. Re-add the share modal includes for each capture on the page, or change the Share UI to target a single generic modal that exists in the DOM.
| <!-- Capture Details Modal --> | ||
| {% include "users/partials/capture_modal.html" %} | ||
| <!-- Quick Add to Dataset Modal (single or multiple captures) --> | ||
| {% include "users/partials/quick_add_to_dataset_modal.html" %} | ||
| <!-- Visualization Selection Modal --> | ||
| {% if VISUALIZATIONS_ENABLED %} | ||
| {% include "visualizations/partials/visualization_modal.html" with visualization_compatibility=visualization_compatibility %} | ||
| {% endif %} | ||
| {% endblock content %} | ||
| {% include "visualizations/partials/visualization_modal.html" with visualization_compatibility=visualization_compatibility %} | ||
| <!-- Web Download Modal --> |
There was a problem hiding this comment.
capture_modal.html already conditionally includes visualizations/partials/visualization_modal.html when VISUALIZATIONS_ENABLED is true. This template now includes the visualization modal unconditionally as well, which can produce duplicate modal IDs (and breaks the feature flag behavior when visualizations are disabled). Keep the inclusion behind the same VISUALIZATIONS_ENABLED guard and ensure the modal is included only once.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.