Skip to content
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

Draft
wants to merge 15 commits into
base: dev
Choose a base branch
from
12 changes: 12 additions & 0 deletions src/libs/ajax/WorkspaceDataService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import _ from 'lodash/fp';
import { authOpts } from 'src/auth/auth-session';
import { fetchWDS } from 'src/libs/ajax/ajax-common';
import {
DeleteRecordsRequest,
RecordQueryResponse,
RecordResponseBody,
RecordTypeSchema,
Expand Down Expand Up @@ -197,6 +198,17 @@ export const WorkspaceData = (signal) => ({
resultJson.records = _.map(_.unset('attributes.sys_name'), resultJson.records);
return resultJson;
},
deleteRecords: async (
root: string,
collectionId: string,
recordType: string,
parameters: DeleteRecordsRequest
): Promise<any> => {
await fetchWDS(root)(
`records/v1/${collectionId}/${recordType}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`records/v1/${collectionId}/${recordType}`,
`records/v1/${collectionId}/${recordType}/delete`,

Update URL to this since I am getting a 404
Screenshot 2024-09-20 at 4 24 40 PM

_.mergeAll([authOpts(), jsonBody(parameters), { signal, method: 'POST' }])
);
},
describeAllRecordTypes: async (root: string, instanceId: string): Promise<any> => {
const res = await fetchWDS(root)(`${instanceId}/types/v0.2`, _.mergeAll([authOpts(), { signal, method: 'GET' }]));
return _.map(
Expand Down
8 changes: 7 additions & 1 deletion src/libs/ajax/data-table-providers/WdsDataTableProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ export interface SearchRequest {
sortAttribute?: string;
}

export interface DeleteRecordsRequest {
record_ids?: string[];
excluded_record_ids?: string[];
delete_all?: boolean;
}

export type RecordAttributes = Record<string, unknown>; // truly "unknown" here; the backend Java representation is Map<String, Object>

export interface RecordResponse {
Expand Down Expand Up @@ -177,7 +183,7 @@ export class WdsDataTableProvider implements DataTableProvider {
supportsExport: false,
supportsPointCorrection: false,
supportsFiltering: false,
supportsRowSelection: false,
supportsRowSelection: true,
Copy link
Contributor

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.

supportsPerColumnDatatype: true,
};
}
Expand Down
98 changes: 98 additions & 0 deletions src/workspace-data/data-table/wds/RecordDeleter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import _ from 'lodash/fp';
import { useState } from 'react';
import { b, div, h } from 'react-hyperscript-helpers';
import { absoluteSpinnerOverlay, DeleteConfirmationModal } from 'src/components/common';
import { Ajax } from 'src/libs/ajax';
import colors from 'src/libs/colors';
import { reportError } from 'src/libs/error';
import * as Utils from 'src/libs/utils';

export const RecordDeleter = ({ onDismiss, onSuccess, dataProvider, collectionId, selectedRecords, runningSubmissionsCount }) => {
const [additionalDeletions, setAdditionalDeletions] = useState([]);
const [deleting, setDeleting] = useState(false);

const selectedKeys = _.keys(selectedRecords);

const doDelete = async () => {
const recordsToDelete = _.flow(
_.map(({ name: entityName, entityType }) => ({ entityName, entityType })),
(records) => _.concat(additionalDeletions, records)
)(selectedRecords);

const recordTypes = _.uniq(_.map(({ entityType }) => entityType, selectedRecords));
if (recordTypes.length > 1) {
await reportError('Something went wrong; more than one recordType is represented in the selection. This should not happen.');
}
const recordType = recordTypes[0];
setDeleting(true);

Copy link
Contributor

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.

Suggested change
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
)
);
};

try {
await Ajax().WorkspaceData.deleteRecords(dataProvider.proxyUrl, collectionId, recordType, {
record_ids: recordsToDelete,
});
onSuccess();
} catch (error) {
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();
Comment on lines +35 to +51
Copy link
Contributor

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

Suggested change
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);

}
}
};

const moreToDelete = !!additionalDeletions.length;

const total = selectedKeys.length + additionalDeletions.length;
return h(
DeleteConfirmationModal,
{
objectType: 'data',
title: `Delete ${total} ${total > 1 ? 'entries' : 'entry'}`,
onConfirm: doDelete,
onDismiss,
},
[
runningSubmissionsCount > 0 &&
b({ style: { display: 'block', margin: '1rem 0' } }, [
`WARNING: ${runningSubmissionsCount} workflows are currently running in this workspace. ` +
'Deleting the following entries could cause workflows using them to fail.',
]),
moreToDelete &&
b({ style: { display: 'block', margin: '1rem 0' } }, [
'In order to delete the selected data entries, the following entries that reference them must also be deleted.',
]),
// Size the scroll container to cut off the last row to hint that there's more content to be scrolled into view
// Row height calculation is font size * line height + padding + border
div(
{ style: { maxHeight: 'calc((1em * 1.15 + 1.2rem + 1px) * 10.5)', overflowY: 'auto', margin: '0 -1.25rem' } },
_.map(
([i, entity]) =>
div(
{
style: {
borderTop: i === 0 && runningSubmissionsCount === 0 ? undefined : `1px solid ${colors.light()}`,
padding: '0.6rem 1.25rem',
},
},
moreToDelete ? `${entity.entityName} (${entity.entityType})` : entity
),
Utils.toIndexPairs(moreToDelete ? additionalDeletions : selectedKeys)
)
),
deleting && absoluteSpinnerOverlay,
]
);
};
28 changes: 27 additions & 1 deletion src/workspace-data/data-table/wds/WDSContent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ const defaultProps: WDSContentProps = {
// attributes are required to avoid an error while destructuring from 'workspace-column-defaults'
attributes: {},
},
workspaceSubmissionStats: {
runningSubmissionsCount: 0,
},
},
recordType: marbleSchema.name,
wdsSchema: [marbleSchema],
Expand All @@ -76,7 +79,7 @@ const defaultFeatures: DataTableFeatures = {
supportsExport: false,
supportsPointCorrection: false,
supportsFiltering: false,
supportsRowSelection: false,
supportsRowSelection: true,
supportsPerColumnDatatype: true,
};

Expand Down Expand Up @@ -243,4 +246,27 @@ describe('WDSContent', () => {
expect(editableValues.length).toEqual(3);
});
});

describe('select rows', () => {
it('', async () => {
// Arrange
const { props } = setup({
...defaultSetupOptions,
props: { ...defaultProps, editable: true },
features: {
...defaultFeatures,
},
});

// Act
await act(() => {
render(h(WDSContent, props));
});

// Assert
// there should be 4 checkboxes for 3 values: one for each value, plus one to select all rows.
const checkboxes = await screen.findAllByRole('checkbox');
expect(checkboxes.length).toEqual(4);
});
});
});
73 changes: 68 additions & 5 deletions src/workspace-data/data-table/wds/WDSContent.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import _ from 'lodash/fp';
import { Fragment, useState } from 'react';
import { h } from 'react-hyperscript-helpers';
import { div, h } from 'react-hyperscript-helpers';
import { ButtonSecondary } from 'src/components/common';
import { icon } from 'src/components/icons';
import { MenuButton } from 'src/components/MenuButton';
import { MenuTrigger } from 'src/components/PopupTrigger';
import { Ajax } from 'src/libs/ajax';
import { DataTableProvider } from 'src/libs/ajax/data-table-providers/DataTableProvider';
import { RecordTypeSchema, wdsToEntityServiceMetadata } from 'src/libs/ajax/data-table-providers/WdsDataTableProvider';
import colors from 'src/libs/colors';
import Events, { extractWorkspaceDetails } from 'src/libs/events';
import { isGoogleWorkspace, WorkspaceWrapper as Workspace } from 'src/workspaces/utils';
import * as WorkspaceUtils from 'src/workspaces/utils';

import DataTable from '../shared/DataTable';
import { RecordDeleter } from './RecordDeleter';

export interface WDSContentProps {
workspace: Workspace;
Expand All @@ -26,11 +35,49 @@ export const WDSContent = ({
}: WDSContentProps) => {
const googleProject = isGoogleWorkspace(workspace) ? workspace.workspace.googleProject : undefined;
// State
const [refreshKey] = useState(0);
const [refreshKey, setRefreshKey] = useState(0);
const [selectedRecords, setSelectedRecords] = useState({});
const [deletingRecords, setDeletingRecords] = useState(false);

// Render
const [entityMetadata, setEntityMetadata] = useState(() => wdsToEntityServiceMetadata(wdsSchema));

const recordsSelected = !_.isEmpty(selectedRecords);

// This is a (mostly) copy/paste from the EntitiesContent component.
// Maintainers of the future should consider abstracting it into its own component or shared function.
const renderEditMenu = () => {
return h(
MenuTrigger,
{
side: 'bottom',
closeOnClick: true,
content: h(Fragment, [
h(
MenuButton,
{
disabled: !recordsSelected,
tooltip: !recordsSelected && 'Select rows to delete in the table',
onClick: () => setDeletingRecords(true),
},
['Delete selected rows']
),
]),
},
[
h(
ButtonSecondary,
{
tooltip: 'Edit data',
...WorkspaceUtils.getWorkspaceEditControlProps(workspace as WorkspaceUtils.WorkspaceAccessInfo),
style: { marginRight: '1.5rem' },
},
[icon('edit', { style: { marginRight: '0.5rem' } }), 'Edit']
),
]
);
};

// dataProvider contains the proxyUrl for an instance of WDS
return h(Fragment, [
h(DataTable, {
Expand All @@ -45,19 +92,35 @@ export const WDSContent = ({
workspace,
snapshotName: undefined,
selectionModel: {
selected: [],
setSelected: () => [],
selected: selectedRecords,
setSelected: setSelectedRecords,
},
setEntityMetadata,
childrenBefore: undefined,
enableSearch: false,
controlPanelStyle: {
background: colors.light(1),
borderBottom: `1px solid ${colors.grey(0.4)}`,
},
border: false,
loadMetadata,
childrenBefore: () => div({ style: { display: 'flex', alignItems: 'center', flex: 'none' } }, [renderEditMenu()]),
}),
deletingRecords &&
h(RecordDeleter, {
onDismiss: () => setDeletingRecords(false),
onSuccess: () => {
setDeletingRecords(false);
setSelectedRecords({});
setRefreshKey(_.add(1));
Ajax().Metrics.captureEvent(Events.workspaceDataDelete, extractWorkspaceDetails(workspace.workspace));
Copy link
Contributor

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

loadMetadata();
},
dataProvider,
collectionId: workspace.workspace.workspaceId,
selectedRecords,
selectedRecordType: recordType,
runningSubmissionsCount: workspace?.workspaceSubmissionStats?.runningSubmissionsCount,
}),
]);
};

Expand Down
Loading