-
Notifications
You must be signed in to change notification settings - Fork 22
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
[AJ-1277] select and delete rows #5090
base: dev
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
parameters: DeleteRecordsRequest | ||
): Promise<any> => { | ||
await fetchWDS(root)( | ||
`records/v1/${collectionId}/${recordType}`, |
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.
setDeletingRecords(false); | ||
setSelectedRecords({}); | ||
setRefreshKey(_.add(1)); | ||
Ajax().Metrics.captureEvent(Events.workspaceDataDelete, extractWorkspaceDetails(workspace.workspace)); |
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.
I like that we are capturing the deletion event
} | ||
const recordType = recordTypes[0]; | ||
setDeleting(true); | ||
|
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.
Add a new function filterAdditionalDeletions
to ensure the logic is testable independently.
const filterAdditionalDeletions = async (error: Response, recordsToDelete: Array<{ entityType: string, entityName: string }>) => { | |
const errorEntities = await error.json(); | |
return _.filter(errorEntities, (errorEntity: { entityType: string, entityName: string }) => | |
!_.some(recordsToDelete, (selectedEntity) => | |
selectedEntity.entityType === errorEntity.entityType && selectedEntity.entityName === errorEntity.entityName | |
) | |
); | |
}; |
switch (error.status) { | ||
case 409: | ||
setAdditionalDeletions( | ||
_.filter( | ||
(errorEntity) => | ||
!_.some( | ||
(selectedEntity) => selectedEntity.entityType === errorEntity.entityType && selectedEntity.entityName === errorEntity.entityName, | ||
recordsToDelete | ||
), | ||
await error.json() | ||
) | ||
); | ||
setDeleting(false); | ||
break; | ||
default: | ||
await reportError('Error deleting data entries', error); | ||
onDismiss(); |
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.
Sonarcloud suggested removing the switch case: "switch" statements should have at least 3 "case" clauses
Use the new function filterAdditionalDeletions
added above in your implementation
switch (error.status) { | |
case 409: | |
setAdditionalDeletions( | |
_.filter( | |
(errorEntity) => | |
!_.some( | |
(selectedEntity) => selectedEntity.entityType === errorEntity.entityType && selectedEntity.entityName === errorEntity.entityName, | |
recordsToDelete | |
), | |
await error.json() | |
) | |
); | |
setDeleting(false); | |
break; | |
default: | |
await reportError('Error deleting data entries', error); | |
onDismiss(); | |
if (error.status != 409){ | |
await reportError('Error deleting data entries', error); | |
return onDismiss(); | |
} | |
// Handle 409 error by filtering additional deletions that need to be deleted first | |
setAdditionalDeletions(await filterAdditionalDeletions(error, recordsToDelete)); | |
setDeleting(false); |
Jira Ticket: https://broadworkbench.atlassian.net/browse/AJ-1277
Summary of changes:
What
Why
Testing strategy