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

Various minor changes to clean and allow multi-notebooks for incidence sharing the same UIDs #63

Merged
merged 5 commits into from Jun 7, 2023

Conversation

dcaliste
Copy link
Contributor

@dcaliste dcaliste commented Jun 7, 2023

@pvuorela, this is a spin off from #60 with the first commits before introducing the separation between DB support and calendar association.

Like for incidences, don't set milliseconds since the
database is only saving up to second precision.

Also ensure that invalid date times for notebook
modification, creation and sync dates are restored
as invalid date time and not as 1970-01-01T00:00.
Also don't specifically hide classes
that need to be, since the code is
compiled with -fvisibility=hidden.
…#60800

Don't base the alarm clearing on UID only,
allowing later to get incidences with the
same UID in different notebooks.
…s to JB#60800

Ensure that selecting a specific incidence from the
components table is also using the notebook UID. This
will allow to hav incidences sharing the same UID in
different notebooks.
@pvuorela
Copy link
Contributor

pvuorela commented Jun 7, 2023

Hm, with this I seem to be having 3 failures on tst_storage.

@dcaliste
Copy link
Contributor Author

dcaliste commented Jun 7, 2023

I've just recompiled this branch, copied the libmkcal-qt5.so.0.7.19 and the three test files to my device. I ran them with LD_LIBRARY_PATH set to use the newly copied library and everything is working. I've rechecked the dates of the files to be sure.

I ran the tst_storage test on a fresh empty database and on an already existing one. Still passing.

There are three warnings, two of them being valid (testing cases emitting a warning). The third one, I can remove if you want. I forgot to add the notebook to the newly changed purgeDeletedIncidences() routine.

@pvuorela
Copy link
Contributor

pvuorela commented Jun 7, 2023

Duh, I did some timezone test earlier and forgot the device to Pago Pago. Switching back to Helsinki made the tests pass.

However, comparing the Pago Pago case, on current master it has two failures while this also fails on:

FAIL!  : tst_storage::tst_storageObserver() Compared values are not the same
   Loc: [/home/pvuorela/src/mkcal/tests/tst_storage.cpp(2271)]
PASS   : tst_storage::cleanupTestCase()

@pvuorela
Copy link
Contributor

pvuorela commented Jun 7, 2023

No wait, the 2271 fail is there even on Helsinki and Paris. Hm.

@pvuorela
Copy link
Contributor

pvuorela commented Jun 7, 2023

What I think is happening is that earlier the deleted instances were returned as the main event first, the unit test assumes that but now there are no more guarantees on what's the order of returned events.

@dcaliste
Copy link
Contributor Author

dcaliste commented Jun 7, 2023

What I think is happening is that earlier the deleted instances were returned as the main event first,

Very good catch. It's due to the last commit of the PR, so I amended it not to assume a specific order in the test.

tests/tst_storage.cpp Outdated Show resolved Hide resolved
instanceIdentifier() can be used as a unique key
to identify an incidence. Use it instead of UID
to keep track of calendar changes.
@pvuorela pvuorela merged commit af9caf1 into sailfishos:master Jun 7, 2023
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