-
Notifications
You must be signed in to change notification settings - Fork 0
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
JNG-3980 filter empty values #449
Conversation
noherczeg
commented
Aug 15, 2024
•
edited by github-actions
bot
Loading
edited by github-actions
bot
JNG-3980 Filter for empty values on tables |
WalkthroughIn this update, several components of the Judo UI framework have undergone significant improvements in filter handling. The logic for filtering operations in components like Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EagerTable
participant LazyTable
participant FilterHelper
User->>EagerTable: Update filters
EagerTable->>FilterHelper: Map filters
FilterHelper-->>EagerTable: Return newFilters
EagerTable->>User: Display updated table
User->>LazyTable: Update filters
LazyTable->>FilterHelper: Map filters
FilterHelper-->>LazyTable: Return newFilters
LazyTable->>User: Display updated table
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (11)
Files skipped from review due to trivial changes (2)
Additional comments not posted (20)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
judo-ui-react/src/main/resources/actor/src/utilities/filter-helper.ts.hbs
Show resolved
Hide resolved
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.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (6)
judo-ui-react/src/main/resources/actor/src/components/table/table-column-operators.tsx.hbs (3)
13-15
: Expand the comment for clarity.The comment explains why 'startsWith' and 'endsWith' are excluded but does not mention 'isAnyOf'. Consider expanding the comment to explain this exclusion as well.
// startsWith and endsWith is filtered out because they'd need complex value mapping... + // isAnyOf is excluded to prevent multiple selections in a single filter.
19-20
: Add a comment to explain exclusion.Consider adding a comment explaining why 'isAnyOf' is excluded for numeric columns.
// Exclude 'isAnyOf' to limit filtering to single numeric values.
Line range hint
38-39
: Add a comment to explain exclusion.Consider adding a comment explaining why 'isAnyOf' is excluded for single select columns.
// Exclude 'isAnyOf' to ensure only one selection is possible.judo-ui-react-itest/ActionGroupTestPro/action_group_test_pro__god/src/test/resources/snapshots/frontend-react/src/components/table/EagerTable.tsx.snapshot (1)
238-241
: Add Check for UndefinedfilterModel.items
inmapFilterModelToFilters
The
mapFilterModelToFilters
function does not handle the case wherefilterModel.items
might be undefined, which could lead to runtime errors. Consider adding a check to ensurefilterModel.items
is defined before iterating over it.
- File:
judo-ui-react/src/main/resources/actor/src/utilities/filter-helper.ts.hbs
- Relevant Code Snippet:
export function mapFilterModelToFilters(filterModel: GridFilterModel, filterOptions: FilterOption[]): Filter[] { const filters: Filter[] = []; for (const item of filterModel.items) { const filterOption = filterOptions.find((o) => o.attributeName === item.field); if (!filterOption) { console.warn(`Could not map filter option for filter item: ${JSON.stringify(item, null, 4)}`); continue; } // ... } // ... }Analysis chain
Ensure handling of undefined values.
The check for defined values in
newModel.items
has been removed. Ensure thatmapFilterModelToFilters
can handle undefined values appropriately to avoid potential runtime errors.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that mapFilterModelToFilters can handle undefined values in newModel.items. # Test: Search for the mapFilterModelToFilters function definition and ensure it handles undefined values. ast-grep --lang typescript --pattern $'function mapFilterModelToFilters($_, $_) { $$$ }'Length of output: 91
Script:
#!/bin/bash # Search for the mapFilterModelToFilters function in the codebase to find its definition or usage. rg 'mapFilterModelToFilters' -A 5Length of output: 11942
Script:
#!/bin/bash # Extract the full implementation of the mapFilterModelToFilters function to verify handling of undefined values. rg 'export function mapFilterModelToFilters' -A 20 judo-ui-react/src/main/resources/actor/src/utilities/filter-helper.ts.hbsLength of output: 811
judo-ui-react/src/main/resources/actor/src/utilities/filter-helper.ts.hbs (2)
100-102
: Clarify logic inisFilterWithoutValue
.The function checks for
isDefined
andisUndefined
operations. Ensure this logic aligns with the intended use of filters without values.- return ['isDefined', 'isUndefined'].includes(getOperationEnumValue(filter, filter.filterBy.operator)) + return ['isDefined', 'isUndefined'].includes(filter.filterBy.operator)
104-107
: Document the purpose ofucFirst
.The function capitalizes the first letter of a string. Consider adding a comment to clarify its purpose for future maintainers.
+// Capitalizes the first letter of a string export const ucFirst = (str: string): string => {