From 95be1e550a43d682426ea53b877145bce18aecab Mon Sep 17 00:00:00 2001 From: Urban Suppiger Date: Sun, 25 Jun 2023 11:34:30 -0400 Subject: [PATCH] chore: add filter to query for all dayResponsibles of a period (avoiding a separate network request for each day) --- api/src/Entity/DayResponsible.php | 2 +- api/src/Entity/Period.php | 13 +++ api/tests/Api/Periods/ReadPeriodTest.php | 2 + .../helpers/__tests__/dayResponsibles.spec.js | 102 ++++++++++++------ common/helpers/dayResponsibles.js | 20 +++- .../components/picasso/DayHeader.jsx | 14 ++- .../components/picasso/PicassoPage.jsx | 5 +- .../print-react/documents/campPrint/index.js | 10 +- print/components/PicassoChunk.vue | 7 +- print/components/PicassoPeriod.vue | 26 ++--- 10 files changed, 127 insertions(+), 74 deletions(-) diff --git a/api/src/Entity/DayResponsible.php b/api/src/Entity/DayResponsible.php index 26ccb0a3f4..a4acf15864 100644 --- a/api/src/Entity/DayResponsible.php +++ b/api/src/Entity/DayResponsible.php @@ -38,7 +38,7 @@ denormalizationContext: ['groups' => ['write']], normalizationContext: ['groups' => ['read']] )] -#[ApiFilter(filterClass: SearchFilter::class, properties: ['day'])] +#[ApiFilter(filterClass: SearchFilter::class, properties: ['day', 'day.period'])] #[UniqueEntity( fields: ['campCollaboration', 'day'], message: 'This campCollaboration (user) is already responsible for this day.', diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 9e8100c474..646f0be3ef 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -260,6 +260,19 @@ public function getContentNodes(): array { return []; } + /** + * A link to all the DayResponsibles in this period. + * The list is anyway replaced by a RelatedCollectionLink, thus we don't need to fetch the data now. + * + * @return DayResponsible[] + */ + #[ApiProperty(writable: false, example: '["/day_responsibles/1a2b3c4d"]')] + #[RelatedCollectionLink(DayResponsible::class, ['day.period' => '$this'])] + #[Groups(['read'])] + public function getDayResponsibles(): array { + return []; + } + /** * @return MaterialItem[] */ diff --git a/api/tests/Api/Periods/ReadPeriodTest.php b/api/tests/Api/Periods/ReadPeriodTest.php index 77d0312a5d..60835247e9 100644 --- a/api/tests/Api/Periods/ReadPeriodTest.php +++ b/api/tests/Api/Periods/ReadPeriodTest.php @@ -64,6 +64,8 @@ public function testGetSinglePeriodIsAllowedForGuest() { 'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()], 'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()], 'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()], + 'contentNodes' => ['href' => '/content_nodes?period=%2Fperiods%2F'.$period->getId()], + 'dayResponsibles' => ['href' => '/day_responsibles?day.period=%2Fperiods%2F'.$period->getId()], ], ]); } diff --git a/common/helpers/__tests__/dayResponsibles.spec.js b/common/helpers/__tests__/dayResponsibles.spec.js index 85e86d3c1f..b4e7774c0a 100644 --- a/common/helpers/__tests__/dayResponsibles.spec.js +++ b/common/helpers/__tests__/dayResponsibles.spec.js @@ -1,46 +1,82 @@ -import { dayResponsiblesCommaSeparated } from '../dayResponsibles' +import { + dayResponsiblesCommaSeparated, + filterDayResponsiblesByDay, +} from '../dayResponsibles' -describe('dayResponsibles', () => { - it('resolves camp collaboration with and without user', () => { - expect( - dayResponsiblesCommaSeparated( +const dayWith2Responsibles = { + _meta: { + self: '/day/1', + }, + period: () => ({ + dayResponsibles: () => ({ + items: [ { - dayResponsibles: () => ({ - items: [ - { - campCollaboration: () => ({ - inviteEmail: 'test@example.com', - }), - }, - { - campCollaboration: () => ({ - user: () => ({ - displayName: 'dummyUser', - }), - }), - }, - ], + campCollaboration: () => ({ + inviteEmail: 'test@example.com', + }), + day: () => ({ + _meta: { self: '/day/1' }, + }), + }, + { + campCollaboration: () => ({ + user: () => ({ + displayName: 'dummyUser', + }), + }), + day: () => ({ + _meta: { self: '/day/1' }, }), }, - null - ) - ).toEqual('test@example.com, dummyUser') - }) - - it('return empty string if no resonsibles', () => { - expect( - dayResponsiblesCommaSeparated( { - dayResponsibles: () => ({ - items: [], + campCollaboration: () => ({ + user: () => ({ + displayName: 'responsibleUserOnAnotherDay', + }), + }), + day: () => ({ + _meta: { self: '/day/2' }, }), }, - null - ) - ).toEqual('') + ], + }), + }), +} + +const dayWithoutResponsibles = { + period: () => ({ + dayResponsibles: () => ({ + items: [], + }), + }), +} + +describe('dayResponsiblesCommaSeparated', () => { + it('resolves camp collaboration with and without user', () => { + expect(dayResponsiblesCommaSeparated(dayWith2Responsibles, null)).toEqual( + 'test@example.com, dummyUser' + ) + }) + + it('return empty string if no responsibles', () => { + expect(dayResponsiblesCommaSeparated(dayWithoutResponsibles, null)).toEqual('') }) it('return empty string for null object', () => { expect(dayResponsiblesCommaSeparated(null, null)).toEqual('') }) }) + +describe('filterDayResponsiblesByDay', () => { + it('resolves camp collaboration with and without user', () => { + expect(filterDayResponsiblesByDay(dayWith2Responsibles).length).toEqual(2) + }) + + it('return empty string if no responsibles', () => { + expect(filterDayResponsiblesByDay(dayWithoutResponsibles)).toEqual([]) + }) + + it('return empty string for null object', () => { + expect(filterDayResponsiblesByDay(null)).toEqual([]) + }) +}) diff --git a/common/helpers/dayResponsibles.js b/common/helpers/dayResponsibles.js index 8c5bf0d2b2..43fb3f0a47 100644 --- a/common/helpers/dayResponsibles.js +++ b/common/helpers/dayResponsibles.js @@ -1,14 +1,26 @@ import campCollaborationDisplayName from './campCollaborationDisplayName.js' -const dayResponsiblesCommaSeparated = (day, tc) => { - if (!day) return '' +/** + * Local filtering of dayResponsibles by day + * (avoids separate network request for each day) + */ +const filterDayResponsiblesByDay = (day) => { + if (!day) return [] return day + .period() .dayResponsibles() - .items.map((dayResponsible) => + .items.filter((dayResponsible) => dayResponsible.day()._meta.self === day._meta.self) +} + +const dayResponsiblesCommaSeparated = (day, tc) => { + if (!day) return '' + + return filterDayResponsiblesByDay(day) + .map((dayResponsible) => campCollaborationDisplayName(dayResponsible.campCollaboration(), tc) ) .join(', ') } -export { dayResponsiblesCommaSeparated } +export { filterDayResponsiblesByDay, dayResponsiblesCommaSeparated } diff --git a/frontend/src/components/print/print-react/components/picasso/DayHeader.jsx b/frontend/src/components/print/print-react/components/picasso/DayHeader.jsx index ba405266e6..0d4f989ed2 100644 --- a/frontend/src/components/print/print-react/components/picasso/DayHeader.jsx +++ b/frontend/src/components/print/print-react/components/picasso/DayHeader.jsx @@ -3,21 +3,19 @@ import React from 'react' import { Text, View } from '@react-pdf/renderer' import dayjs from '@/common/helpers/dayjs.js' import picassoStyles from './picassoStyles.js' -import campCollaborationDisplayName from '../../../../../common/helpers/campCollaborationDisplayName.js' +import { + dayResponsiblesCommaSeparated, + filterDayResponsiblesByDay, +} from '../../../../../common/helpers/dayResponsibles.js' function renderDate(day) { return dayjs.utc(day.start).hour(0).minute(0).second(0).format('ddd LL') } function dayResponsibles(day, $tc) { - const responsibles = day.dayResponsibles().items - if (responsibles.length === 0) return '' + if (filterDayResponsiblesByDay(day).length === 0) return '' const label = $tc('entity.day.fields.dayResponsibles') - const displayNames = responsibles - .map((responsible) => - campCollaborationDisplayName(responsible.campCollaboration(), $tc) - ) - .join(', ') + const displayNames = dayResponsiblesCommaSeparated(day, $tc) return `${label}: ${displayNames}` } diff --git a/frontend/src/components/print/print-react/components/picasso/PicassoPage.jsx b/frontend/src/components/print/print-react/components/picasso/PicassoPage.jsx index da416c6914..4461554877 100644 --- a/frontend/src/components/print/print-react/components/picasso/PicassoPage.jsx +++ b/frontend/src/components/print/print-react/components/picasso/PicassoPage.jsx @@ -10,6 +10,7 @@ import DayHeader from './DayHeader.jsx' import PicassoFooter from './PicassoFooter.jsx' import YSLogo from './YSLogo.jsx' import Categories from './Categories.jsx' +import { filterDayResponsiblesByDay } from '../../../../../common/helpers/dayResponsibles.js' /** * Generates an array of time row descriptions, used for labeling the vertical axis of the picasso. @@ -38,7 +39,9 @@ function PicassoPage(props) { const period = props.period const days = props.days const orientation = props.content.options.orientation - const anyDayResponsibles = days.some((day) => day.dayResponsibles().items.length > 0) + const anyDayResponsibles = days.some( + (day) => filterDayResponsiblesByDay(day).length > 0 + ) const scheduleEntries = period.scheduleEntries().items const times = generateTimes({ getUpTime: props.getUpTime, diff --git a/frontend/src/components/print/print-react/documents/campPrint/index.js b/frontend/src/components/print/print-react/documents/campPrint/index.js index 693adb36fd..f9c73c745d 100644 --- a/frontend/src/components/print/print-react/documents/campPrint/index.js +++ b/frontend/src/components/print/print-react/documents/campPrint/index.js @@ -35,14 +35,8 @@ const picassoData = (config) => { return Promise.all([ period.scheduleEntries().$loadItems(), period.contentNodes().$loadItems(), - period - .days() - .$loadItems() - .then((days) => { - return Promise.all( - days.items.map((day) => day.dayResponsibles().$loadItems()) - ) - }), + period.days().$loadItems(), + period.dayResponsibles().$loadItems(), ]) }) ) diff --git a/print/components/PicassoChunk.vue b/print/components/PicassoChunk.vue index c4f0b87e9e..f60284e968 100644 --- a/print/components/PicassoChunk.vue +++ b/print/components/PicassoChunk.vue @@ -126,7 +126,10 @@