revert change to only count approved visits

[shubham1g5]

Product Description

Reverts https://github.com/dimagi/commcare-android/pull/3286/files based on convo here

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
• Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

📝 Walkthrough

Walkthrough

A conditional status check has been removed from the ConnectJobRecord.numberOfDeliveriesToday() method. Previously, the method incremented the daily visit count only for deliveries with STATUS_APPROVED. Now it increments the count for all deliveries matching today's date, regardless of their approval status, changing the counting behavior in this method.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The change is a single conditional removal affecting one method in one file. However, reviewers should verify that removing the status filter aligns with intended behavior for delivery counting and assess any downstream impacts on features or reports that depend on this count.

Possibly related PRs

• Flagged and Rejected visits are getting counted from the total daily limit #3286: Directly conflicts with this PR by adding back a STATUS_APPROVED check to count only approved deliveries, while this PR removes that check entirely.

Suggested labels

QA Note

Suggested reviewers

• OrangeAndGreen
• Jignesh-dimagi

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description includes a Product Description and a Labels and Review checklist but omits several required sections from the repository template, such as the Technical Summary with a ticket link and design rationale, a Feature Flag section, and the Safety Assurance section covering safety story, automated test coverage, and QA plan.	Please update the PR description to include a Technical Summary that links to the related ticket and outlines the rationale for reverting the change. Add a Feature Flag section or note that it does not apply, and complete the Safety Assurance section with a safety story describing how this change was tested, along with automated test coverage details and a QA plan. Confirm whether a Release Note is required or state that none is needed and ensure all template sections are present to comply with repository standards.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title “revert change to only count approved visits” concisely and accurately summarizes the core change by indicating that the previous logic restricting the count to approved visits is being undone, making the intent immediately clear to reviewers.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch revertApprovedVisitChange

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java (1)

3-3: Remove unused import.

After reverting the status check, the STATUS_APPROVED constant is no longer used in this file. Consider removing this import for code cleanliness.

Apply this diff to remove the unused import:

-import static org.commcare.connect.ConnectConstants.STATUS_APPROVED;
-
 import android.content.Context;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91732a0 and b42406c.

📒 Files selected for processing (1)

• app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java (1 hunks)

🔇 Additional comments (1)

app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java (1)

566-576: Verify the behavioral change and test coverage.

The removal of the status check means numberOfDeliveriesToday() now counts all deliveries for today, regardless of approval status. This affects the daily limit warning logic at line 626, where users may now see "daily max reached" warnings earlier (based on all submissions rather than just approved ones).

Since this is a revert of PR #3286 based on the product decision in QA-8158, please ensure:

1. The behavioral change has been thoroughly tested, especially the daily limit warning scenarios
2. All places that depend on this count have been reviewed for consistency
3. The change aligns with the expected product requirements

Run the following script to identify other locations that might use similar status-based filtering logic for deliveries: