[MS-1418] Replace WorkManager location collection with in-process coroutine

[meladRaouf]

skip it and focus on writing the PR description instead.

MS-1418
Will be released in: 2026.3.0

Notable changes

• Replaced the WorkManager-based location collection with a lightweight in-process coroutine implementation.
• Added logging around the lifecycle of location collection (Started / Saved ... (took Xms) / Finished / Stopped) to make it easier to trace timing and diagnose issues in Crashlytics.
• Tested manually and confirmed the timing is nearly identical between the WorkManager-based approach and the new implementation.

Testing guidance

• Start any flow and confirm the user's location is still saved into the session (check session location fields / logs tagged CollectLocationUseCase).
• Verify cancelling setup early (before location resolves) doesn't crash and logs Stopped collecting location.

Additional work checklist

• Effect on other features and security has been considered
• Design document marked as "In development" (if applicable)
• External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
• Test cases in Testiny are up to date (or ticket created)
• Other teams notified about the changes (if applicable)

[Copilot]

Pull request overview

This PR replaces the setup-time location collection that previously ran via WorkManager with an in-process coroutine-based implementation, aiming to keep behavior/timing similar while adding more detailed lifecycle logging.

Changes:

• Removed the WorkManager worker + scheduler used for saving the user’s location into the current session.
• Introduced CollectLocationUseCase (in-process coroutine job) and LocationStoreImpl (tracks/cancels the active collection job).
• Updated/added unit tests to cover the new implementation (with some needed adjustments for deterministic coroutine testing).

Reviewed changes

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

Show a summary per file

File	Description
feature/setup/src/main/java/com/simprints/feature/setup/SetupModule.kt	Switches DI binding from the WorkManager scheduler to the new in-process LocationStoreImpl.
feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt	New coroutine-based location collection with lifecycle timing logs and cancellation handling.
feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreImpl.kt	New LocationStore implementation that starts/cancels a Job instead of enqueuing/cancelling WorkManager work.
feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreWorkerScheduler.kt	Deleted: previous WorkManager enqueue/cancel implementation.
feature/setup/src/main/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorker.kt	Deleted: previous Worker implementation for collecting/saving location.
feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt	Updated tests for the new use case (needs coroutine-test synchronization to avoid flakiness).
feature/setup/src/test/java/com/simprints/feature/setup/location/LocationStoreImplTest.kt	New tests for job replacement/cancellation behavior in LocationStoreImpl.

Suppressed comments (4)

feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt:12

• The tests call a coroutine-based use case but don't use runTest, and the file doesn't import coroutine-test utilities needed to deterministically run/join the launched job. Adding runTest (and MutableSharedFlow for a cancellation test) will make the tests reliable and allow awaiting the returned Job.

This issue also appears in the following locations of the same file:

• line 39
• line 48
  feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt:43

• CollectLocationUseCase() launches work in the provided scope and returns immediately; this test asserts before waiting for the returned Job to complete, which can make it flaky (or fail) depending on dispatcher behavior. Wrap the test in runTest and join() the returned job before verifying.
  feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt:52
• Like the previous test, this one doesn't wait for the coroutine launched by collectLocation() to finish. Joining the returned Job inside runTest ensures the assertions run after the coroutine has handled the exception path.
  feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt:65
• The cancellation path is part of the new in-process implementation (and the old worker had an explicit cancellation test), but it's not covered here. Adding a runTest-based test that cancels the returned Job before any emission helps prevent regressions where cancellation still results in writing a location.

[luhmirin-s]

Tested manually and confirmed the timing is nearly identical between the WorkManager-based approach and the new implementation.

If the timing is nearly identical, is there any good reason to change how it works already?

[luhmirin-s]

I think the whole point was to do the collection coroutine in the context/scope of the caller to get the benefit of structured concurrency and avoid tracking this job manually.

[meladRaouf]

My Idea was that the collectLocationInBackground gets started in the setup fragment and gets canceled from the orchestrator. That is why I needed ahigher context than the caller so I used AppScope.
I implemented it in a nicer way now in LocationStoreImpl

[BurningAXE]

I think the expected result is for location collection to run as long as the session is active. If we switch it to cancel as soon as Setup is done, that's a behaviour change and likely a negative one.

[meladRaouf]

@BurningAXE The cancellation is happening as before, at the end of the orchestration fragment rather than during the setup.

[luhmirin-s]

@BurningAXE technically, it would be until location is received or session interrupted. This implementation seems to be slightly simpler to understand, although I am surprised that the time benefits are negligible.

[BurningAXE]

I was commenting on the suggestion to use the scope of the caller which is the Setup VM. Would have been easier if we had a dedicated session scope. (We do have SessionCoroutineScope but that one is strictly reserved for events and I don't think this single use case merits creating a new one)

[meladRaouf]

Tested manually and confirmed the timing is nearly identical between the WorkManager-based approach and the new implementation.

If the timing is nearly identical, is there any good reason to change how it works already?

There is definitely some overhead involved in creating and cancelling the WorkManager, but it’s not noticeable in practice.

[Copilot]

Pull request overview

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

Suppressed comments (2)

feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt:25

• runCatching also consumes CancellationException from updateSessionScopeLocationUseCase. If cancellation arrives while the final/current location is being saved, collection completes and logs Finished instead of reaching the Stopped branch; ordinary save failures are likewise silently reported as a successful finish. Let the surrounding catches handle the save directly, and add a regression test where the updater throws CancellationException.

                .collect { location -> runCatching { saveUserLocation(location, requestStartTimeMs) } }

feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt:43

• Elapsed durations should use a monotonic clock. currentTimeMillis() can jump when Android synchronizes or the user changes wall-clock time, producing negative or inflated timing logs and undermining the diagnostics added here. Capture and subtract SystemClock.elapsedRealtime() (or TimeSource.Monotonic) consistently instead.

    private fun elapsedMs(requestStartTimeMs: Long) = System.currentTimeMillis() - requestStartTimeMs

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[luhmirin-s]

Wouldn't the runCatching() wrap and hide the cancellation exception, or any other error? Which means that the outer try-catch is useless.