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

Add Enterprise Case Management Report #35482

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mjriley
Copy link
Contributor

@mjriley mjriley commented Dec 5, 2024

Product Description

Creates a new Enterprise Console Tile and API for the Case Management Report

Console:
image

Generated Report:
image

API:
image

Technical Summary

Associated ticket: https://dimagi.atlassian.net/browse/SAAS-16336. Also covers the Tile and API tickets.

Feature Flag

None

Safety Assurance

Safety story

Tested locally

Automated test coverage

Automated tests to be added later, once the logic is extracted from the report to a general location.

QA Plan

TBD

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

@mjriley mjriley added the product/admin Change affects admin pages only visible to super users / staff label Dec 5, 2024
@mjriley mjriley changed the title Mjr/case management tile Add Enterprise Case Management Report Dec 5, 2024
nospame
nospame previously approved these changes Dec 6, 2024
Copy link
Contributor

@nospame nospame left a comment

Choose a reason for hiding this comment

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

Looks good to me. Two clarifying questions (for my own understanding)

def app_query(self, domain):
return (
AppES().domain(domain)
.filter(filters.term('doc_type', 'Application'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to make sure we don't get LinkedApplications or RemoteApps?


def get_report_task(self, request):
account = BillingAccount.get_account_by_domain(request.domain)
return generate_enterprise_report.s(
Copy link
Contributor

Choose a reason for hiding this comment

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

What's .s() do here?

Base automatically changed from mjr/enterprise-percent-changes to master December 9, 2024 17:10
@mjriley mjriley dismissed nospame’s stale review December 9, 2024 17:10

The base branch was changed.

Copy link
Contributor

@jingcheng16 jingcheng16 left a comment

Choose a reason for hiding this comment

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

Looks good to me except for a few nit.


@property
def headers(self):
return [_('Project Space'), _('# Applications'), _('# Surveys Only'), _('# Cases Only'), _('# Mixed')]
Copy link
Contributor

Choose a reason for hiding this comment

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

# of instead of # for the last 4 columns?

Comment on lines +406 to +411
has_surveys = filters.nested('modules', filters.empty('modules.case_type.exact'))
has_cases = filters.nested('modules', filters.non_null('modules.case_type.exact'))

survey_only_count = app_query.filter(filters.AND(has_surveys, filters.NOT(has_cases))).count()
case_only_count = app_query.filter(filters.AND(has_cases, filters.NOT(has_surveys))).count()
mixed_count = app_query.filter(filters.AND(has_surveys, has_cases)).count()
Copy link
Contributor

Choose a reason for hiding this comment

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

Elegant... nice naming and nice logic


case_management_percent = num_domains_using_case_management / num_domains_with_apps * 100

return "{:.1f}%".format(case_management_percent)
Copy link
Contributor

Choose a reason for hiding this comment

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

I made a util function for display percentage with only one digit in 1aec926. Can reuse it after my PR is merged.


REPORT_SLUG = EnterpriseReport.CASE_MANAGEMENT

def get_report_task(self, request):
Copy link
Contributor

Choose a reason for hiding this comment

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

this function can be removed as I moved get_report_task to base class in 5609175

return bundle

def get_primary_keys(self):
return ('domain',) # very odd report that makes coming up with an actual key challenging
Copy link
Contributor

Choose a reason for hiding this comment

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

I think domain is a valid primary key in this case? Just don't get why it is challenging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/admin Change affects admin pages only visible to super users / staff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants