8000 Update sync progress bar when pending in SAF by sunkup · Pull Request #1445 · bitfireAT/davx5-ose · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update sync progress bar when pending in SAF #1445

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 22 commits into
base: main-ose
Choose a base branch
from
Open

Update sync progress bar when pending in SAF #1445

wants to merge 22 commits into from
+164 −20

Conversation

sunkup
Copy link
Member
@sunkup sunkup commented May 6, 2025 •
edited
Loading

Purpose

After editing an event, task or contact in a local collection / content provider, their entries are marked as dirty and the sync framework can inform us about the pending sync status. We should listen for changes and show the state in DAVx5.

Short description

  • SyncFrameworkIntegration: Create isSyncPending flow which emits sync pending state for given account and authorities
  • AccountProgressUseCase: Collect the flow and show sync pending status in caldav/carddav collection view
  • AccountsModel: Collect the flow and show sync pending status in accounts view
  • Cancel any pending SAF syncs on sync request from SAF before enqueueing workmanager sync. This will reset the (wrongly saved) always pending sync flag in Android 14 and 15 and has no consequences for the functionionality on A13 and below.

Note

For testing, note that sometimes pending syncs get stuck forever if (any?) tasks app is installed and synchronized. Try testing on a clean emulator without tasks apps installed and it should work. Fixed

Note 2

Changes in any calendar will show the calendar authority of all accounts as pending for sync, regardless of which is checked. This does not seem to be the case with contacts. Since it's SAF internal again, I am not sure whether/how this can be avoided.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup linked an issue May 6, 2025 that may be closed by this pull request
Pending sync not shown in some cases #1422
Open
@sunkup sunkup force-pushed the 1422-pending-sync-not-shown-in-some-cases branch 3 times, most recently from 48e605c to 6eaf0ef Compare May 8, 2025 10:25
@sunkup sunkup added the enhancement New feature or request label May 8, 2025
@sunkup sunkup self-assigned this May 8, 2025
@sunkup sunkup force-pushed the 1422-pending-sync-not-shown-in-some-cases branch from 6eaf0ef to bdc8e2e Compare May 8, 2025 11:17
@sunkup sunkup requested a review from ArnyminerZ May 8, 2025 11:28
@sunkup sunkup marked this pull request as ready for review May 8, 2025 11:28
ArnyminerZ
ArnyminerZ requested changes May 8, 2025
View reviewed changes
Copy link
Member
@ArnyminerZ ArnyminerZ left a comment •
edited by sunkup
Loading

Choose a reason for hiding this comment

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

It works fine, but as you've said, the progress bar gets stuck.

image

It doesn't get away even after syncing correctly.

@sunkup
Copy link
Member Author
sunkup commented May 12, 2025

It works fine, but as you've said, the progress bar gets stuck.

See the problem described here: #1422 (comment)

As stated I would like to address it in a new PR, because it's not really related to this feature. Try testing on a clean emulator without tasks apps installed and it should work.

@sunkup sunkup requested a review from ArnyminerZ May 12, 2025 07:34
@sunkup
Copy link
Member Author
sunkup commented May 12, 2025 •
edited

Loading

Good thing, that it happens to you too though. Confirms the issue is not only on my side. :) I have created #1458 to track the problem.

@sunkup sunkup mentioned this pull request May 12, 2025
Open
@sunkup sunkup linked an issue May 13, 2025 that may be closed by this pull request
[Sync framework] Sync always pending on A14+ #1458
Open
@sunkup sunkup removed the request for review from ArnyminerZ May 13, 2025 09:21
@sunkup sunkup marked this pull request as draft May 13, 2025 09:21
@sunkup sunkup marked this pull request as ready for review May 13, 2025 10:30
@sunkup sunkup requested a review from ArnyminerZ May 13, 2025 10:30
ArnyminerZ
ArnyminerZ requested changes May 16, 2025
View reviewed changes
Copy link
Member
@ArnyminerZ ArnyminerZ left a comment •
edited by sunkup
Loading

Choose a reason for hiding this comment

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

The progress bar is still getting stuck for me, I don't know if that should be the case:

image

@sunkup
Copy link
Member Author
sunkup commented May 19, 2025

Were you using Android 16? It's working for me if I do the following:

A14 or A15:

  1. Setup two accounts, enable a calendar collection in both sync them
  2. Go into calendar app (I am using etar) and create/delete/edit an event in a DAVx5 managed calendar
  3. Go back to DAVx5 -> see both account sync states as pending (unfortunately it's authority wide)
  4. Wait
  5. The account of which a calendar event was changed starts syncing
  6. After sync the pending state is cleared for the synced account only
  7. Wait
  8. the remaining account is syncing and the pending state clears.

I have added Android 16 to the check and it works fine there too - but it takes a few seconds before the sync adapter framework initiates the sync :)

@sunkup sunkup force-pushed the 1422-pending-sync-not-shown-in-some-cases branch from c4842e4 to ae42e69 Compare May 22, 2025 12:10
@sunkup sunkup requested a review from ArnyminerZ May 26, 2025 07:13
ArnyminerZ
ArnyminerZ approved these changes May 26, 2025
View reviewed changes
Copy link
Member
@ArnyminerZ ArnyminerZ left a comment

Choose a reason for hiding this comment

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

Now works fine for me :)

@sunkup sunkup requested a review from rfc2822 May 26, 2025 11:08
rfc2822
rfc2822 reviewed Jun 11, 2025
View reviewed changes
Copy link
Member
@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Hm… I wonder whether we should do the cancelSync thing. In theory we could also merge it and say it works for Android <14 (and hopefully soon again as soon as it's fixed in Android).

@sunkup sunkup force-pushed the 1422-pending-sync-not-shown-in-some-cases branch 2 times, most recently from 8c89ba5 to 544a814 Compare June 16, 2025 13:17
@sunkup sunkup requested a review from rfc2822 June 23, 2025 06:48
@rfc2822 rfc2822 force-pushed the 1422-pending-sync-not-shown-in-some-cases branch from 544a814 to 8c53814 Compare June 24, 2025 07:47
rfc2822
rfc2822 reviewed Jun 24, 2025
View reviewed changes
Copy link
Member
@rfc2822 rfc2822 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! Can you please split it into 2 PRs:

The reason is that I think the first one has more code, but is easier to merge.

For the second I'll have a more in-depth look / a few comments, but it's easier if the first one is already merged.

This was unlinked from issues Jun 24, 2025
Pending sync not shown in some cases #1422
Open
[Sync framework] Sync always pending on A14+ #1458
Open
@sunkup sunkup mentioned this pull request Jun 24, 2025
Draft
5 tasks
rfc2822
rfc2822 reviewed Jun 24, 2025
View reviewed changes
sunkup added 22 commits June 25, 2025 09:54
@sunkup
Provide flow to check if sync of account and authority is pending in …
e6fdca0 
…sync framework

Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Show if sync is pending in sync framework
cc1030f 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Show if sync is pending in sync framework
d688c0f 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Fix kdoc
0d56a2a 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Cancel any pending SAF syncs on sync request
2a81957 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Cancel sync adapter sync only after having enqueued our worker sync
9328a9d 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Improve accuracy by also checking isSyncActive
fdfccdf 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Remove log statements
fb0e3b4 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Only query pending state
b676642 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Cancel sync adapter sync only on android 14 and 15
f56f24b 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Cancel sync adapter sync with authority
c565d66 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Cancel sync adapter sync by using sync request instead of canceling d…
a1f951b 
…irectly

Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Include android 16
ff5bd27 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Include all versions after Android 14
0e17847 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Add test which documents wrong pending sync check behaviour
d08483b 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Revert "Add test which documents wrong pending sync check behaviour"
13884a2 
This reverts commit 8c53814.
@sunkup
Drop Android 14+ always pending sync work around
b85dc61 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Differentiate better between enqueued and pending syncs
6bab176 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Refactor sync pending check to use flow for address book accounts
8464275 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Always return false for sync pending state check on Android 14+
8a735f1 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Refactor sync pending check to use flow for address book accounts
ec7e5b2 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup
Add comments
69c21ad 
Signed-off-by: Sunik Kupfer <kupfer@bitfire.at>
@sunkup sunkup force-pushed the 1422-pending-sync-not-shown-in-some-cases branch from b9a7cca to 69c21ad Compare June 25, 2025 07:55
@sunkup sunkup requested a review from rfc2822 June 25, 2025 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArnyminerZ ArnyminerZ ArnyminerZ approved these changes

@rfc2822 rfc2822 Awaiting requested review from rfc2822

At least 1 approving review is required to merge this pull request.

Assignees

@sunkup sunkup

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sunkup @rfc2822 @ArnyminerZ
0