Trigger app sync after a task change

[shubham1g5]

Product Description

https://dimagi.atlassian.net/browse/CCCT-2541

Every time user enters an app, we trigger an automated sync if a task has changes since the last time an app sync was performed.

task_sync.mp4

Technical Summary

Spec

Safety Assurance

Safety story

Tested locally by changing tasks and verified that sync occurs only when a task has changed

Automated test coverage

new behaviour is covered with tests, any regressions should be flagged by existing CC side integration tests.

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, RELEASES.md is updated accordingly
• Does the PR introduce any major changes worth communicating ? If yes, RELEASES.md is updated accordingly
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[shubham1g5]

@coderabbitai full review please

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 3791 --repo dimagi/commcare-android --json title,body,files,additions,deletions,changedFiles,baseRefName,headRefName
echo "---DIFF---"
gh pr diff 3791 --repo dimagi/commcare-android

Length of output: 11057

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,140p' app/src/org/commcare/connect/database/ConnectTaskUtils.kt

Length of output: 5345

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "---ConnectJobHelper.getJobForSeatedApp---"
rg -n "getJobForSeatedApp" -A 20 app/src/org/commcare/connect/ConnectJobHelper.kt 2>/dev/null || rg -n "getJobForSeatedApp" -A 20 --type=java --type=kotlin -g '*.kt' -g '*.java' .

echo "---SyncDetailCalculations.getLastSyncTime---"
rg -n "getLastSyncTime" -A 15 app/src/org/commcare/utils/SyncDetailCalculations.kt 2>/dev/null || rg -n "fun getLastSyncTime" -A 15 -g '*.kt' .

echo "---ConnectJobPreferences taskModifiedTime---"
rg -n "TaskModifiedTime|TIMESTAMP_NOT_SET" -A 5 app/src/org/commcare/preferences/ConnectJobPreferences.kt

Length of output: 6414

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -i "SyncDetailCalculations" 
echo "---"
rg -n "getLastSyncTime|class SyncDetailCalculations|object SyncDetailCalculations" -A 8 $(fd -i "SyncDetailCalculations.kt")

Length of output: 564

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "getLastSyncTime|class SyncDetailCalculations" -A 10 app/src/org/commcare/utils/SyncDetailCalculations.java

Length of output: 3103

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "checkForPendingAppHealthActions" -B5 -A 40 app/src/org/commcare/activities/HomeScreenBaseActivity.java | head -100

Length of output: 4902

---

This looks well-integrated: checkForPendingAppHealthActions() runs once per session (guarded by appHealthChecksCompleted()), so the added getJobForSeatedApp + preference reads won't add per-frame overhead — good design choice.

A couple of things worth double-checking with the author, plus one minor nit:

1. isLastTaskUpdateLaterThanLastSync returns true when SyncDetailCalculations.getLastSyncTime() is 0 (never synced) and a task exists with updateTaskModifiedTime() set. That seems intentional/harmless since CommCareApplication.instance().isSyncPending() would likely already be true in a fresh install, but worth confirming this doesn't cause a redundant duplicate sync trigger on first login when isSyncPending() is also true (not a bug, just double-checking there's no unintended double-trigger down the line).
2. getLastSyncTime() is keyed by AppUtils.getLoggedInUserName() (per-user), while getTaskModifiedTime() is stored per job (ConnectJobPreferences). Since both are scoped correctly to the seated app/user context here, this looks fine, just flagging the cross-source comparison for visibility.
3. Nice test coverage added in ConnectTaskUtilsTest.kt covering all four branches (no job, non-delivering job, unset timestamp, before/after last sync) — no additional cases needed here.

Overall the change is small, well-scoped, and consistent with existing patterns in ConnectTaskUtils.kt (e.g., shouldShowTasksCompletedMessage follows the same job/prefs lookup pattern). No blocking issues found.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.57%. Comparing base (9bacba5) to head (d20f3e7).
⚠️ Report is 174 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3791      +/-   ##
============================================
+ Coverage     26.14%   26.57%   +0.43%     
- Complexity     4462     4581     +119     
============================================
  Files           954      971      +17     
  Lines         57383    58018     +635     
  Branches       6830     6909      +79     
============================================
+ Hits          15002    15421     +419     
- Misses        40545    40715     +170     
- Partials       1836     1882      +46     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds a new utility function isLastTaskUpdateLaterThanLastSync in ConnectTaskUtils that checks whether a seated job's task modification time is later than the last sync time. HomeScreenBaseActivity is updated to trigger a sync when this condition is true, alongside reorganized static imports. Corresponding unit tests are added to cover the new function's behavior across various scenarios.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HomeScreenBaseActivity
  participant ConnectTaskUtils
  participant ConnectJobHelper
  participant SyncDetailCalculations

  HomeScreenBaseActivity->>ConnectTaskUtils: isLastTaskUpdateLaterThanLastSync(context)
  ConnectTaskUtils->>ConnectJobHelper: getJobForSeatedApp()
  ConnectJobHelper-->>ConnectTaskUtils: seated job
  alt job seated and STATUS_DELIVERING and task modified time set
    ConnectTaskUtils->>SyncDetailCalculations: getLastSyncTime()
    SyncDetailCalculations-->>ConnectTaskUtils: last sync time
    ConnectTaskUtils-->>HomeScreenBaseActivity: comparison result
  else conditions not met
    ConnectTaskUtils-->>HomeScreenBaseActivity: false
  end
  HomeScreenBaseActivity->>HomeScreenBaseActivity: triggerSync(true) if true

Loading

Suggested reviewers: OrangeAndGreen, Jignesh-dimagi

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: triggering sync after a task change.
Description check	✅ Passed	The PR description follows the template well, covering product impact, technical summary, safety story, tests, and review checklist.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ccct-2541_task_triggered_sync

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[conroy-ricketts]

Can we update the safety story as well

[Jignesh-dimagi]

The static imports actually got relocated to the top of the file. However, .github/linters/checkstyle.xml prefers them at the bottom. Did they move automatically?

[shubham1g5]

thanks for flagging, reverted this here - 15f531b , It was triggered by rearrange imports shortcut on mac (ctrl + option + O) which doesn't seem to be taking into account the code style file

The base branch was changed.