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

[webpack] Reports and report-dependent code #35599

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

Conversation

orangejenny
Copy link
Contributor

Technical Summary

This moves standard reports, UCR, admin reports, microplanning reports, other one-off reports (e.g., linked project space history and repeat records), custom reports, data interfaces, the export download page, and the view lookup table page to webpack. It's a mixture of areas that were already on requirejs and areas that were still using script tags.

I've been holding off on PRing this until some work extracting the inddex reports' inline js into a separate file was complete (#35586). I'm still working through some bugs in that work, which is why this PR is marked don't merge, but decided it's time to open this PR for review. QA is already complete.

Safety Assurance

Safety story

This is fairly risky from a UI perspective, with the risk being disabling one or more of these UIs. There's no deeper risk to data, etc. QA has done extensive testing.

Automated test coverage

Not much.

QA Plan

https://dimagi.atlassian.net/browse/QA-7353

Rollback instructions

This is a big PR that probably won't roll back cleanly for very long, but there's no technical reason not to roll it back.

  • 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

The non-minified version of datatables errors out.
This is the B3 library so I'm not going to worry about it.
Webpack will minify it.
These should now be included directly in js files where they're needed.
@orangejenny orangejenny added Open for review: do not merge A work in progress QA Passed product/invisible Change has no end-user visible impact labels Jan 9, 2025
@dimagimon dimagimon added dependencies Pull requests that update a dependency file Risk: Medium Change affects files that have been flagged as medium risk. labels Jan 9, 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's wonderful to have gotten this far!

@biyeun
Copy link
Member

biyeun commented Jan 9, 2025

looks like test failures are due to bootstrap diffs. thank you for doing this! looks great!

@orangejenny
Copy link
Contributor Author

@biyeun Totally forgot about diffs, thanks, I'll add those.

@orangejenny
Copy link
Contributor Author

There's also a real test failure, which is intriguing, I'll check that out.

These tests aren't using bundling, which means they're dependent on the ordering of script tags.
@biyeun
Copy link
Member

biyeun commented Jan 10, 2025

@orangejenny had an issue loading submissions by form report. Looks like fixed columns is missing

@orangejenny
Copy link
Contributor Author

@biyeun Thank you, I'm glad you found that before this went live. Fixed in 8962681 + f4fb700

@orangejenny orangejenny removed the Open for review: do not merge A work in progress label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file product/invisible Change has no end-user visible impact QA Passed Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants