-
Notifications
You must be signed in to change notification settings - Fork 84
[ENG-1974] Filtered bulk actions for DSRs #7027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENG-1974] Filtered bulk actions for DSRs #7027
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (70.96%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7027 +/- ##
=======================================
Coverage 87.30% 87.31%
=======================================
Files 532 532
Lines 34936 34962 +26
Branches 4048 4047 -1
=======================================
+ Hits 30502 30526 +24
- Misses 3552 3558 +6
+ Partials 882 878 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile OverviewGreptile SummaryThis PR implements filter-based bulk actions for DSRs, enabling users to act on multiple pages of results by applying filters instead of requiring explicit ID lists. Key Changes:
Implementation Quality:
Confidence Score: 4/5
Important Files ChangedFile Analysis
|
|
@greptile please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 2 comments
tests/ops/api/v1/endpoints/privacy_request/test_privacy_request_endpoints.py
Outdated
Show resolved
Hide resolved
tests/ops/api/v1/endpoints/privacy_request/test_privacy_request_performance.py
Show resolved
Hide resolved
…ing-bulk-actions-on-all-results
|
@greptile please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 2 comments
tests/ops/api/v1/endpoints/privacy_request/test_privacy_request_endpoints.py
Show resolved
Hide resolved
|
@greptile please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 files reviewed, 1 comment
tests/ops/api/v1/endpoints/privacy_request/test_privacy_request_endpoints.py
Show resolved
Hide resolved
|
@greptile please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
src/fides/service/privacy_request/privacy_request_service.py, line 138-146 (link)style:
get_batches_for_bulk_operationreturns[[]]whenrequest_idsis empty, causing unnecessary iteration. Consider early return for empty lists.
7 files reviewed, 1 comment
vcruces
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Thanks for applying the batching change, this will be really useful as we start handling larger numbers of DSRs
Ticket ENG-1974
Description Of Changes
🎯 As a Fides user I want to be able to filter my results and then act on them, so that I can perform actions on many pages of results instead of having to go 1 by 1.
AC:
Code Changes
src/fides/api/api/v1/endpoints/privacy_request_endpoints.py: updated bulk functions with updates schema and functionalitytests/ops/api/v1/endpoints/privacy_request/test_privacy_request_endpoints.py: All existing tests should continue to pass addedTestFilteredBulkActionsfor the new filter functionality.PRs in this chain:
Steps to Confirm
Endpoints:
/api/v1/privacy-request/bulk/soft-delete✅ manually tested with ids and filters/api/v1/privacy-request/bulk/retry✅ manually tested with ids and filters/api/v1/privacy-request/bulk/finalize(if you have it enabled) ✅ manually tested with ids and filters/api/v1/privacy-request/administrate/approve✅ manually tested with ids and filters/api/v1/privacy-request/administrate/deny✅ manually tested with ids and filters/api/v1/privacy-request/administrate/cancel✅ manually tested with ids and filtersExample:
POST /api/v1/privacy-request/bulk/soft-deletewith the following will delete all errored dsrs except the excluded id.Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works