fix(store): propagate organizer/attendees to recurrence-exceptions on…#8456
fix(store): propagate organizer/attendees to recurrence-exceptions on…#8456ndo84bw wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Hi @ndo84bw Thank you for the PR, I will review/test this when I have some time. |
… master in-place update Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Nico Donath <ndo84bw@gmx.de>
b7c2047 to
3952345
Compare
DerDreschner
left a comment
There was a problem hiding this comment.
Tested successfully
There was a problem hiding this comment.
Hi @ndo84bw
I am going to have to block this PR for now. If I am reading the code correctly, there are some side effects of this change.
When this is used on the base event (master in your words) this loops through all exemptions and overrides the attendees and organizer.
The first problem with this is that a recurring exemption can have all kinds of changes, one is that you can have a different organizer for a particular day.
The other major issue is that you are completely overriding attendee responses.
exception.deleteAllProperties('ATTENDEE')
for (const att of masterAttendees) {
exception.addProperty(att.clone())
}
This code deletes the property competely, and applies the cloned attendees from the base event, this means that you are applying responses from the base event to the exception.
So if a attendee responded "declined" to a exception, this would then override that with what the status is in the base event
Essentially the issue is what is already listed in your "Trade-offs" section.
Summary
Fixes #8450.
When the organizer applies "Update this and all future" to the master occurrence (NR-1) of a recurring series that already contains a moved occurrence (RECURRENCE-ID override), calendar-js'
_overridePrimaryItem()copies the new state onto the master but leaves the existing override VEVENT untouched. Consequence: if the user just added the first attendee (and thus the ORGANIZER), the override ends up without either property. Two visible symptoms:OrganizerListItem.vue(organizer.attendeeProperty.getParameterFirstValue(...)onundefined), the attendee list does not render, and the override appears empty even though Thunderbird and other CalDAV clients display it correctly.Fix
+30 LOC in
src/store/calendarObjectInstance.js: one new file-local helpersyncMasterParticipantsOntoExceptions(master, exceptions)plus a four-line guarded call insidesaveCalendarObjectInstance.Immediately after
eventComponent.createRecurrenceException(thisAndAllFuture)returns, if the call entered the master-in-place override path (thisAndAllFuture && isExactForkOfPrimary && primaryItem.isMasterItem()), iterate the recurrence-exceptions whoseRECURRENCE-IDis at or after the master start and overwrite each exception'sATTENDEElist andORGANIZERwith clones of the master's. TheRECURRENCE-ID >= master startfilter is defensive: for any RFC-compliant series every exception already satisfies this, so in practice the filter selects all exceptions, but it guards against pathological data where an exception's RID precedes the current master start.ORGANIZERon the exception is only deleted when a replacement exists, to avoid introducing the same crash this PR fixes.The fix lives in the calendar app store rather than in
calendar-js's_overridePrimaryItem()for delivery speed; the same propagation arguably belongs one layer down so every consumer ofcreateRecurrenceException(true)on a master-aligned fork benefits. A follow-up againstnextcloud/calendar-jsis a reasonable next step.Trade-offs
ATTENDEEandORGANIZER. The same shape of bug almost certainly applies to any other master-level property edit (SUMMARY,DESCRIPTION,LOCATION, ...) when "this and all future" is applied to the master in place: the override keeps its stale value while the master gets the new one. Out of scope for this PR, which targets the reported Adding an attendee to a recurring series with a moved occurrence - moved instance is invisible to the attendee, and shows no attendees to the organizer #8450 attendee-visibility regression specifically. Worth a follow-up if anyone reproduces another property surfacing the same way.PARTSTAT,RSVP,SCHEDULE-STATUSon an override-side attendee are similarly overwritten in the same gesture. This is the intended semantics for "Update this and all future" applied to attendees.Test plan
Manual repro on a live Nextcloud 33 / Calendar 6.4.2 instance, two real accounts (organizer + attendee), both Thunderbird CalDAV and the NC WebUI verified:
ATTENDEEandORGANIZERas the master (Thunderbird as ground truth: the moved occurrence shows up with organizer and attendee on it).tests/javascript/unit/store/calendarObjectInstance.test.js— file does not exist yet; happy to add one in a follow-up if maintainer prefers that here.Related / follow-ups
🤖 AI (if applicable)