-
Notifications
You must be signed in to change notification settings - Fork 21
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
Conversation
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
@@ -177,7 +183,7 @@ export class WdsDataTableProvider implements DataTableProvider { | |||
supportsExport: false, | |||
supportsPointCorrection: false, | |||
supportsFiltering: false, | |||
supportsRowSelection: false, | |||
supportsRowSelection: 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.
with this set to true, we render the "select all" option in the data table. Selecting that option causes an error for WDS-powered data tables.
Co-authored-by: Kevin Marete <[email protected]>
Co-authored-by: Kevin Marete <[email protected]>
Quality Gate passedIssues Measures |
import { reportError } from 'src/libs/error'; | ||
import * as Utils from 'src/libs/utils'; | ||
|
||
export const RecordDeleter = ({ onDismiss, onSuccess, dataProvider, collectionId, selectedRecords, runningSubmissionsCount }) => { |
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.
Any particular reason why this is .js
instead of .ts
? I'd love for new files to be .ts
so we don't have to convert them later!
return onDismiss(); | ||
} | ||
|
||
// Handle 409 error by filtering additional deletions that need to be deleted first |
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.
Is this in case of references? Or why would they need to be deleted first?
|
||
const moreToDelete = !!additionalDeletions.length; | ||
|
||
const total = selectedKeys.length + additionalDeletions.length; |
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.
Should this additionalDeletions
also be checked with !!
?
If I run this branch against your |
Ok I think there's a mismatch between the format of the deleteRecordsRequest in the front end and backend. In the backend, everything is camel case: |
Jira Ticket: https://broadworkbench.atlassian.net/browse/AJ-1277
Summary of changes:
What
Why
Testing strategy