From 3f2a8036c6ede90a0f92205d10e8a90a04d9b13f Mon Sep 17 00:00:00 2001 From: Fabian Stoehr Date: Tue, 30 Jul 2024 13:09:46 +0200 Subject: [PATCH 1/5] fix:mirador_crash --- src/containers/ManifestListItem.js | 2 +- src/state/selectors/manifests.js | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/containers/ManifestListItem.js b/src/containers/ManifestListItem.js index 55f5c43e90..6c6f5dddd8 100644 --- a/src/containers/ManifestListItem.js +++ b/src/containers/ManifestListItem.js @@ -24,7 +24,7 @@ const mapStateToProps = (state, { manifestId, provider }) => { : getCanvases(state, { manifestId }).length; return { active: getWindowManifests(state).includes(manifestId), - error: manifest.error, + error: manifest.error || !manifesto, isCollection, isFetching: manifest.isFetching, isMultipart: isCollection diff --git a/src/state/selectors/manifests.js b/src/state/selectors/manifests.js index 6fc48fc15b..8e5863d084 100644 --- a/src/state/selectors/manifests.js +++ b/src/state/selectors/manifests.js @@ -11,11 +11,14 @@ import { getConfig } from './config'; function createManifestoInstance(json, locale) { if (!json) return undefined; const manifestoObject = Utils.parseManifest(json, locale ? { locale } : undefined); - // Local patching of Manifesto so that when its a Collection, it behaves similarly - if (typeof manifestoObject.getSequences != 'function') { - manifestoObject.getSequences = () => []; + if (manifestoObject) { + // Local patching of Manifesto so that when its a Collection, it behaves similarly + if (typeof manifestoObject.getSequences != 'function') { + manifestoObject.getSequences = () => []; + } + return manifestoObject; } - return manifestoObject; + return undefined; } /** */ From f5d8cf2ec427da836462311b81c81102b8960ac7 Mon Sep 17 00:00:00 2001 From: Fabian Stoehr Date: Mon, 5 Aug 2024 13:02:00 +0200 Subject: [PATCH 2/5] add tests for empty fetched manifest --- __tests__/src/components/ManifestListItem.test.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/__tests__/src/components/ManifestListItem.test.js b/__tests__/src/components/ManifestListItem.test.js index bd444de915..385b23a3ef 100644 --- a/__tests__/src/components/ManifestListItem.test.js +++ b/__tests__/src/components/ManifestListItem.test.js @@ -1,6 +1,6 @@ import { render, screen } from 'test-utils'; import userEvent from '@testing-library/user-event'; - +import { getManifestoInstance } from '../../../src/state/selectors'; import { ManifestListItem } from '../../../src/components/ManifestListItem'; /** */ @@ -47,6 +47,16 @@ describe('ManifestListItem', () => { expect(screen.getByText('manifestError')).toBeInTheDocument(); expect(screen.getByText('http://example.com')).toBeInTheDocument(); }); + it('renders an error message when fetched manifest is empty', () => { + const state = { manifests: { x: { json: {} } } }; + const manifesto = getManifestoInstance(state, { manifestId: 'x' }); + + createWrapper({ error: !manifesto }); + + expect(screen.getByText('manifestError')).toBeInTheDocument(); + expect(screen.getByText('http://example.com')).toBeInTheDocument(); + }); + it('updates and adds window when button clicked', async () => { const user = userEvent.setup(); const addWindow = jest.fn(); From b3c5812805877715f534c71072eaa9b7c4579e8a Mon Sep 17 00:00:00 2001 From: Fabian Stoehr Date: Thu, 19 Dec 2024 14:57:39 +0100 Subject: [PATCH 3/5] test: add new test createManifestoInstance with Collection --- .../src/components/ManifestListItem.test.js | 1 + __tests__/src/selectors/manifests.test.js | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/__tests__/src/components/ManifestListItem.test.js b/__tests__/src/components/ManifestListItem.test.js index b2a1bd4c58..ec90f93e7f 100644 --- a/__tests__/src/components/ManifestListItem.test.js +++ b/__tests__/src/components/ManifestListItem.test.js @@ -46,6 +46,7 @@ describe('ManifestListItem', () => { expect(screen.getByText('The resource cannot be added:')).toBeInTheDocument(); expect(screen.getByText('http://example.com')).toBeInTheDocument(); }); + it('renders an error message when fetched manifest is empty', () => { const state = { manifests: { x: { json: {} } } }; const manifesto = getManifestoInstance(state, { manifestId: 'x' }); diff --git a/__tests__/src/selectors/manifests.test.js b/__tests__/src/selectors/manifests.test.js index 1cf9bf50a3..63a3fff05c 100644 --- a/__tests__/src/selectors/manifests.test.js +++ b/__tests__/src/selectors/manifests.test.js @@ -1,4 +1,6 @@ import { Utils } from 'manifesto.js'; +import { vi } from 'vitest'; + import manifestFixture001 from '../../fixtures/version-2/001.json'; import manifestFixture002 from '../../fixtures/version-2/002.json'; import manifestFixture019 from '../../fixtures/version-2/019.json'; @@ -7,6 +9,7 @@ import manifestFixturev3001 from '../../fixtures/version-3/001.json'; import manifestFixtureWithAProvider from '../../fixtures/version-3/with_a_provider.json'; import manifestFixtureFg165hz3589 from '../../fixtures/version-2/fg165hz3589.json'; import manifestFixtureRelated from '../../fixtures/version-2/related.json'; + import { getManifestoInstance, getManifestLocale, @@ -59,6 +62,19 @@ describe('getManifestoInstance', () => { const received = getManifestoInstance(state, { manifestId: 'x' }); expect(received.id).toEqual('http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json'); }); + it('creates a manifesto instance and patches getSequences if not defined', () => { + const state = { manifests: { x: { json: manifestFixture019 } } }; + vi.spyOn(Utils, 'parseManifest').mockImplementation((json) => ({ + id: 'http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json', + })); + + const received = getManifestoInstance(state, { manifestId: 'x' }); + + expect(received.id).toEqual('http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json'); + + expect(typeof received.getSequences).toBe('function'); + expect(received.getSequences()).toEqual([]); + }); it('is cached based off of input props', () => { const state = { manifests: { x: { json: manifestFixture019 } } }; const received = getManifestoInstance(state, { manifestId: 'x' }); From 22c713ba6726df7fefd4038a44dae93c79abd232 Mon Sep 17 00:00:00 2001 From: Fabian Stoehr Date: Thu, 19 Dec 2024 15:15:15 +0100 Subject: [PATCH 4/5] test: use collection fixture --- __tests__/src/selectors/manifests.test.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/__tests__/src/selectors/manifests.test.js b/__tests__/src/selectors/manifests.test.js index 63a3fff05c..21fd06802b 100644 --- a/__tests__/src/selectors/manifests.test.js +++ b/__tests__/src/selectors/manifests.test.js @@ -1,5 +1,4 @@ import { Utils } from 'manifesto.js'; -import { vi } from 'vitest'; import manifestFixture001 from '../../fixtures/version-2/001.json'; import manifestFixture002 from '../../fixtures/version-2/002.json'; @@ -9,6 +8,7 @@ import manifestFixturev3001 from '../../fixtures/version-3/001.json'; import manifestFixtureWithAProvider from '../../fixtures/version-3/with_a_provider.json'; import manifestFixtureFg165hz3589 from '../../fixtures/version-2/fg165hz3589.json'; import manifestFixtureRelated from '../../fixtures/version-2/related.json'; +import collection from '../../fixtures/version-2/collection.json'; import { getManifestoInstance, @@ -62,17 +62,12 @@ describe('getManifestoInstance', () => { const received = getManifestoInstance(state, { manifestId: 'x' }); expect(received.id).toEqual('http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json'); }); - it('creates a manifesto instance and patches getSequences if not defined', () => { - const state = { manifests: { x: { json: manifestFixture019 } } }; - vi.spyOn(Utils, 'parseManifest').mockImplementation((json) => ({ - id: 'http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json', - })); + it('creates a manifesto collection instance and patches getSequences', () => { + const state = { manifests: { x: { json: collection } } }; const received = getManifestoInstance(state, { manifestId: 'x' }); - - expect(received.id).toEqual('http://iiif.io/api/presentation/2.1/example/fixtures/19/manifest.json'); - expect(typeof received.getSequences).toBe('function'); + expect(received.getSequences()).toEqual([]); }); it('is cached based off of input props', () => { From 13c7a4a35cbd3b8b574a76692f2f600fe6129153 Mon Sep 17 00:00:00 2001 From: Fabian Stoehr Date: Thu, 19 Dec 2024 15:15:43 +0100 Subject: [PATCH 5/5] chore: sort import --- __tests__/src/selectors/manifests.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/src/selectors/manifests.test.js b/__tests__/src/selectors/manifests.test.js index 21fd06802b..b404ddd124 100644 --- a/__tests__/src/selectors/manifests.test.js +++ b/__tests__/src/selectors/manifests.test.js @@ -1,5 +1,6 @@ import { Utils } from 'manifesto.js'; +import collection from '../../fixtures/version-2/collection.json'; import manifestFixture001 from '../../fixtures/version-2/001.json'; import manifestFixture002 from '../../fixtures/version-2/002.json'; import manifestFixture019 from '../../fixtures/version-2/019.json'; @@ -8,7 +9,6 @@ import manifestFixturev3001 from '../../fixtures/version-3/001.json'; import manifestFixtureWithAProvider from '../../fixtures/version-3/with_a_provider.json'; import manifestFixtureFg165hz3589 from '../../fixtures/version-2/fg165hz3589.json'; import manifestFixtureRelated from '../../fixtures/version-2/related.json'; -import collection from '../../fixtures/version-2/collection.json'; import { getManifestoInstance,