CCCT-2019 Periodic Fetch For Release Toggles #3520
Conversation
Implemented a one-time worker task to fetch release toggles in the background for anytime we fetch a new opportunity.
Used unique work to prevent duplicate toggle-fetch requests.
…nto task/CCCT-2020-fetch-toggles-on-new-opportunity
Added check for network errors so that we retry the release toggles fetch.
…nto task/CCCT-2020-fetch-toggles-on-new-opportunity
Moved logic for fetching toggles on receiving new opportunities to JobStoreManager. Reverted changes to ConnectJobHelper, ConnectUnlockFragment, ConnectJobsListsFragment, and CommCareSetupActivity.
Revert changes to ConnectUnlockFragment.
Moved call to fetch toggles out of JobStoreManager and into ConnectOpportunitiesParser. Reverted changes to JobStoreManager.
…nto task/CCCT-2020-fetch-toggles-on-new-opportunity
Implemented a worker to periodically fetch toggles every 4 hours.
conroy-ricketts
requested review from
Jignesh-dimagi,
OrangeAndGreen,
jaypanchal-13 and
shubham1g5
📝 WalkthroughWalkthroughThis pull request introduces a new background worker ( Sequence DiagramsequenceDiagram
participant App as CommCareApplication
participant PM as PersonalIdManager
participant CP as ConnectOpportunitiesParser
participant WM as WorkManager
participant CRW as ConnectReleaseTogglesWorker
participant API as PersonalIdApiHandler
App->>WM: schedulePeriodicFetch (on startup)
PM->>WM: schedulePeriodicFetch (on unlock)
CP->>WM: scheduleOneTimeFetch (after jobs)
WM->>CRW: Execute periodic/one-time fetch
CRW->>PM: Check if user logged in
PM-->>CRW: User status
CRW->>PM: Get user credentials
PM-->>CRW: Credentials
CRW->>API: Fetch release toggles
API-->>CRW: Response or Error
CRW->>WM: Return success or retry
PM->>WM: cancelPeriodicFetch (on forget user)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/org/commcare/connect/workers/ConnectReleaseTogglesWorker.kt`:
- Around line 55-90: Both scheduleOneTimeFetch() and schedulePeriodicFetch() use
the same WORK_NAME causing WorkManager to treat them as the same unique work;
create two distinct constants (e.g. WORK_NAME_ONE_TIME and WORK_NAME_PERIODIC)
and update scheduleOneTimeFetch(), schedulePeriodicFetch(), and
cancelPeriodicFetch() to use the appropriate constant (use WORK_NAME_ONE_TIME in
scheduleOneTimeFetch, WORK_NAME_PERIODIC in schedulePeriodicFetch and in
cancelPeriodicFetch) so the one-time and periodic enqueues/cancellations operate
independently.
🧹 Nitpick comments (1)
app/src/org/commcare/connect/workers/ConnectReleaseTogglesWorker.kt (1)
25-52: Consider treating “not logged in” as a no‑op to avoid marking periodic work as failed.If WorkManager treats
Result.failure()on periodic work as terminal or backoff‑heavy, this could prevent future runs while the user is logged out. Also,getUser(context)can return null; an explicit guard avoids an NPE in background.🛠️ Suggested adjustment
- if (!personalIdManager.isloggedIn()) { - return Result.failure() - } - - val user = personalIdManager.getUser(context) + if (!personalIdManager.isloggedIn()) { + return Result.success() + } + + val user = personalIdManager.getUser(context) ?: return Result.success()
| companion object { | ||
| private const val WORK_NAME = "connect_release_toggles_fetch_worker" | ||
|
|
||
| fun scheduleOneTimeFetch(context: Context) { | ||
| val workRequest = | ||
| OneTimeWorkRequest | ||
| .Builder(ConnectReleaseTogglesWorker::class.java) | ||
| .setConstraints(getWorkConstraints()) | ||
| .build() | ||
|
|
||
| WorkManager.getInstance(context).enqueueUniqueWork( | ||
| WORK_NAME, | ||
| ExistingWorkPolicy.KEEP, | ||
| workRequest, | ||
| ) | ||
| } | ||
|
|
||
| fun schedulePeriodicFetch(context: Context) { | ||
| val workRequest = | ||
| PeriodicWorkRequest | ||
| .Builder( | ||
| ConnectReleaseTogglesWorker::class.java, | ||
| 4, | ||
| TimeUnit.HOURS, | ||
| ).setConstraints(getWorkConstraints()) | ||
| .build() | ||
|
|
||
| WorkManager.getInstance(context).enqueueUniquePeriodicWork( | ||
| WORK_NAME, | ||
| ExistingPeriodicWorkPolicy.KEEP, | ||
| workRequest, | ||
| ) | ||
| } | ||
|
|
||
| fun cancelPeriodicFetch(context: Context) { | ||
| WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WorkManager enqueueUniqueWork enqueueUniquePeriodicWork same work name collision Android
💡 Result:
Yes—enqueueUniqueWork() and enqueueUniquePeriodicWork() share the same “unique work name” namespace (within your app), so using the same uniqueWorkName can collide.
- Android’s docs describe unique work as “only one instance of work with a particular name at a time”, and say it applies to both one-time and periodic work. So the name is intended to be globally unique across your app’s WorkManager, not “unique per type”. [1]
- Internally, WorkManager builds periodic unique work using the same “unique work chain” mechanism as one-time work: it converts
ExistingPeriodicWorkPolicyto anExistingWorkPolicyand creates aWorkContinuationImpl(this, uniqueWorkName, existingWorkPolicy, ...). That means the periodic work is inserted into the same unique-name conflict resolution path as one-time unique work. [3]
What happens on collision
If you call one after the other with the same name, whichever you enqueue second will apply its existing-work policy against whatever is already enqueued under that name:
- One-time uses
ExistingWorkPolicy(KEEP,REPLACE,APPEND, etc.). [2] - Periodic uses
ExistingPeriodicWorkPolicy(KEEP,CANCEL_AND_REENQUEUE, and newerUPDATE, etc.). [4]
Recommendation
Use different unique names for periodic vs one-time work (e.g., "sync_periodic" vs "sync_oneoff"), and use tags if you want to group/observe/cancel both kinds together. [1]
Sources:
[1] Android Developers – Managing work / Unique Work https://developer.android.com/develop/background-work/background-tasks/persistent/how-to/manage-work
[2] Android Developers – WorkManager.enqueueUniqueWork API https://developer.android.com/reference/androidx/work/WorkManager
[3] AOSP/AndroidX source – WorkManagerImpl.createWorkContinuationForUniquePeriodicWork(...) uses WorkContinuationImpl(..., uniqueWorkName, existingWorkPolicy, ...) https://android.googlesource.com/platform/frameworks/support/+/3739ee65f12c3d007d44999afb6786b7575aa50a/work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
[4] Android Developers – ExistingPeriodicWorkPolicy https://developer.android.com/reference/androidx/work/ExistingPeriodicWorkPolicy
🌐 Web query:
Android WorkManager unique work name global namespace one-time periodic
💡 Result:
- Scope (“global namespace”): A
uniqueWorkNameis only “global” within your app’s WorkManager database (your app/package sandbox)—it will not collide with other apps’ work, even if they use the same string name. [1] - One-time vs periodic: WorkManager supports unique names for both:
enqueueUniqueWork(name, ExistingWorkPolicy, OneTimeWorkRequest)for one-time unique work/chains [2]enqueueUniquePeriodicWork(name, ExistingPeriodicWorkPolicy, PeriodicWorkRequest)for periodic unique work [2]
WorkManager’s “unique work” concept is intended to ensure only one active instance for a given unique name. [3]
- Practical guidance: Because uniqueness is enforced by name, treat names as a single app-wide namespace and don’t reuse the same
uniqueWorkNamefor unrelated work, especially mixing “one-time” and “periodic” concepts—use distinct names/prefixes. [2] [3] - Policy differences (important when a name already exists):
- One-time uses
ExistingWorkPolicy(e.g.,KEEP,REPLACE,APPEND, etc.). [2] - Periodic uses
ExistingPeriodicWorkPolicy(e.g.,KEEP,REPLACE,UPDATE). [2] [4]
- One-time uses
Sources:
[1] Stack Overflow (app sandbox / no cross-app collisions) (stackoverflow.com)
[2] Android Developers – WorkManager API reference (developer.android.com)
[3] Android Developers – “Managing work” (Unique Work concept) (developer.android.com)
[4] Android Developers – ExistingPeriodicWorkPolicy (developer.android.com)
Citations:
- 1: https://stackoverflow.com/questions/53574260/workmanagers-uniqueworkname-in-different-apps?utm_source=openai
- 2: https://developer.android.com/reference/androidx/work/WorkManager?utm_source=openai
- 3: https://developer.android.com/develop/background-work/background-tasks/persistent/how-to/manage-work?utm_source=openai
- 4: https://developer.android.com/reference/androidx/work/ExistingPeriodicWorkPolicy?utm_source=openai
🏁 Script executed:
cat -n app/src/org/commcare/connect/workers/ConnectReleaseTogglesWorker.ktRepository: dimagi/commcare-android
Length of output: 4349
Use separate unique work names for one-time and periodic fetches.
Both scheduleOneTimeFetch() and schedulePeriodicFetch() share the same unique work name namespace in WorkManager. When both are used, the second enqueue silently applies its ExistingWorkPolicy against the first, potentially skipping one of the fetches. This defeats the intent of having independent fetch mechanisms.
🛠️ Suggested fix (separate names)
- private const val WORK_NAME = "connect_release_toggles_fetch_worker"
+ private const val WORK_NAME_ONE_TIME = "connect_release_toggles_fetch_worker_once"
+ private const val WORK_NAME_PERIODIC = "connect_release_toggles_fetch_worker_periodic"
@@
- WorkManager.getInstance(context).enqueueUniqueWork(
- WORK_NAME,
+ WorkManager.getInstance(context).enqueueUniqueWork(
+ WORK_NAME_ONE_TIME,
ExistingWorkPolicy.KEEP,
workRequest,
)
@@
- WorkManager.getInstance(context).enqueueUniquePeriodicWork(
- WORK_NAME,
+ WorkManager.getInstance(context).enqueueUniquePeriodicWork(
+ WORK_NAME_PERIODIC,
ExistingPeriodicWorkPolicy.KEEP,
workRequest,
)
@@
- WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME)
+ WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME_PERIODIC)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| companion object { | |
| private const val WORK_NAME = "connect_release_toggles_fetch_worker" | |
| fun scheduleOneTimeFetch(context: Context) { | |
| val workRequest = | |
| OneTimeWorkRequest | |
| .Builder(ConnectReleaseTogglesWorker::class.java) | |
| .setConstraints(getWorkConstraints()) | |
| .build() | |
| WorkManager.getInstance(context).enqueueUniqueWork( | |
| WORK_NAME, | |
| ExistingWorkPolicy.KEEP, | |
| workRequest, | |
| ) | |
| } | |
| fun schedulePeriodicFetch(context: Context) { | |
| val workRequest = | |
| PeriodicWorkRequest | |
| .Builder( | |
| ConnectReleaseTogglesWorker::class.java, | |
| 4, | |
| TimeUnit.HOURS, | |
| ).setConstraints(getWorkConstraints()) | |
| .build() | |
| WorkManager.getInstance(context).enqueueUniquePeriodicWork( | |
| WORK_NAME, | |
| ExistingPeriodicWorkPolicy.KEEP, | |
| workRequest, | |
| ) | |
| } | |
| fun cancelPeriodicFetch(context: Context) { | |
| WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME) | |
| companion object { | |
| private const val WORK_NAME_ONE_TIME = "connect_release_toggles_fetch_worker_once" | |
| private const val WORK_NAME_PERIODIC = "connect_release_toggles_fetch_worker_periodic" | |
| fun scheduleOneTimeFetch(context: Context) { | |
| val workRequest = | |
| OneTimeWorkRequest | |
| .Builder(ConnectReleaseTogglesWorker::class.java) | |
| .setConstraints(getWorkConstraints()) | |
| .build() | |
| WorkManager.getInstance(context).enqueueUniqueWork( | |
| WORK_NAME_ONE_TIME, | |
| ExistingWorkPolicy.KEEP, | |
| workRequest, | |
| ) | |
| } | |
| fun schedulePeriodicFetch(context: Context) { | |
| val workRequest = | |
| PeriodicWorkRequest | |
| .Builder( | |
| ConnectReleaseTogglesWorker::class.java, | |
| 4, | |
| TimeUnit.HOURS, | |
| ).setConstraints(getWorkConstraints()) | |
| .build() | |
| WorkManager.getInstance(context).enqueueUniquePeriodicWork( | |
| WORK_NAME_PERIODIC, | |
| ExistingPeriodicWorkPolicy.KEEP, | |
| workRequest, | |
| ) | |
| } | |
| fun cancelPeriodicFetch(context: Context) { | |
| WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME_PERIODIC) |
🤖 Prompt for AI Agents
In `@app/src/org/commcare/connect/workers/ConnectReleaseTogglesWorker.kt` around
lines 55 - 90, Both scheduleOneTimeFetch() and schedulePeriodicFetch() use the
same WORK_NAME causing WorkManager to treat them as the same unique work; create
two distinct constants (e.g. WORK_NAME_ONE_TIME and WORK_NAME_PERIODIC) and
update scheduleOneTimeFetch(), schedulePeriodicFetch(), and
cancelPeriodicFetch() to use the appropriate constant (use WORK_NAME_ONE_TIME in
scheduleOneTimeFetch, WORK_NAME_PERIODIC in schedulePeriodicFetch and in
cancelPeriodicFetch) so the one-time and periodic enqueues/cancellations operate
independently.
CCCT-2019
Technical Summary
At first, I wanted to wait for #3516 to be merged before implementing this because I wanted to reuse the worker class that I created, but instead of waiting, I went ahead and included those changes in this PR since it was already approved and we're just waiting on the build to run... TLDR - this PR includes changes from #3516.
To make things easier, the only new commit I added was this: 6bab11f.
Besides that, I think that these changes are fairly straightforward from the ticket.
Safety Assurance
Safety story
I've verified the following:
QA Plan
No action needed from QA on this ticket. I think that this will be tested indirectly via CCCT-2022.