From f69707bb98ece8521269c10df1d9013b12523a77 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Thu, 20 Feb 2025 13:23:38 +0100 Subject: [PATCH] Fix child item still visible after parent item created in library view. Fix #600 --- src/js/actions/recognize.js | 2 +- src/js/reducers/libraries/items-top.js | 61 +++++++++++++++++++++----- test/recognize.test.jsx | 6 +-- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/js/actions/recognize.js b/src/js/actions/recognize.js index 8562eeda..223a6bc2 100644 --- a/src/js/actions/recognize.js +++ b/src/js/actions/recognize.js @@ -182,7 +182,7 @@ const undoRetrieveMetadata = (itemKey, libraryKey) => { } const collections = item.collections; - await dispatch(updateItem(originalItemKey, { parentItem: null, collections }, libraryKey)); + await dispatch(updateItem(originalItemKey, { parentItem: false, collections }, libraryKey)); await dispatch(deleteItem(item)); dispatch({ type: COMPLETE_UNRECOGNIZE_DOCUMENT, diff --git a/src/js/reducers/libraries/items-top.js b/src/js/reducers/libraries/items-top.js index d6b50ef9..b0349e0d 100644 --- a/src/js/reducers/libraries/items-top.js +++ b/src/js/reducers/libraries/items-top.js @@ -1,15 +1,21 @@ import { indexByKey } from '../../utils'; -import { injectExtraItemKeys, filterItemKeys, populateItemKeys, sortItemKeysOrClear, -updateFetchingState } from '../../common/reducers'; +import { + injectExtraItemKeys, filterItemKeys, populateItemKeys, sortItemKeysOrClear, + updateFetchingState +} from '../../common/reducers'; import { DROP_TOP_ITEMS, ERROR_TOP_ITEMS, RECEIVE_CREATE_ITEM, RECEIVE_CREATE_ITEMS, RECEIVE_DELETE_ITEM, RECEIVE_DELETE_ITEMS, RECEIVE_DELETED_CONTENT, RECEIVE_FETCH_ITEMS, RECEIVE_MOVE_ITEMS_TRASH, - RECEIVE_RECOVER_ITEMS_TRASH, RECEIVE_TOP_ITEMS, REQUEST_TOP_ITEMS, SORT_ITEMS, + RECEIVE_RECOVER_ITEMS_TRASH, RECEIVE_TOP_ITEMS, RECEIVE_UPDATE_ITEM, RECEIVE_UPDATE_MULTIPLE_ITEMS, + REQUEST_TOP_ITEMS, SORT_ITEMS } from '../../constants/actions.js'; +// TODO: This currenlty only handles RECEIVE_FETCH_ITEMS, but logic for +// RECEIVE_UPDATE_ITEM and RECEIVE_UPDATE_MULTIPLE_ITEMS is very similar, so it +// should be refactored to use a common function const detectChangesInTop = (mappings, state, action, items) => { - if(!('keys' in state)) { + if (!('keys' in state)) { return state; } @@ -18,19 +24,19 @@ const detectChangesInTop = (mappings, state, action, items) => { const keysToRemove = []; action.items.forEach(item => { - if(!item.deleted && !item.parentItem && !newState.keys.includes(item.key)) { + if (!item.deleted && !item.parentItem && !newState.keys.includes(item.key)) { keysToInject.push(item.key); - } else if((item.deleted || item.parentItem) && newState.keys.includes(item.key)) { + } else if ((item.deleted || item.parentItem) && newState.keys.includes(item.key)) { keysToRemove.push(item.key); } }); - if(keysToInject.length > 0) { + if (keysToInject.length > 0) { const allItems = { ...items, ...indexByKey(action.items) }; newState = injectExtraItemKeys(mappings, newState, keysToInject, allItems); } - if(keysToRemove.length > 0) { + if (keysToRemove.length > 0) { newState = filterItemKeys(newState, keysToRemove) } @@ -38,9 +44,9 @@ const detectChangesInTop = (mappings, state, action, items) => { } const itemsTop = (state = {}, action, { items, meta }) => { - switch(action.type) { + switch (action.type) { case RECEIVE_CREATE_ITEM: - if(!action.item.parentItem) { + if (!action.item.parentItem) { return injectExtraItemKeys( meta.mappings, state, @@ -91,6 +97,41 @@ const itemsTop = (state = {}, action, { items, meta }) => { return detectChangesInTop(meta.mappings, state, action, items); case SORT_ITEMS: return sortItemKeysOrClear(meta.mappings, state, action.items, action.sortBy, action.sortDirection); + case RECEIVE_UPDATE_ITEM: { + const item = action.item; + if (!('keys' in state)) { + return state; + } + if (!('deleted' in action.patch || 'parentItem' in action.patch)) { + return state; + } + if (!items[action.item.key]) { + return state; + } + if (!item.deleted && !item.parentItem && !state.keys.includes(item.key)) { + return injectExtraItemKeys(meta.mappings, state, [item.key], items) + } else if ((item.deleted || item.parentItem) && state.keys.includes(item.key)) { + return filterItemKeys(state, [item.key]) + } + return state; + } + case RECEIVE_UPDATE_MULTIPLE_ITEMS: { + if (!('keys' in state)) { + return state; + } + const affectedItemsPatch = action.multiPatch.filter(patch => patch.key in items && ('deleted' in patch || 'parentItem' in patch)) + const patchedItems = affectedItemsPatch.map(patch => ({ ...items[patch.key], ...patch })) + const keysToInject = patchedItems.filter(i => !i.parentItem && !i.deleted).map(i => i.key).filter(k => !state.keys.includes(k)); + const keysToRemove = patchedItems.filter(i => i.parentItem || i.deleted).map(i => i.key).filter(k => state.keys.includes(k)); + let newState = { ...state }; + if (keysToInject.length > 0) { + newState = injectExtraItemKeys(meta.mappings, newState, keysToInject, items); + } + if (keysToRemove.length > 0) { + newState = filterItemKeys(newState, keysToRemove) + } + return newState; + } default: return state; } diff --git a/test/recognize.test.jsx b/test/recognize.test.jsx index 125e0cdb..971d7bfd 100644 --- a/test/recognize.test.jsx +++ b/test/recognize.test.jsx @@ -40,7 +40,7 @@ describe('Metadata Retrieval', () => { }), http.get('https://api.zotero.org/users/1/collections/CSB4KZUU/items/top/tags', () => { return HttpResponse.json([], { headers: { 'Total-Results': '0' } }); - }), + }), ]; const server = setupServer(...handlers) applyAdditionalJestTweaks(); @@ -114,7 +114,7 @@ describe('Metadata Retrieval', () => { expect(item.parentItem).toBe('S8CIV6VJ'); expect(item.collections).toEqual([]); } else { - expect(item.parentItem).toBeNull(); + expect(item.parentItem).toBe(false); expect(item.collections).toEqual(['CSB4KZUU']); } hasPatchedAttachmentItem = true; @@ -135,7 +135,7 @@ describe('Metadata Retrieval', () => { return { getRecognizerData: mockedGetRecognizerData }; }); - expect(screen.getByRole('row', { name: 'attention-is-all-you-need.pdf' }) ).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('row', { name: 'attention-is-all-you-need.pdf' })).toHaveAttribute('aria-selected', 'true'); const recognizeBtn = screen.getByRole('button', { name: 'Retrieve Metadata' } );