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

Simplify Enterprise OData Report #35520

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

Conversation

mjriley
Copy link
Contributor

@mjriley mjriley commented Dec 12, 2024

Product Description

The previous OData Feed report contained summary fields which made normal tabular data awkward. This PR shifts the summary fields to the Domain report while modernizing the remaining fields.

Domain Report:
image

Domain API:
image

OData Feeds Report:
image

OData Feeds Report, too many exports:
image

OData Feeds API:
image

Technical Summary

This is being tracked by mainly https://dimagi.atlassian.net/browse/SAAS-16358 and https://dimagi.atlassian.net/browse/SAAS-16357

Safety Assurance

Safety story

Verified these changes locally

Automated test coverage

Updated previous OData Report tests

QA Plan

No QA

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

Copy link

sentry-io bot commented Dec 12, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: corehq/apps/enterprise/enterprise.py

Function Unhandled Issue
rows_for_domain TypeError: '<=' not supported between instances of 'datetime.datetime' and 'str' /a/{domain}/en...
Event Count: 3
rows_for_domain ESError: ConnectionTimeout caused by - ReadTimeoutError(HTTPConnectionPool(host='10.202.41.10', port=9200)... ...
Event Count: 1
rows_for_domain UnboundLocalError: local variable 'version' referenced before assignment /a/{domain}/enterprise...
Event Count: 1

Did you find this useful? React with a 👍 or 👎

@mjriley mjriley added the product/admin Change affects admin pages only visible to super users / staff label Dec 12, 2024
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 just a small question

num_feeds_available = fields.IntegerField(null=True)
report_name = fields.CharField(null=True)
domain = fields.CharField()
report_name = fields.CharField()
report_rows = fields.IntegerField(null=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw null=True is removed from domain and report_name, why report_rows still have it? I checked report_rows will be CaseExportInstance or FormExportInstance's self.get_query().count(), is it possible for this to return Null ?

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.

2 participants