Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds bulk operations for DQ (Data Quality) checks and implements various UI/UX improvements across the DQ checks management interface. The changes include adding a bulk add feature for creating multiple DQ checks simultaneously, improving table functionality with filtering and sorting capabilities, and making minor UI enhancements like consistent styling and better user guidance.
Key changes include:
- Implementation of bulk add functionality for DQ checks using a new API endpoint
- Enhanced table features with column filters, sorting controls, and a clear button
- UI improvements including consistent button styling, improved descriptions with documentation links, and better field labeling
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/redux/dqChecks/apiService.ts | Adds new postDQChecksBulk API function for bulk operations |
| src/modules/DQ/DQChecks/DQChecksHome.tsx | Simplifies component logic by auto-redirecting to manage page |
| src/modules/DQ/DQChecks/DQChecksManage.tsx | Updates table headers and adds documentation links |
| src/modules/DQ/DQChecks/DQCheckGroup*.tsx | Implements bulk add functionality, table enhancements, and UI improvements across all check group components |
| src/components/DQCheckDrawer/*.tsx | Adds bulk mode support with multi-select capabilities and improved form labeling |
| sorter: (a: any, b: any) => a.questionName.localeCompare(b.questionName), | ||
| render: (questionName: any, record: any) => | ||
| questionName + (record.isRepeatGroup ? "_*" : ""), | ||
| rfilters: dqCheckData?.map((record: any) => ({ |
There was a problem hiding this comment.
The property name 'rfilters' appears to be a typo. It should be 'filters' to match the other column definitions.
| rfilters: dqCheckData?.map((record: any) => ({ | |
| filters: dqCheckData?.map((record: any) => ({ |
| setDrawerMode("single"); | ||
| setDrawerData(selectedVariableRows[0]); | ||
| showAddManualDrawer(); | ||
| setDrawerData(null); |
There was a problem hiding this comment.
Setting drawerData to null in handleEditCheck will clear the data that should be passed to the drawer for editing. This line should be removed since the drawer needs the selected row data.
| setDrawerData(null); |
| setDrawerMode("single"); | ||
| setDrawerData(selectedVariableRows[0]); | ||
| showAddManualDrawer(); | ||
| setDrawerData(null); |
There was a problem hiding this comment.
Setting drawerData to null in handleEditCheck will clear the data that should be passed to the drawer for editing. This line should be removed since the drawer needs the selected row data.
| setDrawerData(null); |
| setDrawerMode("single"); | ||
| setDrawerData(selectedVariableRows[0]); | ||
| showAddManualDrawer(); | ||
| setDrawerData(null); |
There was a problem hiding this comment.
Setting drawerData to null in handleEditCheck will clear the data that should be passed to the drawer for editing. This line should be removed since the drawer needs the selected row data.
| setDrawerData(null); |
| <> | ||
| This check verifies whether certain continuous variables contain | ||
| outliers, where an outlier is defined to be a certain multiple | ||
| of the IQR or SD or as values beyond a given percentile.{""} |
There was a problem hiding this comment.
There's an empty string concatenation that appears to be a typo. The text should end with a space before the DescriptionLink component.
| of the IQR or SD or as values beyond a given percentile.{""} | |
| of the IQR or SD or as values beyond a given percentile.{" "} |
jeenut27
left a comment
jeenut27
left a comment
There was a problem hiding this comment.
Thank Utkarsh. Just a minor nit: Can you change the drawer header "Add Bulk DQ Checks" to "Add DQ Checks in Bulk" and change the button to "Bulk Add" instead of "Add Bulk". Rest looks good
2 participants
[SS-2126,2127] DQ : Add bulk checks and Minor UI Changes
Ticket
Fixes: SS-2126: DQ: UI/UX Changes & SS-2127: DQ : Bulk add option for mismatches, protocol violations etc
Description, Motivation and Context
How Has This Been Tested?
Local
UI Changes
To-do before merge
Checklist:
This checklist is a useful reminder of small things that can easily be forgotten.
Put an
xin all the items that apply and remove any items that are not relevant to this PR.