-
Notifications
You must be signed in to change notification settings - Fork 21
[O2B-1491] Add current status filtering for environments #2031
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
[O2B-1491] Add current status filtering for environments #2031
Conversation
Introduces a filter for the environment's current status. Make backend filtering logic handle unknown statuses and to handle known 'MIXED' and 'DONE' statuses. Updates tests to verify filtering functionality.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2031 +/- ##
==========================================
+ Coverage 45.47% 45.64% +0.17%
==========================================
Files 1023 1023
Lines 17051 17053 +2
Branches 3092 3092
==========================================
+ Hits 7754 7784 +30
+ Misses 9297 9269 -28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Houwie7000
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! The only feedback that I have is that the filtering now gives the same result if you check every box and when you reset the filters. This makes sense from a technical point of view but makes me wonder if the status filters should have been
- All checked by default
- Maybe be an excluding filter? Every checked option excludes that status from results?
I should add that the current functionality matches that of all other filters on the BKP pages, so this is more a discussion point for the Bookkeeping filters as a whole and not your PR specifically.
Yes, interesting point. From a UX perspective, I think the current behaviour is quite standard, most sites treat nothing being selected as "don't apply this filter". Maybe a "Select all" button to quickly be able to get to the state where you only need to select one or two filters not lots to could be a solution could facilitate excluding. A whole new filter might be overkill, users will let us know. |
…s-to-envs-filtering-panel
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.
Functionality wise, the changes deliver on the promises. Good job!
There are a few extra bits which are think are not worth adding. Please have a look and let me know what you think
lib/public/views/Environments/Overview/EnvironmentOverviewModel.js
Outdated
Show resolved
Hide resolved
As a result of PR comments removed the status mixed from possible statuses ENUM. Also removed filtering by the 'UNKNOWN' status given it is an unreachable status.
Introduces a filter for the environment's current status.
Make backend filtering logic handle unknown statuses and to handle known 'MIXED' and 'DONE' statuses.
Updates tests to verify filtering functionality.
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
Changes made to the database: