fix: update, delete, accept, decline all occurrences - #7600
fix: update, delete, accept, decline all occurrences#7600SebastianKrupinski wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Hi, |
|
Hello, that's a big problem for us too. It would be great if that could be fixed soon. |
b9f5658 to
43c8c49
Compare
|
@SebastianKrupinski ; may this PR resolve #6146 as well ? |
That is the idea, I just need to figure out the best work flow logic. |
928f361 to
9b343a4
Compare
|
PHP errors are unrelated |
9b343a4 to
ebf29dc
Compare
|
/backport to stable6.6 |
|
/backport to stable6.5 |
ebf29dc to
8b424b7
Compare
|
@odzhychko should be good now |
8b424b7 to
cc7fd03
Compare
odzhychko
left a comment
There was a problem hiding this comment.
Tested. Works as expected.
Aside from the failing tests,
https://github.com/nextcloud/calendar/pull/7600/changes#r3878791313 might be a blocking issues.
| @@ -154,16 +185,10 @@ export default { | |||
| async setParticipationStatus(participationStatus) { | |||
There was a problem hiding this comment.
nit(non-blocking): changeAttendeesParticipationStatus is not unused.
e2571ee to
32619c4
Compare
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
32619c4 to
c26c525
Compare
| logger.error('Could not find master component to save series-wide changes to') | ||
| } else { | ||
| const isBaseOccurrence = !eventComponent.originalRecurrenceId | ||
| || eventComponent.originalRecurrenceId.compare(baseComponent.startDate) === 0 |
There was a problem hiding this comment.
issue: eventComponent.originalRecurrenceId.compare(baseComponent.startDate) === 0 breaks if
- Create event on Thursday
- Set weekly recurrence on Mondays
- Save
- Edit time first occurrence
- Actual: Seeing the warning and changes are discarded
- Expected: Time change saved
There was a problem hiding this comment.
Hey, I was not able to reproduce this
There was a problem hiding this comment.
Nvm, managed to reproduce it
There was a problem hiding this comment.
Good find btw, would have never thought of trying this
There was a problem hiding this comment.
I'm not sure, if finding by recurrence ID can really be made robust.
Might be easier to record what instance the calendar started editing. Maybe a blunt solution like storing a copy of the original event (or only the original start/end dates) when starting to edit and then diffing with it on save.
There was a problem hiding this comment.
Its caused by @nextcloud/calendar-js's RecurrenceManager.updateStartDateOfMasterItem():
for (const recurrenceException of this.getRecurrenceExceptionIterator()) { // live Map iterator
this.removeRecurrenceException(recurrenceException) // deletes current entry
recurrenceException.recurrenceId.addDuration(difference) // shifts its key
this.relateRecurrenceException(recurrenceException) // re-inserts under a NEW key
}
There was a problem hiding this comment.
Well lets see how many more libraries we can fix while trying to fix this functionality
There was a problem hiding this comment.
I think I found part of the confusion, the event is not actually changing, its only showing the new time on the pop up
I modified the second event by one hour you can see the diff in the popup and actual event (also I did get a warning)
So I just need to make the pop state refresh when we do not allow time update
|
|
||
| const dateTimeWasChanged = eventComponent.startDate.compare(eventComponent.originalRecurrenceId) !== 0 | ||
| || currentDuration.compare(originalDuration) !== 0 | ||
|
|
There was a problem hiding this comment.
compare calculates correctly with timezones.
So warning is not shown (and changes are silently discarded), when uses changes the timezone but the actual UTC time stays identical.
Not sure, if this is an issue.
- create repeating event A starting at 10AM Berlin/Europe
- Edit changing timezone to London/Europe and start to 9AM
- Save for series
- Expected: The warning
- Actual: No warning
There was a problem hiding this comment.
Screencast.From.2026-09-01.16-20-31.mp4
Happens only if the UTC time stays the same after timezone change. E.g. user updates from "10 Europe/Madrid === 11 UTC" to "11 Europe/London === 11 UTC".
Becomes an issue, because (a) user sets timezone for a reason and (b) even if UTC time is same at time of comparison it might be different later for example if one time zone uses summer time/winter time and the other does not.
Probably just needs an additional check here, that timzone in start and end date stayed the same.
|
@odzhychko Do you mind giving this one last test!? |
odzhychko
left a comment
There was a problem hiding this comment.
Looked and tested the fix for the last issue.
Found two broken edge case with regards to saving series.
I've seen that some more code changed in the meanwhile.
I'll try to look at it and retest PR from scratch after the know issues are fixed ^^
70a63fe to
56588d5
Compare
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
56588d5 to
afb5911
Compare


Summary
Logic Changes