mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-22 07:28:14 +00:00
Feat/issue 7848 d0002a (#7852)
* 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 `<uid>_<occurrence>` 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
This commit is contained in:
parent
2baf5fc512
commit
55dc14a4f1
3 changed files with 333 additions and 6 deletions
|
|
@ -69,6 +69,37 @@ test.describe('Sections', () => {
|
|||
): ReturnType<import('@playwright/test').Page['locator']> =>
|
||||
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<void> => {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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 `<uid>`, while occurrences get `<uid>_<time>` — distinct
|
||||
// ids, so dedup must NOT fold the plain event into the series.
|
||||
it('does NOT fold a plain VEVENT into a recurring series with the same UID', async () => {
|
||||
const icalData = `BEGIN:VCALENDAR
|
||||
VERSION:2.0
|
||||
PRODID:-//Test//Test//EN
|
||||
BEGIN:VEVENT
|
||||
UID:mixed@example.com
|
||||
DTSTART:20250115T100000Z
|
||||
DTEND:20250115T110000Z
|
||||
RRULE:FREQ=DAILY;COUNT=3
|
||||
SUMMARY:Mixed Series
|
||||
END:VEVENT
|
||||
BEGIN:VEVENT
|
||||
UID:mixed@example.com
|
||||
DTSTART:20250120T160000Z
|
||||
DTEND:20250120T170000Z
|
||||
SUMMARY:Mixed One-Off
|
||||
END:VEVENT
|
||||
END:VCALENDAR`;
|
||||
|
||||
const events = await getRelevantEventsForCalendarIntegrationFromIcal(
|
||||
icalData,
|
||||
calProviderId,
|
||||
startTimestamp,
|
||||
endTimestamp,
|
||||
);
|
||||
|
||||
// 3 recurring occurrences + 1 standalone plain event = 4
|
||||
expect(events.length).toBe(4);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -281,6 +281,17 @@ export const getRelevantEventsForCalendarIntegrationFromIcal = async (
|
|||
});
|
||||
// Log.timeEnd('TEST');
|
||||
|
||||
// Collapse events that resolve to the same id. Per RFC 5545 a UID (without a
|
||||
// distinct RECURRENCE-ID) identifies a single event, but some providers
|
||||
// (Google/Outlook) emit the same UID twice for invited/modified events — once
|
||||
// fully populated and once sparse — which surfaced as duplicated overlay
|
||||
// entries (#7848, #3837). Recurring occurrences are unaffected because each
|
||||
// carries a unique `<uid>_<occurrence>` id.
|
||||
// Must run BEFORE the Google-URL synthesis below so the richness tie-break
|
||||
// compares only feed-provided data (a generated URL would otherwise inflate
|
||||
// both copies equally) and we synthesize at most one URL per surviving event.
|
||||
calendarIntegrationEvents = dedupeCalendarEventsById(calendarIntegrationEvents);
|
||||
|
||||
// Generate URLs for Google Calendar events that don't already have one
|
||||
if (icalUrl) {
|
||||
const googleEmail = getGoogleCalendarEmail(icalUrl);
|
||||
|
|
@ -296,6 +307,33 @@ export const getRelevantEventsForCalendarIntegrationFromIcal = async (
|
|||
return calendarIntegrationEvents;
|
||||
};
|
||||
|
||||
/**
|
||||
* Scores the optional fields that distinguish a fully-populated copy of an event
|
||||
* from a sparse one (description, url) — the only fields that realistically vary
|
||||
* between the duplicate same-UID copies providers emit. Used purely as the
|
||||
* dedup tie-breaker; on an equal score the first-seen copy is kept.
|
||||
*/
|
||||
const calendarEventRichness = (ev: CalendarIntegrationEvent): number =>
|
||||
(ev.description ? 1 : 0) + (ev.url ? 1 : 0);
|
||||
|
||||
/**
|
||||
* Removes duplicate events that share the same id, keeping the richest copy.
|
||||
* Insertion order is preserved; the original array is returned unchanged when
|
||||
* there are no duplicates (the common case).
|
||||
*/
|
||||
const dedupeCalendarEventsById = (
|
||||
events: CalendarIntegrationEvent[],
|
||||
): CalendarIntegrationEvent[] => {
|
||||
const byId = new Map<string, CalendarIntegrationEvent>();
|
||||
for (const ev of events) {
|
||||
const existing = byId.get(ev.id);
|
||||
if (!existing || calendarEventRichness(ev) > calendarEventRichness(existing)) {
|
||||
byId.set(ev.id, ev);
|
||||
}
|
||||
}
|
||||
return byId.size === events.length ? events : Array.from(byId.values());
|
||||
};
|
||||
|
||||
const calculateEventDuration = (vevent: ICalVEvent, startTimeStamp: number): number => {
|
||||
// NOTE: Per RFC 2445, events can have either DTEND or DURATION, but not both
|
||||
// If neither is present, the event ends at DTSTART (zero duration)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue