Messaging option not visible in the side menu after signing in issue solved #3413
Conversation
📝 WalkthroughWalkthroughThis pull request consolidates notification preference cleanup, refactors navigation drawer visibility control, and modifies messaging item rendering in the drawer. PersonalIdManager now clears notification read preferences when a user is forgotten. BaseDrawerActivity introduces a Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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
🧹 Nitpick comments (1)
app/src/org/commcare/navdrawer/BaseDrawerController.kt (1)
199-214: Cache messaging channels before reuse.We call
ConnectMessagingDatabaseHelper.getMessagingChannels(activity)twice in quick succession. Stashing it in a localvalavoids duplicate DB work and makes the intent clearer, especially if the helper ever does more than read memory.- val unreadCount = - if (ConnectMessagingDatabaseHelper.getMessagingChannels(activity).isNotEmpty()) { + val messagingChannels = ConnectMessagingDatabaseHelper.getMessagingChannels(activity) + val unreadCount = + if (messagingChannels.isNotEmpty()) { ConnectMessagingDatabaseHelper.getUnviewedMessages(activity).size } else { 0 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/org/commcare/connect/PersonalIdManager.java(2 hunks)app/src/org/commcare/navdrawer/BaseDrawerActivity.kt(6 hunks)app/src/org/commcare/navdrawer/BaseDrawerController.kt(1 hunks)app/src/org/commcare/preferences/NotificationPrefs.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 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.
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T13:40:19.645Z
Learning: PR #3048 introduces a comprehensive messaging system in the Connect feature, implementing secure encryption using AES-GCM for message content, proper channel management with consent flows, and a well-designed UI separation between sent and received messages with real-time notification integration.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.
📚 Learning: 2025-05-09T10:57:41.073Z
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.
Applied to files:
app/src/org/commcare/navdrawer/BaseDrawerController.kt
🧬 Code graph analysis (1)
app/src/org/commcare/connect/PersonalIdManager.java (1)
app/src/org/commcare/preferences/NotificationPrefs.kt (1)
removeNotificationReadPref(27-31)
⏰ 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 (1)
app/src/org/commcare/navdrawer/BaseDrawerActivity.kt (1)
46-67: Nice drawer setup hook.The new
shouldShowDrawer()gate plus centralized controller wiring makes extending subclasses cleaner and removes the eager broadcast wiring—looks solid.
| fun removeNotificationReadPref(context: Context) { | ||
| context | ||
| .getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE) | ||
| .edit { remove(PREF_NAME) } |
There was a problem hiding this comment.
Fix preference removal key.
removeNotificationReadPref is removing PREF_NAME, but the stored flag lives under KEY_NOTIFICATION_READ_STATUS. As written, nothing is deleted, so a forgotten user keeps the previous read state and the new account sees stale badge visibility. Update the removal to target the actual key (or clear the file).
fun removeNotificationReadPref(context: Context) {
context
.getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE)
- .edit { remove(PREF_NAME) }
+ .edit { remove(KEY_NOTIFICATION_READ_STATUS) }
}📝 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.
| fun removeNotificationReadPref(context: Context) { | |
| context | |
| .getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE) | |
| .edit { remove(PREF_NAME) } | |
| fun removeNotificationReadPref(context: Context) { | |
| context | |
| .getSharedPreferences(PREF_NAME, Context.MODE_PRIVATE) | |
| .edit { remove(KEY_NOTIFICATION_READ_STATUS) } |
🤖 Prompt for AI Agents
In app/src/org/commcare/preferences/NotificationPrefs.kt around lines 27-30, the
removeNotificationReadPref function calls remove(PREF_NAME) which deletes the
preferences file key rather than the stored flag; change the removal to
remove(KEY_NOTIFICATION_READ_STATUS) (or call clear() on the editor if you
intend to wipe the file) and commit/apply the edit so the notification-read flag
is actually deleted.
Technical Summary
https://dimagi.atlassian.net/browse/CCCT-1633
Labels and Review