From 84a16a95b462c3b29e32680356c49636b4011d24 Mon Sep 17 00:00:00 2001 From: Dinika Saxena Date: Wed, 21 Feb 2024 11:52:56 +0100 Subject: [PATCH 1/2] Download of resources with tags in their ids should work When storing these resources and their distributions to the localstorage the tag should be stored as well. Otherwise the `key` (required to add/remove items from data panel) is inconsistent, leading to bugs (example, adding the resource works, but removing them from the data panel does not, because the id is not the same). Signed-off-by: Dinika Saxena --- .../ResourceViewActionsContainer.tsx | 3 +- src/shared/hooks/useAccessDataForTable.tsx | 7 ++- .../molecules/MyDataTable/MyDataTable.tsx | 20 +++++-- src/shared/utils/__tests__/datapanel.spec.ts | 54 +++++++++++++++---- src/shared/utils/datapanel.ts | 16 +++--- .../dataExplorer/DataExplorerTable.tsx | 5 +- .../search/containers/SearchContainer.tsx | 4 +- 7 files changed, 80 insertions(+), 29 deletions(-) diff --git a/src/shared/containers/ResourceViewActionsContainer.tsx b/src/shared/containers/ResourceViewActionsContainer.tsx index 139ea6791..46ed65339 100644 --- a/src/shared/containers/ResourceViewActionsContainer.tsx +++ b/src/shared/containers/ResourceViewActionsContainer.tsx @@ -69,7 +69,8 @@ const ResourceViewActionsContainer: React.FC<{ } else { const localStorageObjects = toLocalStorageResources( resource, - 'resource-view' + 'resource-view', + recordKey ); selectedRowKeys = uniq([...selectedRowKeys, recordKey]); diff --git a/src/shared/hooks/useAccessDataForTable.tsx b/src/shared/hooks/useAccessDataForTable.tsx index 57ad4e773..3f3de7fd6 100644 --- a/src/shared/hooks/useAccessDataForTable.tsx +++ b/src/shared/hooks/useAccessDataForTable.tsx @@ -499,13 +499,15 @@ export const useAccessDataForTable = ( return; }) .process(async row => { + const recordKey = getStudioLocalStorageKey(row); const fetchedRow = await fetchResourceForDownload( getStudioLocalStorageKey(row), nexus ); const localStorageResources = toLocalStorageResources( fetchedRow, - 'studios' + 'studios', + recordKey ); rowKeysForLS.push(getStudioLocalStorageKey(row)); @@ -544,7 +546,8 @@ export const useAccessDataForTable = ( const deltaResource = await fetchResourceForDownload(recordKey, nexus); const localStorageResource = toLocalStorageResources( deltaResource, - 'studios' + 'studios', + recordKey ); localStorageRowKeys = [...localStorageRowKeys, recordKey]; localStorageRows = [...localStorageRows, ...localStorageResource]; diff --git a/src/shared/molecules/MyDataTable/MyDataTable.tsx b/src/shared/molecules/MyDataTable/MyDataTable.tsx index b8885ddc7..d3f19b158 100644 --- a/src/shared/molecules/MyDataTable/MyDataTable.tsx +++ b/src/shared/molecules/MyDataTable/MyDataTable.tsx @@ -414,11 +414,16 @@ const MyDataTable: React.FC = ({ setFetchingResources(true); const expandedResource = await fetchResourceForDownload(recordKey, nexus); setFetchingResources(false); - localStorageRows = toLocalStorageResources(expandedResource, 'my-data'); + localStorageRows = toLocalStorageResources( + expandedResource, + 'my-data', + recordKey + ); } else { - localStorageRows = toLocalStorageResources(record, 'my-data'); + localStorageRows = toLocalStorageResources(record, 'my-data', recordKey); } - toLocalStorageResources(record, 'my-data'); + // TODO: Check if needed + toLocalStorageResources(record, 'my-data', recordKey); let selectedRowKeys = dataPanelLS?.selectedRowKeys || []; let selectedRows = dataPanelLS?.selectedRows || []; @@ -488,10 +493,15 @@ const MyDataTable: React.FC = ({ const fetchedRow = await fetchResourceForDownload(row._self, nexus); localStorageResources = toLocalStorageResources( fetchedRow, - 'my-data' + 'my-data', + row._self ); } else { - localStorageResources = toLocalStorageResources(row, 'my-data'); + localStorageResources = toLocalStorageResources( + row, + 'my-data', + row._self + ); } return localStorageResources; }); diff --git a/src/shared/utils/__tests__/datapanel.spec.ts b/src/shared/utils/__tests__/datapanel.spec.ts index 0247a5e5b..0fd642105 100644 --- a/src/shared/utils/__tests__/datapanel.spec.ts +++ b/src/shared/utils/__tests__/datapanel.spec.ts @@ -20,7 +20,8 @@ describe('datapanel utilities', () => { it('serializes resources with no distribution correctly to local storage object', () => { const actualLSResources = toLocalStorageResources( resourceWithoutDistrition, - 'studios' + 'studios', + resourceWithoutDistrition._self ); const expectedParentDistributionValue = { hasDistribution: false, @@ -49,7 +50,8 @@ describe('datapanel utilities', () => { it('serializes resource of type file with no distribution correctly', () => { const actualLSResources = toLocalStorageResources( fileResourceWithNoDistribution, - 'my-data' + 'my-data', + fileResourceWithNoDistribution._self ); expect(actualLSResources.length).toEqual(1); const expectedParentDistributionValue = { @@ -66,7 +68,11 @@ describe('datapanel utilities', () => { it('serializes resources with distribution array correctly to local storage object', () => { const resource = resourceWithDistributionArray; - const serializedItems = toLocalStorageResources(resource, 'studios'); + const serializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(serializedItems.length).toEqual(5); const actualParentDistributionValue = serializedItems[0].distribution; @@ -88,7 +94,11 @@ describe('datapanel utilities', () => { label: undefined, name: ['Sterling', 'Malory', 'Archer'], }; - const serializedItems = toLocalStorageResources(resource, 'studios'); + const serializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(serializedItems.length).toEqual(5); expect(serializedItems[0].name).toEqual('Sterling-Malory-Archer'); @@ -96,7 +106,11 @@ describe('datapanel utilities', () => { it('serializes correct distribution value for each distribution item in array', () => { const resource = resourceWithDistributionArray; - const serializedItems = toLocalStorageResources(resource, 'studios'); + const serializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); const originalDistItems = resource.distribution; const serializedDistItems = serializedItems.slice(1); @@ -119,7 +133,11 @@ describe('datapanel utilities', () => { it('serializes resources with distribution objects correctly', () => { const resource = resourceWithDistributionObject; - const actualSerializedItems = toLocalStorageResources(resource, 'studios'); + const actualSerializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(actualSerializedItems.length).toEqual(2); @@ -158,7 +176,11 @@ describe('datapanel utilities', () => { contentSize: 123, }, }; - const actualSerializedItems = toLocalStorageResources(resource, 'studios'); + const actualSerializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(actualSerializedItems[1].distribution?.contentSize).toEqual(123); }); @@ -171,14 +193,22 @@ describe('datapanel utilities', () => { contentSize: [10, 20], }, }; - const actualSerializedItems = toLocalStorageResources(resource, 'studios'); + const actualSerializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(actualSerializedItems[1].distribution?.contentSize).toEqual(30); }); it('serializes resources when distribution is empty array', () => { const resource = { ...resourceWithoutDistrition, distribution: [] }; - const actualSerializedItems = toLocalStorageResources(resource, 'studios'); + const actualSerializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(actualSerializedItems.length).toEqual(1); const expectedDistributionValue = { @@ -195,7 +225,11 @@ describe('datapanel utilities', () => { it('serializes resources when distribution is empty object', () => { const resource = { ...resourceWithoutDistrition, distribution: {} }; - const actualSerializedItems = toLocalStorageResources(resource, 'studios'); + const actualSerializedItems = toLocalStorageResources( + resource, + 'studios', + resource._self + ); expect(actualSerializedItems.length).toEqual(2); expect(actualSerializedItems[0].distribution).toBeDefined(); diff --git a/src/shared/utils/datapanel.ts b/src/shared/utils/datapanel.ts index 8866689ac..b34063223 100644 --- a/src/shared/utils/datapanel.ts +++ b/src/shared/utils/datapanel.ts @@ -11,11 +11,12 @@ import { parseURL } from './nexusParse'; const baseLocalStorageObject = ( resource: Resource, source: string, + key: string, keySuffix?: string ): Omit => { return { source, - key: keySuffix ? `${resource._self}-${keySuffix}` : resource._self, + key: keySuffix ? `${key}-${keySuffix}` : key, _self: resource._self, id: resource['@id'], name: getResourceLabel(resource), @@ -31,7 +32,8 @@ const baseLocalStorageObject = ( export const toLocalStorageResources = ( resource: Resource, - source: string + source: string, + key: string ): TDataSource[] => { const resourceName = getResourceLabel(resource); try { @@ -39,7 +41,7 @@ export const toLocalStorageResources = ( if (isNil(resource.distribution)) { return [ { - ...baseLocalStorageObject(resource, source), + ...baseLocalStorageObject(resource, source, key), localStorageType: 'resource', distribution: { hasDistribution: false, @@ -63,7 +65,7 @@ export const toLocalStorageResources = ( // First store an object for the parent resource. localStorageObjs.push({ - ...baseLocalStorageObject(resource, source), + ...baseLocalStorageObject(resource, source, key), localStorageType: 'resource', distributionItemsLength: resource.distribution.length, distribution: { @@ -79,7 +81,7 @@ export const toLocalStorageResources = ( // Now store an object for each distribution item resource.distribution.forEach((distItem, index) => { localStorageObjs.push({ - ...baseLocalStorageObject(resource, source, `${index}`), + ...baseLocalStorageObject(resource, source, key, `${index}`), localStorageType: 'distribution', distribution: { hasDistribution: true, // So, we don't download the distribution twice @@ -100,7 +102,7 @@ export const toLocalStorageResources = ( return [ // First store an object for the parent resource. { - ...baseLocalStorageObject(resource, source), + ...baseLocalStorageObject(resource, source, key), localStorageType: 'resource', distribution: { hasDistribution: true, @@ -113,7 +115,7 @@ export const toLocalStorageResources = ( }, // Now store an object for the distribution item. { - ...baseLocalStorageObject(resource, source, '1'), + ...baseLocalStorageObject(resource, source, key, '1'), localStorageType: 'distribution', distribution: { hasDistribution: true, diff --git a/src/subapps/dataExplorer/DataExplorerTable.tsx b/src/subapps/dataExplorer/DataExplorerTable.tsx index f7b16ab20..79310def4 100644 --- a/src/subapps/dataExplorer/DataExplorerTable.tsx +++ b/src/subapps/dataExplorer/DataExplorerTable.tsx @@ -126,7 +126,8 @@ export const DataExplorerTable = forwardRef( const localStorageRows = toLocalStorageResources( record, - DATA_EXPLORER_NAMESPACE + DATA_EXPLORER_NAMESPACE, + recordKey ); let selectedRowKeys = dataPanelLS?.selectedRowKeys || []; let selectedRows = dataPanelLS?.selectedRows || []; @@ -177,7 +178,7 @@ export const DataExplorerTable = forwardRef( if (selected) { const results = changeRows.map(row => - toLocalStorageResources(row, DATA_EXPLORER_NAMESPACE) + toLocalStorageResources(row, DATA_EXPLORER_NAMESPACE, row._self) ); selectedRows = [...selectedRows, ...results.flat()]; selectedRowKeys = [...selectedRowKeys, ...changeRows.map(t => t._self)]; diff --git a/src/subapps/search/containers/SearchContainer.tsx b/src/subapps/search/containers/SearchContainer.tsx index 053b2f7d1..113f0c380 100644 --- a/src/subapps/search/containers/SearchContainer.tsx +++ b/src/subapps/search/containers/SearchContainer.tsx @@ -149,8 +149,8 @@ const SearchContainer: React.FC = () => { resetAll(); }; const handleSelect = (record: Resource, selected: any) => { - const newRecords = toLocalStorageResources(record, layout); const recordKey = record._self; + const newRecords = toLocalStorageResources(record, layout, recordKey); const dataPanelLS: TResourceTableData = JSON.parse( localStorage.getItem(DATA_PANEL_STORAGE)! ); @@ -195,7 +195,7 @@ const SearchContainer: React.FC = () => { ) => { const changedRowsLS: TDataSource[] = []; changeRows.forEach(row => { - const localStorageRows = toLocalStorageResources(row, layout); + const localStorageRows = toLocalStorageResources(row, layout, row._self); changedRowsLS.push(...localStorageRows); }); From 3a2afc49d155e800fcdaa228b4cce0179e9eaaee Mon Sep 17 00:00:00 2001 From: Dinika Saxena Date: Wed, 21 Feb 2024 18:04:05 +0100 Subject: [PATCH 2/2] Studio // Dont override column config when one aleady exists Signed-off-by: Dinika Saxena --- src/shared/components/EditTableForm.tsx | 58 ++++++++++++++-------- src/shared/hooks/useAccessDataForTable.tsx | 1 + 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/shared/components/EditTableForm.tsx b/src/shared/components/EditTableForm.tsx index 3283ba687..603e85987 100644 --- a/src/shared/components/EditTableForm.tsx +++ b/src/shared/components/EditTableForm.tsx @@ -273,14 +273,18 @@ const EditTableForm: React.FC<{ return Object.assign(result, current); }, {}); - return Object.keys(mergedItem).map(title => ({ - '@type': '', - name: title, - format: '', - enableSearch: false, - enableSort: false, - enableFilter: false, - })); + return Object.keys(mergedItem).map(title => { + const currentItemConfig = findCurrentColumnConfig(title); + return { + '@type': '', + name: title, + format: '', + enableSearch: false, + enableSort: false, + enableFilter: false, + ...currentItemConfig, + }; + }); } const result = await querySparql( @@ -295,14 +299,19 @@ const EditTableForm: React.FC<{ .sort((a, b) => { return a.title > b.title ? 1 : -1; }) - .map(x => ({ - '@type': 'text', - name: x.dataIndex, - format: '', - enableSearch: false, - enableSort: false, - enableFilter: false, - })); + .map(header => { + const currentHeaderConfig = + findCurrentColumnConfig(header.dataIndex) ?? {}; + return { + '@type': 'text', + name: header.dataIndex, + format: '', + enableSearch: false, + enableSort: false, + enableFilter: false, + ...currentHeaderConfig, + }; + }); }) .catch(error => { // Sometimes delta's error message can be in `name` or `reason` field. @@ -322,12 +331,7 @@ const EditTableForm: React.FC<{ { onSuccess: data => { updateTableDataError(null); - if ( - isNil(configuration) || - (configuration as TableColumn[]).length === 0 - ) { - setConfiguration(data); - } + setConfiguration(data); }, onError: (error: Error) => { updateTableDataError(error); @@ -339,6 +343,16 @@ const EditTableForm: React.FC<{ } ); + const findCurrentColumnConfig = (name: string) => { + if (Array.isArray(configuration)) { + return configuration.find(column => column.name === name); + } + if (configuration?.name === name) { + return { ...configuration }; + } + return undefined; + }; + const onChangeName = (event: any) => { setName(event.target.value); setNameError(false); diff --git a/src/shared/hooks/useAccessDataForTable.tsx b/src/shared/hooks/useAccessDataForTable.tsx index 3f3de7fd6..fc81bf68a 100644 --- a/src/shared/hooks/useAccessDataForTable.tsx +++ b/src/shared/hooks/useAccessDataForTable.tsx @@ -634,6 +634,7 @@ export const useAccessDataForTable = ( ); return result; } + return {}; }, {