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

Fixed an index error with the enterprise domains report #35799

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

mjriley
Copy link
Contributor

@mjriley mjriley commented Feb 18, 2025

Product Description

Technical Summary

This is a fix in response to https://dimagi.atlassian.net/browse/SAAS-16537.
Recently, the domains report was updated to include some data that was previously available in the OData Feeds report, which shifted the domain field down several fields. Unfortunately, I did not update the API to reference that new index, and the old index contained an integer, rather than a string, which led to a mismatch due to the expected schema.

Safety Assurance

Safety story

Verified locally that I could import the domains report into PowerBI successfully.

Automated test coverage

This should have a test covering it, but I think the effort to create a framework to test reports is probably outside the scope of this ticket when a client is currently blocked by this. The API pulls from the domains enterprise report, so the straight-forward way to test this would be to construct a full enterprise report. Unfortunately, that requires creating a billing account, domain + subscription, web users, and setting up the elasticsearch indexes just to get a report that returns data for an empty domain. If we wanted to look at a realistic domains report, we'd also need to create exports etc. Long-term, I think we could create fake generators for all this data, but right now I don't think should attempt that.

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

@mjriley mjriley added the product/invisible Change has no end-user visible impact label Feb 18, 2025
Copy link
Contributor

@millerdev millerdev left a comment

Choose a reason for hiding this comment

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

It seems fine to get this deployed ASAP to unblock that customer and then build the proper test infrastructure after the pressure is off.

@mjriley mjriley merged commit 46377f1 into master Feb 19, 2025
14 checks passed
@mjriley mjriley deleted the mjr/enterprise-domain-api-fix branch February 19, 2025 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/invisible Change has no end-user visible impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants