Skip to content

[Push] Update registration worker to actively select distributor#2166

Merged
ArnyminerZ merged 36 commits into
mainfrom
2153-push-update-registration-worker-to-actively-select-distributor-new
May 8, 2026
Merged

[Push] Update registration worker to actively select distributor#2166
ArnyminerZ merged 36 commits into
mainfrom
2153-push-update-registration-worker-to-actively-select-distributor-new

Conversation

@ArnyminerZ
Copy link
Copy Markdown
Member

@ArnyminerZ ArnyminerZ commented Apr 28, 2026

Purpose

Implements the automatic push distributor selection logic detailed in #2153

Short description

  • Add automatic push distributor resolution

  • Add notification for instances where a distributor cannot be resolved automatically (no distributor, or more than one available)

    Screenshots Screenshot_20260423_124617 image Screenshot_20260423_125839

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.

Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ self-assigned this Apr 28, 2026
@ArnyminerZ ArnyminerZ added pr-feature Implements a new feature or functionality (only for PRs) push Related to WebDAV-Push labels Apr 28, 2026
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ requested a review from Copilot April 28, 2026 11:45
@ArnyminerZ ArnyminerZ marked this pull request as ready for review April 28, 2026 11:51
@ArnyminerZ ArnyminerZ requested a review from sunkup April 28, 2026 11:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements the push distributor auto-selection flow described in #2153 by adding distributor resolution, a user-facing notification when selection is required, and a dialog activity to guide the user through distributor setup.

Changes:

  • Added automatic UnifiedPush distributor resolution (getAckDistributor + resolveDefaultDistributor) when push is enabled.
  • Added a high-importance “user interaction” notification channel and a notification prompting distributor selection.
  • Introduced PushDistributorSelectionActivity to launch the UP selection flow or guide installation/disable push.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
core/src/main/res/values/strings.xml Adds notification/channel strings related to distributor selection and missing distributors.
core/src/main/kotlin/at/bitfire/davdroid/ui/NotificationRegistry.kt Adds a new notification ID and a high-importance channel for user-interaction notifications.
core/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt Triggers unregister/unsubscribe + posts “select distributor” notification when no distributor is available.
core/src/main/kotlin/at/bitfire/davdroid/push/PushNotificationManager.kt Generalizes notification posting API used by both push-pending and selection-required notifications.
core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorSelectionActivity.kt New dialog activity to drive distributor selection / show “no distributor” guidance.
core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Implements distributor resolution logic (saved/ack distributor, otherwise resolve default).
core/src/main/AndroidManifest.xml Registers the new selection activity.
core/src/androidTest/kotlin/at/bitfire/davdroid/push/PushDistributorManagerTest.kt Updates tests to use getAckDistributor (but needs additional stubbing for the new resolution path).

Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
Comment thread core/src/main/res/values/strings.xml Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorSelectionActivity.kt Outdated
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

When I add an account with push-capable collections, like our nextcloud testaccount, currently no distributor is actively being chosen (by the registration worker). Not FCM and not any other.

I get 10:50:21.883 D Push is disabled. Unregistering all instance, and unsubscribing from all services

Line 100 in PushDistributorManager has me think though it should be enabled by default now?

/**
 * Checks whether push notifications are enabled in settings.
 *
 * @return `true` if push notifications are enabled (default), `false` if disabled.
 */
fun isPushEnabled(): Boolean =
    settingsManager.getBooleanOrNull(Settings.PUSH_ENABLED) ?: true

Comment thread core/src/main/res/values/strings.xml Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/ui/NotificationRegistry.kt
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorSelectionActivity.kt Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
@sunkup sunkup linked an issue Apr 30, 2026 that may be closed by this pull request
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

core/src/androidTest/kotlin/at/bitfire/davdroid/push/PushDistributorManagerTest.kt:92

  • issue (blocking): testGetDistributor_PushEnabled_NoDistributorToUse doesn't stub UnifiedPush.resolveDefaultDistributor()

getDistributorToUse() calls resolveDefaultDistributor() when getAckDistributor() returns null, so this test will hit an un-stubbed static UnifiedPush method and typically fails under MockK. Stub resolveDefaultDistributor() to a deterministic value (and consider adding dedicated tests for Found/ToSelect/NoneAvailable).

    fun testGetDistributor_PushEnabled_NoDistributorToUse() {
        // Given push is enabled but no distributor is set
        settingsManager.putBoolean(Settings.PUSH_ENABLED, true)
        every { UnifiedPush.getAckDistributor(any()) } returns null

        // When getting current distributor
        val result = pushDistributorManager.getDistributorToUse()
        

Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
ArnyminerZ and others added 3 commits April 30, 2026 11:47
…anager.kt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rker-to-actively-select-distributor-new' into 2153-push-update-registration-worker-to-actively-select-distributor-new
…vely-select-distributor-new

# Conflicts:
#	core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt
@ArnyminerZ ArnyminerZ requested a review from sunkup May 3, 2026 10:15
Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

The earlier issue is unfortunately still present. I think it would be good to address it in this PR, otherwise we should create an issue for it - which is also fine.

issue: sunup (+other distributors?) installed after DAVx5 will become default, but won't register.

  • install davx5 on device with play services (chosen as default)
  • install sunup (sunup is automatically chosen as default)
  • setup account with push capable collections
  • -> registration with sunup does not happen. Not even when enabling/disabling or refreshing collections.

--

I found another problem with the curent version. See this video:

Details
Screencast.From.2026-05-04.09-50-24.mp4

The notification reappears when selecting a non-default distributor and the choice is not persisted / reset?.

ArnyminerZ added 2 commits May 6, 2026 10:37
…vely-select-distributor-new

# Conflicts:
#	core/src/main/kotlin/at/bitfire/davdroid/ui/push/PushSettingsModel.kt
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
ArnyminerZ added 4 commits May 8, 2026 08:59
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
@ArnyminerZ
Copy link
Copy Markdown
Member Author

The notification reappears when selecting a non-default distributor and the choice is not persisted / reset?.

I think this was caused only for FCM, because the choice was not being persisted. Fixed

@ArnyminerZ ArnyminerZ requested review from cketti and sunkup May 8, 2026 07:08
@ArnyminerZ
Copy link
Copy Markdown
Member Author

The earlier issue is unfortunately still present. I think it would be good to address it in this PR, otherwise we should create an issue for it - which is also fine.

I've skipped this on this PR (installing a distributor after automatically having selected FCM) to keep this one shorter, and to further discuss behavior if needed

Copy link
Copy Markdown
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Works well now. Only minor things.

Comment thread core/src/main/kotlin/at/bitfire/davdroid/ui/push/PushSettingsViewModel.kt Outdated
Comment thread core/src/main/res/values/strings.xml Outdated
Comment thread core/src/main/res/values/strings.xml Outdated
ArnyminerZ added 4 commits May 8, 2026 11:02
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
@sunkup
Copy link
Copy Markdown
Member

sunkup commented May 8, 2026

The earlier issue is unfortunately still present. I think it would be good to address it in this PR, otherwise we should create an issue for it - which is also fine.

I've skipped this on this PR (installing a distributor after automatically having selected FCM) to keep this one shorter, and to further discuss behavior if needed

Very well. I have created #2199

Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
Comment thread core/src/main/kotlin/at/bitfire/davdroid/push/PushDistributorManager.kt Outdated
ArnyminerZ added 4 commits May 8, 2026 12:22
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
…rker-to-actively-select-distributor-new' into 2153-push-update-registration-worker-to-actively-select-distributor-new
@ArnyminerZ ArnyminerZ merged commit 33fa443 into main May 8, 2026
5 checks passed
@ArnyminerZ ArnyminerZ deleted the 2153-push-update-registration-worker-to-actively-select-distributor-new branch May 8, 2026 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature Implements a new feature or functionality (only for PRs) push Related to WebDAV-Push

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Push] Update registration worker to actively select distributor

4 participants