diff --git a/src/mixins/EditorMixin.js b/src/mixins/EditorMixin.js index 71e4eb68ef..6ecd2cfcd3 100644 --- a/src/mixins/EditorMixin.js +++ b/src/mixins/EditorMixin.js @@ -646,7 +646,7 @@ export default { keyboardDuplicateEvent(event) { if (event.key === 'd' && event.ctrlKey === true) { event.preventDefault() - if (!this.isNew && !this.isReadOnly && !this.canCreateRecurrenceException) { + if (!this.isNew && !this.canCreateRecurrenceException) { this.duplicateEvent() } } @@ -702,12 +702,20 @@ export default { }, /** - * Duplicates a calendar-object and saves it + * Duplicates the calendar-object. If the source calendar is + * read-only, the duplicate is created in the first writable calendar. * * @return {Promise} */ async duplicateEvent() { - await this.calendarObjectInstanceStore.duplicateCalendarObjectInstance() + const calendarId = this.isReadOnly + ? (this.calendarsStore.sortedCalendars[0]?.id ?? null) + : (this.calendarObject?.calendarId ?? null) + await this.calendarObjectInstanceStore.duplicateCalendarObjectInstance({ calendarId }) + + // The editor's calendar picker is driven by this.calendarId, which is + // separate from the store's calendarObject.calendarId. + this.calendarId = this.calendarObject?.calendarId ?? null }, /** diff --git a/src/models/event.js b/src/models/event.js index 521cd4453e..df38996a7b 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -191,18 +191,6 @@ function mapEventComponentToEventObject(eventComponent) { function copyCalendarObjectInstanceIntoEventComponent(eventObject, eventComponent) { const sourceEventComponent = eventObject.eventComponent - const unexpectedRecurrenceProperties = new Set([ - 'RRULE', - 'EXRULE', - 'RDATE', - 'EXDATE', - ]) - for (const property of sourceEventComponent.getPropertyIterator()) { - if (unexpectedRecurrenceProperties.has(property.name)) { - throw new Error(`Illegal argument: Event objects has recurrence related property ${property.name}.`) - } - } - const propertiesExcludedFromCopying = new Set([ // These properties are regenerated for the new copy. 'UID', @@ -213,11 +201,49 @@ function copyCalendarObjectInstanceIntoEventComponent(eventObject, eventComponen // Currently only copying as exact occurrences. // Therefore, do not preserve any RECURRENCE-ID. 'RECURRENCE-ID', + // A duplicate is always a single, standalone event. If the source is + // the master item of a recurring series (e.g. when duplicating its + // first occurrence, calendar-js returns the master item itself), + // its recurrence-defining properties must not carry over. + 'RRULE', + 'EXRULE', + 'RDATE', + 'EXDATE', + ]) + + // Properties that must not occur more than once on a VEVENT (RFC 5545 + // section 3.6.1) but that the source may legitimately carry a value for. + const propertiesReplacedWhenCopying = new Set([ + 'DTSTART', + 'DTEND', + 'DURATION', + 'CLASS', + 'DESCRIPTION', + 'GEO', + 'LOCATION', + 'ORGANIZER', + 'PRIORITY', + 'STATUS', + 'SUMMARY', + 'TRANSP', + 'URL', ]) + + // DTEND and DURATION are mutually exclusive. If the source uses DURATION, + // a pre-existing target DTEND (or vice versa) would otherwise be left + // behind alongside it, which is invalid. + if (sourceEventComponent.hasProperty('DTEND') || sourceEventComponent.hasProperty('DURATION')) { + eventComponent.deleteAllProperties('DTEND') + eventComponent.deleteAllProperties('DURATION') + } + for (const property of sourceEventComponent.getPropertyIterator()) { if (propertiesExcludedFromCopying.has(property.name)) { continue } + if (propertiesReplacedWhenCopying.has(property.name)) { + eventComponent.deleteAllProperties(property.name) + } const successful = eventComponent.addProperty(property.clone()) if (!successful) { throw new Error(`Illegal state: Property ${property.name} could not be copied.`) diff --git a/src/store/calendarObjectInstance.js b/src/store/calendarObjectInstance.js index f73c7325df..aa791a32c2 100644 --- a/src/store/calendarObjectInstance.js +++ b/src/store/calendarObjectInstance.js @@ -1540,9 +1540,11 @@ export default defineStore('calendarObjectInstance', { /** * Duplicate calendar-object-instance * + * @param {object} data The destructuring object + * @param {string=} data.calendarId The id of the calendar to duplicate the event into. Defaults to the source event's calendar * @return {Promise} */ - async duplicateCalendarObjectInstance() { + async duplicateCalendarObjectInstance({ calendarId } = {}) { const calendarObjectsStore = useCalendarObjectsStore() const oldCalendarObjectInstance = this.calendarObjectInstance @@ -1554,7 +1556,7 @@ export default defineStore('calendarObjectInstance', { end: endDate.unixTime, timezoneId: oldEventComponent.startDate.timezoneId, isAllDay: oldEventComponent.isAllDay(), - calendarId: this.calendarObject?.calendarId ?? null, + calendarId: calendarId ?? this.calendarObject?.calendarId ?? null, }) const eventComponent = getObjectAtRecurrenceId(calendarObject, startDate.jsDate) copyCalendarObjectInstanceIntoEventComponent(oldCalendarObjectInstance, eventComponent) diff --git a/src/views/EditFull.vue b/src/views/EditFull.vue index 1f7ae44974..8f7b1b5503 100644 --- a/src/views/EditFull.vue +++ b/src/views/EditFull.vue @@ -69,7 +69,7 @@ {{ $t('calendar', 'Export') }} - + diff --git a/src/views/EditSimple.vue b/src/views/EditSimple.vue index e379783036..5d9a71c23a 100644 --- a/src/views/EditSimple.vue +++ b/src/views/EditSimple.vue @@ -82,7 +82,7 @@ {{ $t('calendar', 'Export') }} - + diff --git a/tests/javascript/unit/models/event.test.js b/tests/javascript/unit/models/event.test.js index 899eeaf3f2..67402adc9f 100644 --- a/tests/javascript/unit/models/event.test.js +++ b/tests/javascript/unit/models/event.test.js @@ -967,6 +967,55 @@ describe('Test suite: Event model (models/event.js)', () => { expect(targetEventComponent.getFirstPropertyFirstValue('A-CUSTOM-PROPERTY')).toBe('TRUE') }) + it('should replace, not duplicate, DTSTART/DTEND in the new event component', () => { + // Given + // The target already has its own DTSTART/DTEND set from the occurrence + // it was created for. Simply appending the source's on top would leave + // the component with duplicate DTSTART/DTEND properties, which servers + // reject as invalid iCalendar - so the target's own values must be + // replaced by the source's, not added alongside them. + const sourceRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2016, 7, 16, 7, 0, 0)), true) + const sourceEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-timed', sourceRecurrenceId) + const sourceEventObject = mapEventComponentToEventObject(sourceEventComponent) + + const targetRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2016, 7, 16, 9, 0, 0)), true) + const targetEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-minimal', targetRecurrenceId) + + // When + copyCalendarObjectInstanceIntoEventComponent(sourceEventObject, targetEventComponent) + + // Then + expect([...targetEventComponent.getPropertyIterator('DTSTART')]).toHaveLength(1) + expect([...targetEventComponent.getPropertyIterator('DTEND')]).toHaveLength(1) + expect(targetEventComponent.startDate.unixTime).toEqual(sourceEventComponent.startDate.unixTime) + expect(targetEventComponent.endDate.unixTime).toEqual(sourceEventComponent.endDate.unixTime) + }) + + it('should not leave a stale DTEND when the source uses DURATION instead', () => { + // Given + // DTEND and DURATION are mutually exclusive on a VEVENT. The target + // (created via calendar-js's createEvent()) always has a DTEND, so if + // the source describes its length with DURATION instead, the target's + // DTEND must still be cleared - not just left behind alongside the + // newly-added DURATION. + const sourceRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2016, 7, 16, 7, 0, 0)), true) + const sourceEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-timed', sourceRecurrenceId) + sourceEventComponent.deleteAllProperties('DTEND') + sourceEventComponent.updatePropertyWithValue('DURATION', DurationValue.fromSeconds(3600)) + const sourceEventObject = mapEventComponentToEventObject(sourceEventComponent) + + const targetRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2016, 7, 16, 9, 0, 0)), true) + const targetEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-minimal', targetRecurrenceId) + expect(targetEventComponent.hasProperty('DTEND')).toBe(true) + + // When + copyCalendarObjectInstanceIntoEventComponent(sourceEventObject, targetEventComponent) + + // Then + expect([...targetEventComponent.getPropertyIterator('DTEND')]).toHaveLength(0) + expect([...targetEventComponent.getPropertyIterator('DURATION')]).toHaveLength(1) + }) + it('should not copy recurrence ID into a new event component', () => { // Given const sourceRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2020, 2, 8, 14, 0, 0)), true) @@ -983,8 +1032,12 @@ describe('Test suite: Event model (models/event.js)', () => { expect(targetEventComponent.hasProperty('RECURRENCE-ID')).toBeFalsy() }) - it('should not copy recurring events into a new event component', () => { + it('should not copy recurrence-defining properties into a new event component', () => { // Given + // The first occurrence's recurrence-id matches the master item's own DTSTART, + // so calendar-js returns the master item itself here (RRULE and all) instead + // of a forked occurrence. A duplicate must still come out as a single, + // non-recurring event rather than throwing or inheriting the recurrence rule. const sourceRecurrenceId = DateTimeValue.fromJSDate(new Date(Date.UTC(2020, 2, 1, 14, 0, 0)), true) const sourceEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-recurring', sourceRecurrenceId) const sourceEventObject = mapEventComponentToEventObject(sourceEventComponent) @@ -993,8 +1046,15 @@ describe('Test suite: Event model (models/event.js)', () => { const targetEventComponent = getEventComponentFromAsset('vcalendars/vcalendar-event-minimal', targetRecurrenceId) // When - expect(() => copyCalendarObjectInstanceIntoEventComponent(sourceEventObject, targetEventComponent)) - .toThrow('Illegal argument: Event objects has recurrence related property RRULE.') + copyCalendarObjectInstanceIntoEventComponent(sourceEventObject, targetEventComponent) + + // Then + expect(targetEventComponent.hasProperty('RRULE')).toBeFalsy() + expect(targetEventComponent.hasProperty('EXRULE')).toBeFalsy() + expect(targetEventComponent.hasProperty('RDATE')).toBeFalsy() + expect(targetEventComponent.hasProperty('EXDATE')).toBeFalsy() + // Non-recurrence properties from the source are still copied as usual. + expect(targetEventComponent.title).toEqual(sourceEventComponent.title) }) it('should copy subcomponents into a new event component', () => { diff --git a/tests/javascript/unit/store/calendarObjectInstance.test.ts b/tests/javascript/unit/store/calendarObjectInstance.test.ts new file mode 100644 index 0000000000..cda4df323a --- /dev/null +++ b/tests/javascript/unit/store/calendarObjectInstance.test.ts @@ -0,0 +1,88 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { createPinia, setActivePinia } from 'pinia' +import { describe, expect, it, vi } from 'vitest' +import { copyCalendarObjectInstanceIntoEventComponent, mapEventComponentToEventObject } from '@/models/event.js' +import useCalendarObjectInstanceStore from '@/store/calendarObjectInstance.js' +import useCalendarObjectsStore from '@/store/calendarObjects.js' +import { getObjectAtRecurrenceId } from '@/utils/calendarObject.js' + +vi.mock('@/models/event.js') +vi.mock('@/utils/calendarObject.js') + +const mockedCopyCalendarObjectInstanceIntoEventComponent = vi.mocked(copyCalendarObjectInstanceIntoEventComponent) +const mockedMapEventComponentToEventObject = vi.mocked(mapEventComponentToEventObject) +const mockedGetObjectAtRecurrenceId = vi.mocked(getObjectAtRecurrenceId) + +describe('store/calendarObjectInstance test suite', () => { + beforeEach(() => { + setActivePinia(createPinia()) + + mockedCopyCalendarObjectInstanceIntoEventComponent.mockReset() + mockedMapEventComponentToEventObject.mockReset().mockReturnValue({ eventComponent: {} }) + mockedGetObjectAtRecurrenceId.mockReset().mockReturnValue({}) + }) + + describe('duplicateCalendarObjectInstance', () => { + /** + * @param store The calendarObjectInstance store + * @param calendarId The id of the calendar the source event lives in + */ + function setUpSourceEvent(store: ReturnType, calendarId: string) { + store.calendarObject = { calendarId } + store.calendarObjectInstance = { + eventComponent: { + startDate: { + timezoneId: 'UTC', + getInUTC: () => ({ unixTime: 1000, jsDate: new Date(1000 * 1000) }), + }, + endDate: { + getInUTC: () => ({ unixTime: 2000 }), + }, + isAllDay: () => false, + }, + } + } + + it('duplicates into the explicitly given calendar instead of the source calendar', async () => { + const store = useCalendarObjectInstanceStore() + const calendarObjectsStore = useCalendarObjectsStore() + setUpSourceEvent(store, 'readonly-calendar') + vi.spyOn(calendarObjectsStore, 'createNewEvent').mockResolvedValue({ calendarComponent: {} }) + + await store.duplicateCalendarObjectInstance({ calendarId: 'writable-calendar' }) + + expect(calendarObjectsStore.createNewEvent).toHaveBeenCalledWith( + expect.objectContaining({ calendarId: 'writable-calendar' }), + ) + }) + + it('falls back to the source calendar when no calendarId is given', async () => { + const store = useCalendarObjectInstanceStore() + const calendarObjectsStore = useCalendarObjectsStore() + setUpSourceEvent(store, 'source-calendar') + vi.spyOn(calendarObjectsStore, 'createNewEvent').mockResolvedValue({ calendarComponent: {} }) + + await store.duplicateCalendarObjectInstance() + + expect(calendarObjectsStore.createNewEvent).toHaveBeenCalledWith( + expect.objectContaining({ calendarId: 'source-calendar' }), + ) + }) + + it('marks the duplicated event as a new, unsaved calendar-object', async () => { + const store = useCalendarObjectInstanceStore() + const calendarObjectsStore = useCalendarObjectsStore() + setUpSourceEvent(store, 'source-calendar') + const newCalendarObject = { calendarComponent: {} } + vi.spyOn(calendarObjectsStore, 'createNewEvent').mockResolvedValue(newCalendarObject) + + await store.duplicateCalendarObjectInstance({ calendarId: 'writable-calendar' }) + + expect(store.isNew).toBe(true) + expect(store.calendarObject).toStrictEqual(newCalendarObject) + }) + }) +})