Update Case Activity Report to Account for Shared Cases #34464

zandre-eng · 2024-04-19T11:42:38Z

Product Description

A new filter is added to change whether the cases in the report should be grouped by user or group/location:

This filter only appears if the domain has case sharing enabled. Additionally, a notice will be shown on the report if the above filter is enabled:

When the view by filter is set to "Groups" the report will only show groups/locations (users will not be shown):

It should be noted that even if one or more users are selected to filter by, the report will simply default to showing all groups/locations.

Technical Summary

Link to ticket here.

The "Case Activity" report has been reworked to support showing report data for cases that are owned by case sharing groups or locations. This is different from something like the Case List report where filtering by a group will also show cases owned by a mobile worker that belong in a specific group.

Given how tightly integrated the report is with expecting user data, some rework has been done to support both users and groups/locations. This includes:

A more generic RowData class for holding user/group data such as name and ID.
Property functions specific to users, such as users_by_id have all been refactored (e.g. owners_by_id) to handle either users or groups/locations
Overriding the get_raw_user_link() function to accommodate getting links for groups/locations as well as users.

Feature Flag

None

Safety Assurance

Safety story

Local testing
QA done

Automated test coverage

No automated tests exist for this report.

QA Plan

QA has been done. Ticket is available here.

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

Charl1996 · 2024-04-22T08:39:14Z

Had a pass at the code; looks good, but I'll wait till QA is done in case there's some changes that needs to be made.

mkangia · 2024-04-22T10:48:01Z

corehq/apps/reports/standard/monitoring.py

@@ -334,15 +336,46 @@ def make_column(title, help_text, num_days):
    def selected_users(self):
        return _get_selected_users(self.domain, self.request)

+    @property
+    def has_group_filters(self):


nit: should we use "case_group" instead of group unless specifically referring to groups?
It does seem to get confusing at times if we use only "group" for case group and groups in HQ.

I can see how this might get mistaken for the Group Django model. I like your suggestion of case_group. I'll change this out.

mkangia · 2024-04-22T10:56:03Z

corehq/apps/reports/standard/monitoring.py

+            return format_html(_(
+                "Note that when viewing this report by group it will only include "
+                "cases which are assigned to a Case Sharing Group/Location. Learn "
+                "more about troubleshooting issues with Case Sharing Groups "


troubleshooting? Confused by that term in a report.
Why are we adding it here?

I pulled this text from the acceptance criteria of the linked Jira ticket, but you make a good point. There's nothing that went wrong, so I can see it being confusing that we're linking to troubleshooting documentation.

I had an offline discussion with Mary regarding the above text and will be changing it to:

Note that when viewing this report by group it will only include cases which are assigned to a Case Sharing Group/Location. To learn more about Case Sharing click on this documentation.

The above change serves to link the user to more general documentation if they'd like to learn more about Case Sharing.

mkangia · 2024-04-24T10:01:29Z

corehq/apps/reports/standard/monitoring.py

+                "cases which are assigned to a Case Sharing Group/Location. To learn "
+                "more about Case Sharing click on "
+                "<a href='{}' target='blank'>this</a> "
+                "documentation."


nit:
Just thinking about more standard ways:

To learn more about Case Sharing click <a href='{}' target='blank'>here</a>.

More about Case Sharing on <a href='{}' target='blank'>this</a> documentation.

mkangia · 2024-04-24T10:01:53Z

corehq/apps/reports/standard/monitoring.py

mkangia · 2024-04-24T10:07:14Z

corehq/apps/reports/standard/monitoring.py

+    @property
+    @memoized
+    def view_by_groups(self):
+        return self.request.GET.get('view_by', None) == 'groups'


nit: isn't the default value already None?

This is true. I put this in for better readability on what the default is (also done in other places such as here). No strong feelings with keeping this format however, so I'll remove the default value.

mkangia

Thanks for the follow up @zandre-eng

Just minor non blocking suggestions.

mkangia · 2024-04-24T10:09:31Z

corehq/apps/reports/standard/monitoring.py

+    @property
+    @memoized
+    def selected_groups(self):
+        slugs = EMWF.get_value(self.request, self.domain)


nit: we need slugs only if self.has_case_group_filters is true? So, it can move into the if block?

mkangia · 2024-04-24T10:12:07Z

corehq/apps/reports/standard/monitoring.py

            .size(0)
        )
+        query = query.owner(owner_ids) if self.view_by_groups else query.user_ids_handle_unknown(owner_ids)
+


nit: using if block was definitely more readable like done here.

mkangia · 2024-04-24T10:13:47Z

corehq/apps/reports/static/reports/js/case_activity.js

+            const $specialNotice = $("#report-special-notice");
+            if ($specialNotice) {
+                const urlParams = new URLSearchParams(window.location.search);
+                (urlParams.get('view_by') === 'groups') ? $specialNotice.show() : $specialNotice.hide();


Does this make it possible to show/hide the banner instantly when user selects a certain option in the UI? or its just at the time of page load?
if its latter, might be better to just add/skip the section in the template.

This only gets called when the user clicks "Apply" on the filters or when the page is loaded/refreshed. We could try and get the value from the dropdown and use JQuery to instantly show the banner. This would look something like this:

$(document).ajaxSuccess(function (event, xhr, settings) { if (settings.url.match(/reports\/async\/case_activity/)) { setSpecialNoticeVisibility(); $('#report_filter_view_by').on("change", setSpecialNoticeVisibility); } }); function setSpecialNoticeVisibility() { const $specialNotice = $("#report-special-notice"); if ($specialNotice) { const viewByOption = $('#report_filter_view_by').val(); (viewByOption === 'groups') ? $specialNotice.show() : $specialNotice.hide(); } }

if its latter, might be better to just add/skip the section in the template.

It is a bit challenging to handle the show logic in the template since this only gets rendered on page load. The only other alternative would be to use something like knockout to dynamically show the banner.

Given the above, I'm not sure if all the extra code justifies showing the banner instantly since the user will still see it when it's relevant. Still, no strong feelings in changing this so that the banner is shown instantly. @mkangia What are your thoughts?

Okay, just checking.
How about we keep it simple and just show that message as a help text on that option itself next to "View by users/groups"? in a information icon? and not as a banner that we show/hide.

This filter is also being used in the Worker Activity report here. The Worker Activity report data includes cases which aren't directly owned by a specific group, and so the help text wouldn't hold true in this scenario.

Okay.

Fine to leave it as it is @zandre-eng. Thanks for talking this through.

mkangia · 2024-04-24T18:53:14Z

corehq/apps/reports/standard/monitoring.py

@@ -515,7 +515,7 @@ def rows(self):

    @property
    def get_all_rows(self):
-        es_results = self.es_queryset(user_ids=self.owner_ids)
+        es_results = self.es_queryset(owner_ids=self.owner_ids)


was this a bug?

It was. Silly mistake on my end where I forgot to update the param name for the function call.

Cool, no worries. Glad that it was caught 👍

zandre-eng added 10 commits April 19, 2024 13:09

add filter to view report by groups

6ffa4fa

change header/footer text if viewing by groups

14f55f7

create generic class for users and group/location data

683946a

add properties for retrieving selected groups/locations

7217031

refactor prop for retrieving selected users to utilize generic class

3f67f18

refactor property funcs to retrieve owners instead of users

f841fe7

refactor query func to use owners

d522dfa

refactor row funcs to use owners

2808dad

support getting filter link for groups/locations

a7632bc

change special notice message to highlight viewing by group

2af4844

zandre-eng added awaiting QA QA in progress. Do not merge product/all-users-all-environments Change impacts all users on all environments labels Apr 19, 2024

zandre-eng requested review from kaapstorm, mkangia, Charl1996 and ajeety4 April 19, 2024 11:42

zandre-eng marked this pull request as ready for review April 19, 2024 11:42

mkangia reviewed Apr 22, 2024

View reviewed changes

mkangia reviewed Apr 24, 2024

View reviewed changes

corehq/apps/reports/standard/monitoring.py

Copy link

Contributor

mkangia Apr 24, 2024

Choose a reason for hiding this comment

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

👍

mkangia reviewed Apr 24, 2024

View reviewed changes

mkangia approved these changes Apr 24, 2024

View reviewed changes

mkangia reviewed Apr 24, 2024

View reviewed changes

zandre-eng added QA Passed and removed awaiting QA QA in progress. Do not merge labels Apr 30, 2024

zandre-eng requested review from orangejenny and biyeun as code owners May 8, 2024 13:17

zandre-eng force-pushed the ze/view-by-groups-case-activity-report branch from 5937bcf to f73fa01 Compare May 8, 2024 14:32

zandre-eng added 4 commits May 8, 2024 17:03

rewording for special notice banner

88b4d64

change property function name

7e98206

code refactoring

14a9f40

fix incorrect param name

fa208da

zandre-eng force-pushed the ze/view-by-groups-case-activity-report branch from f73fa01 to fa208da Compare May 8, 2024 15:04

zandre-eng added 3 commits May 8, 2024 17:04

Merge branch 'master' into ze/view-by-groups-case-activity-report

2c71486

conditionally hide special notice

520c15c

Bootstrap 5 Migration - Rebuilt diffs

a3cc49a

mkangia approved these changes May 8, 2024

View reviewed changes

zandre-eng merged commit e90635f into master May 9, 2024
13 checks passed

zandre-eng deleted the ze/view-by-groups-case-activity-report branch May 9, 2024 07:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Case Activity Report to Account for Shared Cases #34464

Update Case Activity Report to Account for Shared Cases #34464

zandre-eng commented Apr 19, 2024 •

edited

Charl1996 commented Apr 22, 2024

mkangia Apr 22, 2024

zandre-eng Apr 22, 2024

zandre-eng Apr 22, 2024

mkangia Apr 22, 2024

zandre-eng Apr 22, 2024

zandre-eng Apr 22, 2024

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia Apr 24, 2024

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia left a comment

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia Apr 24, 2024

zandre-eng Apr 24, 2024

mkangia Apr 24, 2024

mkangia Apr 24, 2024

zandre-eng Apr 25, 2024

mkangia Apr 25, 2024

Update Case Activity Report to Account for Shared Cases #34464

Update Case Activity Report to Account for Shared Cases #34464

Conversation

zandre-eng commented Apr 19, 2024 • edited

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Charl1996 commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkangia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zandre-eng commented Apr 19, 2024 •

edited