Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mkcal] Add a special case for notebook change in save. #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dcaliste
Copy link
Contributor

Handle notebook change as a deletion in the previous notebook,
and an addition in the new one. Like that any sync on the
previous notebook can propagate the deletion.

@pvuorela , from our discussion yesterday, I think I've found a low-level way to properly treat the notebook change. I've added a test to demonstrate it. It should work also for recurring events with exceptions, but there is a bug upstream, see https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/99.

When a downstream code is calling calendar->setNotebook() on an existing incidence, the calendar is calling calendarIncidenceChanged() for every observers. Then, on next save() call this incidence is sent to SqliteFormat::modifyComponent() with a DBUpdate operation. The idea of the patch is to read the old notebook value in case of an update operation, in the same SQL request than the one fetching the rowid. If the notebook differs, we substitute the update operation with a mark as deleted on the incidence followed by an insert operation on the same incidence but with a different notebook id.

If you think it's ok, I will have to update the bindings to replace the current delete + add code with the proper calendar->setNotebook() one. And also reenable all the signals that a notebook has been changed and reenable the action in the UI. But first things first, what do you think of this low level patch ?

<< incidence->recurrenceId() << "as deleted for notebook change";
goto error;
} else {
dbop = DBInsert;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So guess this effectively creates a new row with a new component id. Quite special so could add some note.

But how about if the instance is moved from notebook a->b and then back b->a. The latter should update the deleted instance as undeleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh eh, good point. This will fail at the moment. I'll need to move the purge delete action on addition from the SqliteStorage::Privae::saveIncidence() to here, to ensure that any addition don't leave any mark as deleted entries.

Handle notebook change as a deletion in the previous notebook,
and an addition in the new one. Like that any sync on the
previous notebook can propagate the deletion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants