[mkcal] Ensure that insertion pattern from ICalFormat::fromRawString(… #7
Conversation
|
Thinking about it a bit more, one could also move the delete action in the ::save() routine before the insertion action. What do you think is a better idea ? |
| while (deleted != d->mIncidencesToDelete.end()) { | ||
| if ((*deleted)->recurrenceId() == incidence->recurrenceId()) { | ||
| deleted = d->mIncidencesToDelete.erase(deleted); | ||
| calendarIncidenceChanged(incidence); |
There was a problem hiding this comment.
I assume this method updates the incidence in the database with the new values?
There was a problem hiding this comment.
Exact, it's the one that is called when an incidence is modified.
|
Am I understanding correctly, that there are two possible orderings (within a transaction): (delete, add) or (add, delete); this PR basically ensures that the first one (i.e. delete followed by add) works correctly, by "un-deleting" the event with that UID/recID in the second (add) step? /edit: adding discussion from IRC: yes, the PR converts the (delete+add) into an (update) basically. Note that the other proposed solution (re-ordering the operations) might cause issues in the (add+delete) transaction case. |
src/sqlitestorage.cpp
Outdated
| ++deleted; | ||
| } | ||
| } | ||
| } else if (!d->mIncidencesToInsert.contains(incidence->uid(), incidence) && !d->mIsLoading) { |
There was a problem hiding this comment.
might be best to now hoist this d->mIsLoading check up, so that we early-out return prior to initializing the deleted variable.
There was a problem hiding this comment.
Sure, that's right. Updated.
|
Also, please add "Contributes to JB#55786" to the commit message - thanks! |
…) is working. Contributes to JB#55786 If an incidence is already in the calendar with a lower revision, the incidence is deleted from the calendar and added again in the new revision. This commit ensures that deleting an event and adding an event with the same UID,RecID will actually modify it in the database.
|
Commit message added. |
|
Thanks very much! |
…) is working.
If an incidence is already in the calendar with a lower revision,
the incidence is deleted from the calendar and added again in the
new revision. This commit ensures that deleting an event and adding
an event with the same UID,RecID will actually modify it in the
database.
This comes from a brainstorm started by this forum thread : https://forum.sailfishos.org/t/bug-4-2-0-21-cannot-import-calendar-entries-via-ics-file/7988/13
If the user is importing data with a same UID but with an increasing revision (not exactly the user case), and if the nemo-qml-plugin-calendar was actually loading the calendar before expanding it with the ICS data (which is not the case neither at the moment), the scheme used in icalformat_p.cpp#2809 in KCalendarCore is not working in mKCal at the moment.
Indeed, the events are deleted after being inserted in SQliteStorage::save(). So the insertion of the revised incidence is failing because of UNIQUE violation on the UID,RecID pair, and then the old incidence is deleted from the database. So at the end we have lost the incidence...
@chriadam and @pvuorela this is correcting this behaviour. At the moment, this deletion + addition scheme is not used anywhere but since it is actually used upstream in KCalendarCore, we may use it later.