From b4d0e2c62ceef9097f4aad8cb0e5e4b730294680 Mon Sep 17 00:00:00 2001 From: Maximilian Martin Date: Sat, 26 Sep 2026 14:20:29 +0000 Subject: [PATCH] fix(chore): allow editing and deleting first occurrences of recurring events Signed-off-by: Maximilian Martin --- src/mixins/EditorMixin.js | 6 - src/store/calendarObjectInstance.js | 13 -- .../unit/mixins/EditorMixin.test.js | 18 +-- .../unit/store/calendarObjectInstance.test.ts | 144 +++++++++++++----- 4 files changed, 114 insertions(+), 67 deletions(-) diff --git a/src/mixins/EditorMixin.js b/src/mixins/EditorMixin.js index bd1c1809ec..5cffd10485 100644 --- a/src/mixins/EditorMixin.js +++ b/src/mixins/EditorMixin.js @@ -741,9 +741,6 @@ export default { if (this.isViewedByAttendee) { return scope === 'series' || (this.isEditingExceptionInstance && scope === 'occurrence') } - if (!this.isEditingExceptionInstance && this.isEditingBaseInstance && scope !== 'series') { - return false - } return ['occurrence', 'future', 'series'].includes(scope) }, @@ -819,9 +816,6 @@ export default { if (this.isViewedByAttendee) { return scope === 'series' || (this.isEditingExceptionInstance && scope === 'occurrence') } - if (!this.isEditingExceptionInstance && this.isEditingBaseInstance && scope !== 'series') { - return false - } return ['occurrence', 'future', 'series'].includes(scope) }, diff --git a/src/store/calendarObjectInstance.js b/src/store/calendarObjectInstance.js index 585dc4df86..549f8130ac 100644 --- a/src/store/calendarObjectInstance.js +++ b/src/store/calendarObjectInstance.js @@ -1555,12 +1555,6 @@ export default defineStore('calendarObjectInstance', { logger.error('Only "this occurrence" can be updated while editing an existing recurrence exception') return } - // Do not permit "this occurrence"/"this and future" edits on the primary - if (isForkedItem && (scope === 'occurrence' || scope === 'future') && isBaseOccurrence(calendarObject, eventComponent)) { - logger.error('Only "series" can be updated while editing the primary occurrence of a series') - return - } - let original = null let fork = null @@ -1639,13 +1633,6 @@ export default defineStore('calendarObjectInstance', { return } - // Do not permit "this occurrence"/"this and future" deletes on the primary - // occurrence of a series - only "series" makes sense there - if ((scope === 'occurrence' || scope === 'future') && isBaseOccurrence(this.calendarObject, eventComponent)) { - logger.error('Only "series" can be deleted while editing the primary occurrence of a series') - return - } - // Recurring event - remove this occurrence or this and all future const isRecurrenceSetEmpty = eventComponent.removeThisOccurrence(scope === 'future') if (isRecurrenceSetEmpty) { diff --git a/tests/javascript/unit/mixins/EditorMixin.test.js b/tests/javascript/unit/mixins/EditorMixin.test.js index 6f077fd091..ef996a51cc 100644 --- a/tests/javascript/unit/mixins/EditorMixin.test.js +++ b/tests/javascript/unit/mixins/EditorMixin.test.js @@ -63,12 +63,10 @@ describe('mixins/EditorMixin test suite', () => { [{ isRecurringInstance: true, isEditingExceptionInstance: true }, 'occurrence', true], [{ isRecurringInstance: true, isEditingExceptionInstance: true }, 'future', false], [{ isRecurringInstance: true, isEditingExceptionInstance: true }, 'series', false], - // The primary occurrence IS the whole series - deleting "just this occurrence" - // or "this and future" doesn't offer anything meaningfully different from - // deleting "the whole series" here (for the organizer; an attendee's own - // RSVP scope is unrelated and stays governed by isViewedByAttendee above). - [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'occurrence', false], - [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'future', false], + // Deleting the primary occurrence removes it from the recurrence set without + // necessarily deleting later occurrences. + [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'occurrence', true], + [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'future', true], [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'series', true], // isEditingBaseInstance is purely position-based, so it can also be true for an // exception that happens to sit at the primary occurrence's own position - the @@ -150,11 +148,9 @@ describe('mixins/EditorMixin test suite', () => { [{ isRecurringInstance: true, isEditingExceptionInstance: true }, 'future', false], [{ isRecurringInstance: true, isEditingExceptionInstance: true }, 'series', false], [{ isRecurringInstance: true, isEditingExceptionInstance: true, isViewedByAttendee: true }, 'series', false], - // The primary occurrence IS the whole series - "this occurrence" and "this and - // future" aren't offered there, only "series" (for the organizer; an attendee's - // own RSVP scope is unrelated and stays governed by isViewedByAttendee above). - [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'occurrence', false], - [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'future', false], + // The primary occurrence can also be edited independently by creating an exception. + [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'occurrence', true], + [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'future', true], [{ isRecurringInstance: true, isEditingBaseInstance: true }, 'series', true], // isEditingBaseInstance is purely position-based, so it can also be true for an // exception that happens to sit at the primary occurrence's own position - the diff --git a/tests/javascript/unit/store/calendarObjectInstance.test.ts b/tests/javascript/unit/store/calendarObjectInstance.test.ts index 46e108c7f4..29ed5c086c 100644 --- a/tests/javascript/unit/store/calendarObjectInstance.test.ts +++ b/tests/javascript/unit/store/calendarObjectInstance.test.ts @@ -2,7 +2,7 @@ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { createEvent, DateTimeValue, getParserManager } from '@nextcloud/calendar-js' +import { createEvent, DateTimeValue, DurationValue, getParserManager } from '@nextcloud/calendar-js' import { showWarning } from '@nextcloud/dialogs' import { translate } from '@nextcloud/l10n' import { createPinia, setActivePinia } from 'pinia' @@ -535,35 +535,6 @@ describe('store/calendarObjectInstance test suite', () => { expect(calendarObjectsStore.updateCalendarObject).toHaveBeenCalledWith({ calendarObject }) }) - it.each(['occurrence', 'future'] as const)('refuses to save %s-wide changes while editing the primary occurrence of a series', async (scope) => { - const store = useCalendarObjectInstanceStore() - const calendarObjectsStore = useCalendarObjectsStore() - const baseComponent = setUpBaseComponent(1000, 2000) - const primaryOccurrence = setUpEventComponent(1000, 1000, 2000) - // "occurrence"/"future" would otherwise reach createRecurrenceException(), which - // isn't stubbed here - if the early return is ever bypassed, this throws loudly - // instead of silently succeeding against an undefined method. - const calendarObject = { - calendarId: 'calendar-1', - calendarComponent: { - getComponentIterator: vi.fn().mockReturnValue([baseComponent, primaryOccurrence]), - }, - } - store.calendarObject = calendarObject - store.calendarObjectInstance = { eventComponent: primaryOccurrence } - vi.spyOn(calendarObjectsStore, 'updateCalendarObject').mockResolvedValue() - mockedisBaseOccurrence.mockReturnValue(true) - - await store.saveCalendarObjectInstance({ - scope, - calendarId: 'calendar-1', - }) - - expect(baseComponent.deleteAllProperties).not.toHaveBeenCalled() - expect(baseComponent.addProperty).not.toHaveBeenCalled() - expect(calendarObjectsStore.updateCalendarObject).not.toHaveBeenCalled() - }) - it('does not consult isBaseOccurrence() for a brand new, never-forked event', async () => { // Regression test: isBaseOccurrence() calls isPartOfRecurrenceSet(), which // needs a recurrence-manager/master item a brand new event doesn't have yet @@ -1060,6 +1031,55 @@ describe('store/calendarObjectInstance test suite', () => { expect(calendarObjectsStore.updateCalendarObject).toHaveBeenCalledWith({ calendarObject }) }) + it('creates a real recurrence-exception when saving an edited first occurrence with occurrence scope (real calendar-js)', async () => { + const ics = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'PRODID:-//Nextcloud//calendar-js tests//EN', + 'BEGIN:VEVENT', + 'UID:first-occurrence-create-test', + 'DTSTART:20260907T100000Z', + 'DTEND:20260907T110000Z', + 'DTSTAMP:20260901T000000Z', + 'SUMMARY:Original title', + 'RRULE:FREQ=WEEKLY;COUNT=3', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n') + + const parser = getParserManager().getParserForFileType('text/calendar') + parser.parse(ics) + const calendarComponent = parser.getItemIterator().next().value + const masterComponent = [...calendarComponent.getComponentIterator()][0] + const firstOccurrence = masterComponent.recurrenceManager.getOccurrenceAtExactly(masterComponent.startDate) + const firstOccurrenceRecurrenceId = firstOccurrence.getReferenceRecurrenceId() + firstOccurrence.updatePropertyWithValue('SUMMARY', 'Edited title') + firstOccurrence.markDirty() + + const calendarObject = { calendarId: 'personal', calendarComponent } + const store = useCalendarObjectInstanceStore() + const calendarObjectsStore = useCalendarObjectsStore() + store.calendarObject = calendarObject + store.calendarObjectInstance = { eventComponent: markRaw(firstOccurrence) } + vi.spyOn(calendarObjectsStore, 'updateCalendarObject').mockResolvedValue() + vi.spyOn(calendarObjectsStore, 'createCalendarObjectFromFork').mockResolvedValue() + mockedisBaseOccurrence.mockReturnValue(true) + + await store.saveCalendarObjectInstance({ + scope: 'occurrence', + calendarId: 'personal', + }) + + const components = [...calendarComponent.getComponentIterator()] + const exceptionComponent = components.find((component) => component.hasProperty('RECURRENCE-ID')) + expect(masterComponent.title).toBe('Original title') + expect(exceptionComponent).toBeDefined() + expect(exceptionComponent.title).toBe('Edited title') + expect(exceptionComponent.getFirstPropertyFirstValue('RECURRENCE-ID').compare(firstOccurrenceRecurrenceId)).toBe(0) + expect(calendarObjectsStore.createCalendarObjectFromFork).not.toHaveBeenCalled() + expect(calendarObjectsStore.updateCalendarObject).toHaveBeenCalledWith({ calendarObject }) + }) + it('preserves the original RRULE when saving series scope from a non-primary occurrence (real calendar-js)', async () => { // Regression test for a real bug: forkItem() adjusts a forked occurrence's // own RRULE COUNT down to "occurrences remaining from this point" (needed @@ -1219,12 +1239,7 @@ describe('store/calendarObjectInstance test suite', () => { expect(calendarObjectsStore.updateCalendarObject).not.toHaveBeenCalled() }) - it.each(['occurrence', 'future'] as const)('refuses to delete %s scope from the primary occurrence of a series', async (scope) => { - // canDelete() in EditorMixin already restricts the primary occurrence to - // "series" only in the UI, mirroring canUpdate()'s rule for - // saveCalendarObjectInstance - this is the backend-side enforcement of - // that same rule, so a caller that bypasses the UI can't delete just one - // occurrence (or truncate the series) starting from the primary occurrence. + it.each(['occurrence', 'future'] as const)('deletes %s scope from the primary occurrence of a series', async (scope) => { const store = useCalendarObjectInstanceStore() const calendarObjectsStore = useCalendarObjectsStore() const eventComponent = setUpEventComponent(true) @@ -1237,9 +1252,64 @@ describe('store/calendarObjectInstance test suite', () => { await store.deleteCalendarObjectInstance({ scope }) - expect(eventComponent.removeThisOccurrence).not.toHaveBeenCalled() + expect(eventComponent.removeThisOccurrence).toHaveBeenCalledWith(scope === 'future') + expect(calendarObjectsStore.updateCalendarObject).toHaveBeenCalledWith({ calendarObject }) expect(calendarObjectsStore.deleteCalendarObject).not.toHaveBeenCalled() }) + + it.each([ + { scope: 'occurrence' as const, expectedEmpty: false }, + { scope: 'future' as const, expectedEmpty: true }, + ])('removes the first occurrence with real calendar-js for scope "$scope"', async ({ scope, expectedEmpty }) => { + const ics = [ + 'BEGIN:VCALENDAR', + 'VERSION:2.0', + 'PRODID:-//Nextcloud//calendar-js tests//EN', + 'BEGIN:VEVENT', + 'UID:first-occurrence-delete-test', + 'DTSTART:20260907T100000Z', + 'DTEND:20260907T110000Z', + 'DTSTAMP:20260901T000000Z', + 'SUMMARY:Recurring event', + 'RRULE:FREQ=WEEKLY;COUNT=3', + 'END:VEVENT', + 'END:VCALENDAR', + ].join('\r\n') + + const parser = getParserManager().getParserForFileType('text/calendar') + parser.parse(ics) + const calendarComponent = parser.getItemIterator().next().value + const masterComponent = [...calendarComponent.getComponentIterator()][0] + const firstOccurrence = masterComponent.recurrenceManager.getOccurrenceAtExactly(masterComponent.startDate) + const rangeEnd = masterComponent.startDate.clone() + rangeEnd.addDuration(DurationValue.fromSeconds(14 * 24 * 60 * 60)) + const firstOccurrenceRecurrenceId = firstOccurrence.getReferenceRecurrenceId() + const nextOccurrenceRecurrenceId = masterComponent.recurrenceManager.getAllOccurrencesBetween(masterComponent.startDate, rangeEnd)[1].getReferenceRecurrenceId() + const calendarObject = { calendarId: 'personal', calendarComponent } + const store = useCalendarObjectInstanceStore() + const calendarObjectsStore = useCalendarObjectsStore() + store.calendarObject = calendarObject + store.calendarObjectInstance = { eventComponent: markRaw(firstOccurrence) } + vi.spyOn(calendarObjectsStore, 'deleteCalendarObject').mockResolvedValue() + vi.spyOn(calendarObjectsStore, 'updateCalendarObject').mockResolvedValue() + + await store.deleteCalendarObjectInstance({ scope }) + + const remainingOccurrences = masterComponent.recurrenceManager.getAllOccurrencesBetween(masterComponent.startDate, rangeEnd) + expect({ + deleted: calendarObjectsStore.deleteCalendarObject.mock.calls.length > 0, + updated: calendarObjectsStore.updateCalendarObject.mock.calls.length > 0, + remainingCount: remainingOccurrences.length, + hasFirstOccurrence: remainingOccurrences.some((occurrence) => occurrence.getReferenceRecurrenceId().compare(firstOccurrenceRecurrenceId) === 0), + hasNextOccurrence: remainingOccurrences.some((occurrence) => occurrence.getReferenceRecurrenceId().compare(nextOccurrenceRecurrenceId) === 0), + }).toEqual({ + deleted: expectedEmpty, + updated: !expectedEmpty, + remainingCount: expectedEmpty ? 0 : 2, + hasFirstOccurrence: false, + hasNextOccurrence: !expectedEmpty, + }) + }) }) describe('removeAttendee', () => {