-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fix old address book accounts not being deleted #1039
Conversation
…ccount for address book account
This PR/issue depends on: |
1 similar comment
This PR/issue depends on: |
e64acf0
to
e95e58e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, let's see if @rfc2822 has anything to say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have 😁 First bunch (will have a look at AccountsCleanupWorker
and AccountSettings
afterwards).
I have also merged a few comments / KDoc in #1059, please update from main-ose branch accordingly.
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettingsMigrations.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettingsMigrations.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettingsMigrations.kt
Outdated
Show resolved
Hide resolved
38a6042
to
c25b2d9
Compare
c25b2d9
to
6616c57
Compare
b3f8077
to
f4a857e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, hopefully we didn't overlook something.
Depends on #1047 so that the migration is not run in main thread
Purpose
#989 has renamed address book accounts. The introduced migration creates new address books and fails at deleting the old ones. Now old orphan ones are lying around and even caused exceptions.
An exception should not have been thrown. We should simply log a warning instead.
Orphaned address book accounts should of course not exist in the first place and be deleted by the AccountsCleanupWorker in case they do exist.
In the migration: Instead of syncing to create new address book accounts and then deleting the old ones with the AccountsCleanupworker we should actually rename the existing address book accounts before a first sync happens in the account settings migration.
Short description
AccountsCleanupWorker
(was not working)Checklist