CCCT-1773 Tweak Message Card Banner Colors

[conroy-ricketts]

CCCT-1773

Product Description

I tweaked the message card colors in the event that the user is returning to a Learn app after successfully completing the associated assessment.

Technical Summary

The main issue here is that we perviously assumed that all messages to be displayed on the message card are warning messages or negative in a sense. So I refactored the code a bit to reflect the fact that the message card is not always a warning or negative.

Also, I noticed that we had an incorrect non-English translation for this particular message card - so I went ahead and updated that.

Safety Assurance

Safety story

I verified that both the text color and the background color of the message card were updated.

QA Plan

I think that it would be worth it for QA to test all paths/flows where this message card banner appears to ensure we are always displaying the correct color scheme (whether that be a positive color scheme or a negative color scheme).

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the Connect job delivery messaging system from warning-focused to a generic card message approach. It updates the Tigrinya language translation for a delivery readiness message, renames getWarningMessages() to getCardMessageText() in the job data model, and modifies UI controllers to conditionally style the message card based on delivery status (green styling for delivery-ready states, orange for warnings).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• StandardHomeActivityUIController.java — Introduces conditional styling logic with multiple new imports and color/background assignments based on delivery status; verify styling logic consistency
• ConnectJobRecord.java — Public method rename (getWarningMessages() → getCardMessageText()) requires verification that all call sites are updated correctly
• ConnectDeliveryProgressFragment.java — Message handling refactor from updateWarningMessage() to updateCardMessage(); verify visibility and text display behavior
• Localization file — Cross-check Tigrinya translation accuracy and ensure the translated string is contextually appropriate for the delivery-ready state

Possibly related PRs

• Delivery progress scrolling fix #3009 — Modifies message card UI in ConnectDeliveryProgressFragment and introduces getCardMessageText() method usage with layout restructuring
• Additional tweaks to delivery progress text #3010 — Updates ConnectDeliveryProgressFragment and the same Tigrinya string resource (connect_progress_ready_for_transition_to_delivery)
• Learn-to-deliver transition improvement #2924 — Modifies StandardHomeActivityUIController message/card logic and Tigrinya string translations with overlapping UI changes

Suggested labels

skip-integration-tests

Suggested reviewers

• OrangeAndGreen
• Jignesh-dimagi
• shubham1g5
• jaypanchal-13

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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 (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "CCCT-1773 Tweak Message Card Banner Colors" directly describes the primary change in the pull request—adjusting the colors of the message card banner based on context (green for successful completion, orange for warnings). The title is concise, specific, and clearly summarizes the main change without vague terminology. It accurately captures the refactoring effort to treat message cards as more than just warnings and the associated UI styling updates.
Description Check	✅ Passed	The pull request description addresses most of the required template sections. The Product Description is present with a visual illustration of the change. The Technical Summary includes the ticket link (CCCT-1773), rationale for the refactoring (treating message cards as non-negative), and mentions the translation fix. The Safety Assurance section includes both a safety story (verification of text and background color updates) and a QA Plan (testing all paths where the message card appears). However, the automated test coverage subsection is not addressed, and the final checklist items are not completed. Despite these gaps, the description is substantially complete with the most critical information clearly articulated.

✨ 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 task/CCCT-1773-tweak-successful-learn-task-completion-banner-color

---

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/src/org/commcare/activities/StandardHomeActivityUIController.java (1)

154-157: RecyclerView can stay hidden after state change to DELIVERING.

You only set GONE when not delivering/finished; there’s no else to restore VISIBLE when the job later becomes deliverable. Add an else branch.

 RecyclerView recyclerView = viewJobCard.findViewById(R.id.rdDeliveryTypeList);
 if (job.getStatus() != STATUS_DELIVERING || job.isFinished()) {
     recyclerView.setVisibility(View.GONE);
+} else {
+    recyclerView.setVisibility(View.VISIBLE);
 }

🧹 Nitpick comments (2)

app/src/org/commcare/activities/StandardHomeActivityUIController.java (2)

105-113: Redundant app-record fetch used only as a gate; align behavior with fragment.

You fetch ConnectAppRecord solely to gate message rendering, but the record isn’t otherwise used. This performs an extra DB read and creates divergence from ConnectDeliveryProgressFragment (which doesn’t gate). Either:

• Gate explicitly on login/eligibility (e.g., PersonalIdManager) and document it, or
• Drop the gate and rely on job.getCardMessageText(...) being null when nothing should show.

Consider simplifying:

-String messageText = null;
-String appId = CommCareApplication.instance().getCurrentApp().getUniqueId();
-ConnectAppRecord record = ConnectJobUtils.getAppRecord(activity, appId);
-ConnectJobRecord job = activity.getActiveJob();
-
-if (job != null && record != null) {
-    messageText = job.getCardMessageText(activity);
-}
+ConnectJobRecord job = activity.getActiveJob();
+String messageText = job != null ? job.getCardMessageText(activity) : null;

If you keep the gate, please ensure it’s intentional and consistent with the fragment.

---

115-135: Guard against empty strings and consider extracting shared color logic.

Use TextUtils.isEmpty to avoid rendering blank messages, and consider extracting the color-pick logic into a helper to reuse in fragments.

+import android.text.TextUtils;
@@
-        if (messageText != null) {
+        if (!TextUtils.isEmpty(messageText)) {
             @ColorInt int textColor;
             @ColorInt int backgroundColor;
@@
             connectMessageCard.setVisibility(View.VISIBLE);
         } else {
             connectMessageCard.setVisibility(View.GONE);
         }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50bf05b and ca9832b.

📒 Files selected for processing (4)

• app/res/values-ti/strings.xml (1 hunks)
• app/src/org/commcare/activities/StandardHomeActivityUIController.java (2 hunks)
• app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java (9 hunks)
• app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (6 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-07-29T14:11:36.386Z

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3248
File: app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java:0-0
Timestamp: 2025-07-29T14:11:36.386Z
Learning: In ConnectJobsListsFragment.java error handling for JSON parsing, if the JSONObject obj is null when passed to handleCorruptJob(), the team prefers to let the code crash rather than adding defensive null checks. This follows the fail-fast philosophy used throughout the Connect feature to catch programming errors immediately rather than masking them.

Applied to files:

• app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java

📚 Learning: 2025-05-08T11:08:18.530Z

Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

Applied to files:

• app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java

🧬 Code graph analysis (1)

app/src/org/commcare/activities/StandardHomeActivityUIController.java (2)

app/src/org/commcare/CommCareApplication.java (1)

• CommCareApplication (157-1279)

app/src/org/commcare/connect/database/ConnectJobUtils.java (1)

• ConnectJobUtils (25-383)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint Code Base

🔇 Additional comments (2)

app/res/values-ti/strings.xml (1)

327-327: Verify translation intent and duplication with connect_progress_warning_not_started.

The new Tigrinya for connect_progress_ready_for_transition_to_delivery looks positive, but connect_progress_warning_not_started (Line 326) currently shows identical text. These keys convey different states; they likely shouldn't be the same. Please confirm with localization and adjust if needed. Also ensure punctuation/spacing matches project conventions.

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

617-636: Verify old getWarningMessages() method has been fully migrated.

The search returned no references to the old getWarningMessages() method, which suggests the migration may be complete. However, this cannot be definitively confirmed through automated search. Please manually verify that:

• The old getWarningMessages(Context) method has been removed or properly deprecated in ConnectJobRecord.java
• All call sites have been migrated to use the new getCardMessageText(Context) method
• No dynamic or reflection-based invocations remain

[OrangeAndGreen]

Looks pretty good and thanks for also addressing some lint. One question to check on.

[shubham1g5]

nothing blocking from me.

[shubham1g5]

@damagatchi retest this please

[conroy-ricketts]

@damagatchi retest this please

[conroy-ricketts]

@damagatchi retest this please

[shubham1g5]

@damagatchi retest this please