Skip to content

Commit

Permalink
fix: request full entities from API in item pinning modal (#2303)
Browse files Browse the repository at this point in the history
  • Loading branch information
rwd authored May 7, 2024
1 parent 0e04ae7 commit 4bd6e38
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 45 deletions.
5 changes: 4 additions & 1 deletion packages/portal/src/components/item/ItemPinButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
v-if="identifier && entities.length > 0"
:identifier="identifier"
:modal-id="pinModalId"
:entities="entities"
:entity-uris="entityUris"
data-qa="pin item to entities modal"
/>
</div>
Expand Down Expand Up @@ -102,6 +102,9 @@
},
computed: {
entityUris() {
return this.entities.map((entity) => entity.about);
},
pinned() {
return this.$store.getters['entity/isPinned'](this.identifier);
},
Expand Down
32 changes: 15 additions & 17 deletions packages/portal/src/components/item/ItemPinModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
v-for="(entity, index) in entities"
:key="index"
:disabled="!fetched"
:pressed="selected === entity.about"
:pressed="selected === entity.id"
:data-qa="`pin item to entity choice`"
class="btn-collection w-100 text-left d-flex"
@click="selectEntity(entity.about)"
@click="selectEntity(entity.id)"
>
<span
class="mr-auto"
Expand All @@ -26,11 +26,11 @@
class="icons text-left d-flex justify-content-end"
>
<span
v-if="selected === entity.about"
v-if="selected === entity.id"
class="icon-check-circle d-inline-flex ml-auto"
/>
<span
v-if="pinnedTo(entity.about)"
v-if="pinnedTo(entity.id)"
class="icon-pin d-inline-flex"
/>
</span>
Expand Down Expand Up @@ -92,14 +92,9 @@
required: true
},
/**
* Entities associated with the item, to which it may be pinned
* @example
* [
* { about: 'entityUri1', prefLabel: { en: 'entity en label 1' } },
* { about: 'entityUri2', prefLabel: { en: 'entity en label 2' } }
* ]
* URIs of entities to which item may be pinned
*/
entities: {
entityUris: {
type: Array,
required: true
},
Expand All @@ -118,6 +113,7 @@
data() {
return {
entities: [],
fetched: false,
selected: null,
/**
Expand All @@ -128,10 +124,7 @@
* 'entityUri2': { id: 'setId2', pinned: ['itemUri1', 'itemUri2'] },
* }
*/
sets: this.entities.reduce((memo, entity) => {
memo[entity.about] = this.setFactory();
return memo;
}, {})
sets: []
};
},
Expand Down Expand Up @@ -161,7 +154,7 @@
return this.selectedEntityPrefLabel?.values?.[0];
},
selectedEntity() {
return this.entities.find(entity => entity.about === this.selected);
return this.entities.find(entity => entity.id === this.selected);
},
selectedEntitySet() {
return this.sets[this.selected];
Expand All @@ -181,13 +174,18 @@
return;
}
// Fetch the full entities first
this.entities = await this.$apis.entity.find(this.entityUris, {
fl: 'skos_prefLabel.*'
});
const searchParams = {
query: 'type:EntityBestItemsSet',
profile: 'minimal',
pageSize: 1
};
await Promise.all(this.entities.map(async(entity) => {
const entityUri = entity.about;
const entityUri = entity.id;
// TODO: "OR" the ids to avoid multiple requests, but doesn't seem supported.
const searchResponse = await this.$apis.set.search({
...searchParams,
Expand Down
10 changes: 0 additions & 10 deletions packages/portal/src/plugins/europeana/record.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,16 +230,6 @@ export default class EuropeanaRecordApi extends EuropeanaApi {
const parsed = this.parseRecordDataFromApiResponse(response.data, options);
const reduced = reduceLangMapsForLocale(parsed, parsed.metadataLanguage || options.locale, { freeze: false });

// Restore `en` prefLabel on entities, e.g. for use in EntityBestItemsSet-type sets
for (const entityType of ['agents', 'concepts', 'organizations', 'places', 'timespans']) {
for (const reducedEntity of (reduced[entityType] || [])) {
const fullEntity = parsed[entityType].find(entity => entity.about === reducedEntity.about);
if (fullEntity.prefLabel?.en !== reducedEntity.prefLabel?.en) {
reducedEntity.prefLabel.en = fullEntity.prefLabel.en;
}
}
}

return {
record: reduced,
error: null
Expand Down
61 changes: 44 additions & 17 deletions packages/portal/tests/unit/components/item/ItemPinModal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ const setGetApiResponseWithPinnedItem = {
items: ['http://data.europeana.eu/item/123/abc']
};

const entityApiFindResponse = [
{
id: ENTITY_URI,
prefLabel: { en: ['Agent entity'] }
},
{
id: 'http://data.europeana.eu/topic/123',
prefLabel: { en: ['Topic entity'] }
},
{
id: 'http://data.europeana.eu/organisation/123456789',
prefLabel: { en: ['Organisation entity'] }
}
];

const fullPins = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24'];

const fixtures = {
Expand Down Expand Up @@ -92,6 +107,7 @@ const fixtures = {
}
};

const entityApiFindStub = sinon.stub().resolves(entityApiFindResponse);
const setApiGetStub = sinon.stub().resolves(setGetApiResponseWithPinnedItem);
const setApiSearchStub = sinon.stub().resolves({});
const setApiCreateStub = sinon.stub().resolves({ id: '457' });
Expand All @@ -103,33 +119,31 @@ const factory = ({ propsData, data } = {}) => mount(ItemPinModal, {
identifier: '/123/abc',
modalStatic: true,
modalId: 'pin-modal-/123/abc',
entities: [
{
about: ENTITY_URI,
prefLabel: { en: ['Agent entity'] }
},
{
about: 'http://data.europeana.eu/topic/123',
prefLabel: { en: ['Topic entity'] }
},
{
about: 'http://data.europeana.eu/organisation/123456789',
prefLabel: { en: ['Organisation entity'] }
}
entityUris: [
ENTITY_URI,
'http://data.europeana.eu/topic/123',
'http://data.europeana.eu/organisation/123456789'
],
...propsData
},
data,
mocks: {
localePath: () => {},
$apis: {
entity: {
find: entityApiFindStub
},
set: {
get: setApiGetStub,
search: setApiSearchStub,
create: setApiCreateStub,
modifyItems: setApiModifyItemsStub
}
},
$error: (error) => {
console.error(error);
throw error;
},
$i18n: { locale: 'en' },
$store: {
commit: sinon.spy(),
Expand All @@ -156,15 +170,17 @@ describe('components/item/ItemPinModal', () => {
});

describe('option buttons', () => {
it('show a button for each entity option', async() => {
it('shows a button for each entity option', async() => {
const wrapper = factory();
await wrapper.vm.fetchEntityBestItemsSets();

expect(wrapper.findAll('button[data-qa="pin item to entity choice"]').length).toEqual(3);
});

describe('when an option is selected', () => {
it('shows the check icon on the selected option', async() => {
const wrapper = factory();
await wrapper.vm.fetchEntityBestItemsSets();
await wrapper.setData({
selected: ENTITY_URI
});
Expand All @@ -174,8 +190,9 @@ describe('components/item/ItemPinModal', () => {
});
});
describe('when an option is pinned', () => {
it('shows the pin icon on the pinned option', () => {
it('shows the pin icon on the pinned option', async() => {
const wrapper = factory(fixtures.itemAlreadyPinned);
await wrapper.vm.fetchEntityBestItemsSets();

const button = wrapper.find('button[data-qa="pin item to entity choice"]');

Expand Down Expand Up @@ -295,6 +312,7 @@ describe('components/item/ItemPinModal', () => {
describe('when there is NO existing set', () => {
it('creates a set and pins the item, updates the store', async() => {
const wrapper = factory(fixtures.setDoesNotExist);
await wrapper.vm.fetchEntityBestItemsSets();

await wrapper.find('[data-qa="toggle pin button"]').trigger('click');
await new Promise(process.nextTick);
Expand Down Expand Up @@ -373,6 +391,15 @@ describe('components/item/ItemPinModal', () => {
setApiSearchStub.resolves(setSearchApiResponse);
});

it('fetches and stores the entities from the Entity API', async() => {
const wrapper = factory();

await wrapper.vm.fetchEntityBestItemsSets();

expect(entityApiFindStub.called).toBe(true);
expect(wrapper.vm.entities).toEqual(entityApiFindResponse);
});

it('iterates over all entityIds and searches the set API for relevant sets', async() => {
const wrapper = factory();

Expand Down Expand Up @@ -561,7 +588,7 @@ describe('components/item/ItemPinModal', () => {
describe('entityDisplayLabel', () => {
describe('when there is an english prefLabel', () => {
const entityWithEnglishPrefLabel = {
about: ENTITY_URI,
id: ENTITY_URI,
prefLabel: { en: ['English label', 'another Label'], fr: ['French label'] }
};
it('uses the first english pref label value', async() => {
Expand All @@ -575,7 +602,7 @@ describe('components/item/ItemPinModal', () => {

describe('when there is NO english prefLabel', () => {
const entityWithFrenchPrefLabel = {
about: ENTITY_URI,
id: ENTITY_URI,
prefLabel: { fr: ['French label'] }
};
it('uses the first prefLabel value of an available language', async() => {
Expand Down

0 comments on commit 4bd6e38

Please sign in to comment.