-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Remove concept of main accounts #989
Conversation
5ab7c47
to
b5f196e
Compare
b5f196e
to
a118521
Compare
9ae52e2
to
8bb9a0b
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.
Very nice! We can do it even without bitfireAT/davx5#603.
A few thoughts. Haven't tried in detail but I think it should work.
We probably should also have a talk about this, but maybe you can already have a look.
app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt
Outdated
Show resolved
Hide resolved
958bda8
to
39e4390
Compare
…y may exist on their own now
39e4390
to
67d0409
Compare
5ebdad9
to
5c31a8a
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.
Please have a look at the changes and requested changes.
app/src/main/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorker.kt
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/sync/AddressBookSyncer.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
Outdated
Show resolved
Hide resolved
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.
It's a lot. But I think it's actually coming along rather nicely now 👍
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 hope it should be fine now. Can you do a final check? Then I can also have a last look and merge.
app/src/androidTest/kotlin/at/bitfire/davdroid/repository/AccountRepositoryTest.kt
Outdated
Show resolved
Hide resolved
I think we're good. I played around with a bit more as well and it seems to hold up 👍 |
Purpose
Right now a
LocalAddressBook
still holds a reference to it's main account, but this is obsolete. We already have a relation between an address book account and the main account it belongs to via the corresponding database collection:"main" account ↔ DB collection ↔ address book ↔ address book account
In the future we want to rely on system accounts as little as possible. Therefore this PR removes the direct relation between an address book account to its "main" account where possible.
Short description
LocalAddressBook
Note: We can probably not use
getAccountsByTypeAndFeatures
to query for an account belonging to a specific db collection. That's because I think we can not set those features ourselves - that is to add the collection ID. AlsoaddAccountExplicitly
which we want to user over the normaladdAccount
, does not seem to have the option to "require account features".Checklist