Skip to content
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

[Reminders app] Pass on logged in user ID to data update broadcast #2700

Merged
merged 4 commits into from
Oct 4, 2023

Conversation

avazirna
Copy link
Contributor

@avazirna avazirna commented Aug 25, 2023

Summary

This PR includes the current user ID in the data update broadcast. The goal is to prevent CommCare users from being notified over reminders they have sent to other users. There are also changes required on the Reminders app side that are in this PR.

Safety Assurance

  • I have confidence that this PR will not introduce a regression for the reasons below

@avazirna avazirna added this to the 2.54 milestone Sep 14, 2023
@avazirna
Copy link
Contributor Author

avazirna commented Oct 2, 2023

@damagatchi retest this please

// This is to be used to skip any cases that are not owned by the user but that might be
// in the database at the time of the data refresh
if (CommCareApplication.instance().getSession().isActive()) {
i.putExtra("logged-in-user-id", CommCareApplication.instance().getCurrentUserId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be cc-logged-in-user-id instead as per this PR ?

Comment on lines 25 to 26
// This is to be used to skip any cases that are not owned by the user but that might be
// in the database at the time of the data refresh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reword to - Used by external apps to do any user based filtering

@avazirna
Copy link
Contributor Author

avazirna commented Oct 4, 2023

@shubham1g5 these comments were not visible to me, were they pending still on your end?

@shubham1g5
Copy link
Contributor

yeah I only submitted them yesterday.

@avazirna avazirna merged commit bff4a1e into master Oct 4, 2023
4 of 6 checks passed
@avazirna avazirna deleted the include_loggeduser_in_data_update_broadcast branch October 4, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants