From 55dc14a4f1bf1bfca4ea40b4848c8e73fc3a3640 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sat, 30 May 2026 01:03:41 +0200 Subject: [PATCH] Feat/issue 7848 d0002a (#7852) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(e2e): harden section reorder drag against bounding-box races boundingBox() is a one-shot snapshot that returns null when the element has no layout box — which happens after every CDK drop, since the section task-list re-renders (@for track reorders nodes) and freshly-dropped tasks run the expandInOnly enter animation at height:0. Add a stableBoundingBox() helper that waits for visibility then polls for a real, non-null box before driving the pointer gesture, and exclude .ng-animating nodes when picking the reorder source/target (matching the existing 'before' capture). Test-only change; reorder behavior itself is covered by section.reducer unit tests. * fix(calendar): dedupe duplicate-UID iCal events Some providers (Google/Outlook) emit the same UID twice for invited or modified events — once fully populated, once sparse. These are plain VEVENTs (no RRULE, no RECURRENCE-ID), so per RFC 5545 they are the same event, yet they rendered as two identical entries in the schedule overlay. The prior fix for #3837 only deduplicated recurring-series RECURRENCE-ID overrides; plain duplicate-UID copies were never collapsed. Collapse events sharing a resolved id at the end of the iCal parser, keeping the richer copy (the one carrying a description/url). Recurring occurrences are unaffected — each carries a unique `_` id. The pass runs before Google-URL synthesis so the tie-break compares only feed-provided data. The no-duplicate common case returns the original array unchanged. Tie-break uses field "richness", not RFC SEQUENCE: the model carries no SEQUENCE and the observed duplicates share a revision (SEQUENCE would tie). A test documents this decision. Refs #7848, #3837 --- e2e/tests/work-view/sections.spec.ts | 44 ++- .../get-relevant-events-from-ical.spec.ts | 257 ++++++++++++++++++ .../ical/get-relevant-events-from-ical.ts | 38 +++ 3 files changed, 333 insertions(+), 6 deletions(-) diff --git a/e2e/tests/work-view/sections.spec.ts b/e2e/tests/work-view/sections.spec.ts index 6f2a09ecfc..7b2593440f 100644 --- a/e2e/tests/work-view/sections.spec.ts +++ b/e2e/tests/work-view/sections.spec.ts @@ -69,6 +69,37 @@ test.describe('Sections', () => { ): ReturnType => page.locator('.section-container').filter({ hasText: title }); + /** + * Read a locator's bounding box defensively. `boundingBox()` takes a + * one-shot snapshot and returns `null` whenever the element has no + * layout box at that instant — which happens constantly here because + * the section task-list re-renders after every CDK drop (Angular's + * `@for track` reorders nodes) and freshly-dropped tasks run an + * `expandInOnly` enter animation that holds them at `height: 0`. Wait + * for the element to be visible, then poll until it reports a real box + * so the read can't race the re-render. This is the failure the two + * `test.fixme`s below hit; keeping the guard here lets the active tests + * stay reliable instead of throwing "no bounding box". + */ + const stableBoundingBox = async ( + locator: import('@playwright/test').Locator, + ): Promise<{ x: number; y: number; width: number; height: number }> => { + await locator.waitFor({ state: 'visible', timeout: 10000 }); + await expect + .poll( + async () => { + const b = await locator.boundingBox(); + return !!b && b.width > 0 && b.height > 0; + }, + { timeout: 10000 }, + ) + .toBe(true); + // The poll guarantees a real box exists; re-read it for the gesture. + const box = await locator.boundingBox(); + if (!box) throw new Error('drag source/target has no bounding box'); + return box; + }; + /** * CDK drag-drop is event-driven via Angular CDK's own pointer-event * handling. Playwright's `dragTo` uses HTML5 drag-and-drop events, @@ -80,9 +111,8 @@ test.describe('Sections', () => { source: import('@playwright/test').Locator, target: import('@playwright/test').Locator, ): Promise => { - const sBox = await source.boundingBox(); - const tBox = await target.boundingBox(); - if (!sBox || !tBox) throw new Error('drag source/target has no bounding box'); + const sBox = await stableBoundingBox(source); + const tBox = await stableBoundingBox(target); /* eslint-disable no-mixed-operators */ const sx = sBox.x + sBox.width / 2; const sy = sBox.y + sBox.height / 2; @@ -293,9 +323,11 @@ test.describe('Sections', () => { expect(before.length).toBe(3); // Drag the first task onto the last task. After the drop, the first - // task should no longer be at index 0. - const firstTask = section.locator('task').nth(0); - const lastTask = section.locator('task').nth(2); + // task should no longer be at index 0. Exclude `.ng-animating` nodes + // (matching the `before` capture) so we never target a task that's + // still running its enter animation at `height: 0`. + const firstTask = section.locator('task:not(.ng-animating)').nth(0); + const lastTask = section.locator('task:not(.ng-animating)').nth(2); await cdkDragTo(page, firstTask.locator('done-toggle').first(), lastTask); await expect diff --git a/src/app/features/schedule/ical/get-relevant-events-from-ical.spec.ts b/src/app/features/schedule/ical/get-relevant-events-from-ical.spec.ts index 331c575b50..77f3ad9087 100644 --- a/src/app/features/schedule/ical/get-relevant-events-from-ical.spec.ts +++ b/src/app/features/schedule/ical/get-relevant-events-from-ical.spec.ts @@ -892,4 +892,261 @@ END:VCALENDAR`; }).not.toThrow(); }); }); + + describe('duplicate UID handling (#7848)', () => { + // Real-world shape from #3837: a provider (Google/Outlook) emits the same + // UID twice for an invited/modified event — one copy sparse, one with a + // DESCRIPTION. Both are plain (no RRULE, no RECURRENCE-ID), so without + // dedup they render as two identical overlay entries. + it('collapses two plain VEVENTs sharing a UID, keeping the copy with a description', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:invited-event@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Invited Meeting +END:VEVENT +BEGIN:VEVENT +UID:invited-event@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +DESCRIPTION:full details here +SUMMARY:Invited Meeting +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(1); + expect(events[0].description).toBe('full details here'); + }); + + it('keeps the description-bearing copy regardless of VEVENT order', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:invited-event@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +DESCRIPTION:full details here +SUMMARY:Invited Meeting +END:VEVENT +BEGIN:VEVENT +UID:invited-event@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Invited Meeting +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(1); + expect(events[0].description).toBe('full details here'); + }); + + it('does NOT collapse distinct events that merely share a start time', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:event-a@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Meeting A +END:VEVENT +BEGIN:VEVENT +UID:event-b@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Meeting B +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(2); + }); + + it('does NOT collapse occurrences of a recurring series (distinct ids preserved)', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:daily@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +RRULE:FREQ=DAILY;COUNT=3 +SUMMARY:Daily Standup +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(3); + }); + + it('collapses three or more copies of the same UID into the richest one', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:triple@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Triple +END:VEVENT +BEGIN:VEVENT +UID:triple@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +DESCRIPTION:the real one +SUMMARY:Triple +END:VEVENT +BEGIN:VEVENT +UID:triple@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Triple +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(1); + expect(events[0].description).toBe('the real one'); + }); + + // Two plain VEVENTs sharing a UID but with DIFFERENT start times. Per RFC 5545 + // a UID without a distinct RECURRENCE-ID is the SAME event, so we collapse them. + // On a richness tie (both sparse) the first-seen copy wins — documenting that we + // keep the earlier-listed slot and intentionally drop the other. + it('collapses same-UID copies with different start times (first-seen wins on a tie)', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:moved@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SUMMARY:Moved +END:VEVENT +BEGIN:VEVENT +UID:moved@example.com +DTSTART:20250115T140000Z +DTEND:20250115T150000Z +SUMMARY:Moved +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(1); + expect(events[0].start).toBe(new Date('2025-01-15T10:00:00Z').getTime()); + }); + + // DECISION GUARD: the tie-breaker is "richness" (has description/url), NOT the + // RFC 5545 SEQUENCE. The model carries no SEQUENCE, and the observed duplicates + // share a revision (one full, one sparse), so SEQUENCE would tie anyway. This + // test pins that choice: the populated copy wins even though the sparse copy has + // a HIGHER SEQUENCE. If a real feed ever needs SEQUENCE precedence, this test is + // where that decision must change. + it('keeps the populated copy over a higher-SEQUENCE sparse copy (richness, not SEQUENCE)', async () => { + const icalData = `BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Test//Test//EN +BEGIN:VEVENT +UID:seq@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SEQUENCE:5 +SUMMARY:Seq Test +END:VEVENT +BEGIN:VEVENT +UID:seq@example.com +DTSTART:20250115T100000Z +DTEND:20250115T110000Z +SEQUENCE:0 +DESCRIPTION:populated but lower sequence +SUMMARY:Seq Test +END:VEVENT +END:VCALENDAR`; + + const events = await getRelevantEventsForCalendarIntegrationFromIcal( + icalData, + calProviderId, + startTimestamp, + endTimestamp, + ); + + expect(events.length).toBe(1); + expect(events[0].description).toBe('populated but lower sequence'); + }); + + // A plain VEVENT that shares a UID with a recurring series but has no + // RECURRENCE-ID gets id ``, while occurrences get `_