-
-
Notifications
You must be signed in to change notification settings - Fork 45
Login to PersonalId if not while coming from notification #3364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a new localized string resource personalid_not_login_from_fcm_error across multiple locales (default, es, fr, hi, lt, no, pt, sw, ti). Updates ConnectActivity and ConnectMessagingActivity to gate their onCreate flows on PersonalIdManager login status. When logged in, they proceed with existing initialization (state/nav graph/navigation bar and message retrieval). When not logged in, they show a toast with the new string, start the PersonalId activity via startActivityForResult using REQUEST_CODE_PERSONAL_ID_ACTIVITY (1000), and finish the current activity. Imports Toast where needed and removes unconditional initialization previously executed in onCreate. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ConnectActivity
participant PersonalIdManager
participant NavController as Nav/Fragments
participant PersonalId as PersonalIdActivity
User->>ConnectActivity: onCreate()
ConnectActivity->>PersonalIdManager: isLoggedIn()
alt Logged in
ConnectActivity->>ConnectActivity: initStateFromExtras()
ConnectActivity->>ConnectActivity: updateBackButton()
ConnectActivity->>NavController: setStartDestination\nattach fragments
ConnectActivity->>NavController: retrieveMessages()
Note right of NavController: Normal flow continues
else Not logged in
ConnectActivity->>ConnectActivity: Toast(personalid_not_login_from_fcm_error)
ConnectActivity->>PersonalId: startActivityForResult(REQUEST_CODE=1000)
ConnectActivity-->>User: finish()
end
sequenceDiagram
autonumber
actor User
participant ConnectMessagingActivity
participant PersonalIdManager
participant UI as Nav Bar / Redirects
participant PersonalId as PersonalIdActivity
User->>ConnectMessagingActivity: onCreate()
ConnectMessagingActivity->>PersonalIdManager: isLoggedIn()
alt Logged in
ConnectMessagingActivity->>UI: setupNavigationBar()
ConnectMessagingActivity->>UI: handleRedirectIfNeeded()
else Not logged in
ConnectMessagingActivity->>ConnectMessagingActivity: Toast(personalid_not_login_from_fcm_error)
ConnectMessagingActivity->>PersonalId: startActivityForResult(REQUEST_CODE=1000)
ConnectMessagingActivity-->>User: finish()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-05-08T11:08:18.530ZApplied to files:
📚 Learning: 2025-05-09T10:57:41.073ZApplied to files:
🧬 Code graph analysis (2)app/src/org/commcare/activities/connect/ConnectMessagingActivity.java (1)
app/src/org/commcare/activities/connect/ConnectActivity.java (1)
⏰ 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)
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 |
shubham1g5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a code organisation comment but not blocking on it.
| }else{ | ||
| Toast.makeText(this,R.string.personalid_not_login_from_fcm_error,Toast.LENGTH_LONG).show(); | ||
| personalIdManager.launchPersonalId(this,REQUEST_CODE_PERSONAL_ID_ACTIVITY); | ||
| finish(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have a util method like requirePersonalIdLogin(activity) instead that checks for Personal Id login and if not navigate user to sign up page. That way we can simply add a single statement at the top level to these activities without duplicating this code as I think this will need to be added to other places as well soon in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 I was even thinking to create the BasePersonalIdConnectActivity or also Util but this is small changes so thought of putting directly as only 2 activities are only getting effected. I am still ok moving forward with BasePersonalIdConnectActivity where check for personalId or connect (if any) can be done, your thoughts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be good with BasePersonalIdConnectActivity too, think anything that avoids duplicating this code next time we have to do so should be good enough here. But happy for this change to get merged without that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 Ok let's create the base activity if anything major comes up, merging this changes now.
Technical Summary
https://dimagi.atlassian.net/browse/CCCT-1566
Labels and Review