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

[EASI-4457] Dataloaders Refactor #2727

Merged
merged 28 commits into from
Aug 1, 2024
Merged

[EASI-4457] Dataloaders Refactor #2727

merged 28 commits into from
Aug 1, 2024

Conversation

mynar7
Copy link
Contributor

@mynar7 mynar7 commented Jul 31, 2024

EASI-4457

Description

  • Adds dataloaders for most of the separate db tables associated with TRB and System intake requests. This should speed up the loading of the admin home page for both System Intake and TRB admin views.
    1. TRB Status dataloader
    2. TRB Requester Component dataloader
    3. TRB Request Form dataloader
    4. TRB Advice Letter dataloader
    5. TRB all feedback dataloader
    6. TRB newest feedback dataloader
    7. TRB Attendees dataloader
    8. TRB Documents dataloader
    9. TRB Admin notes dataloader
    10. TRB Request form funding sources dataloader
    11. TRB Request form system intakes dataloader (this is intakes that are attached when filling the form through LCID lookup)
    12. System Intake funding sources dataloader
    13. System Intake notes dataloader
    14. System Intake Actions dataloader
    15. System Intake Gov Req Feedback dataloader
    16. System Intake Documents dataloader
    17. System Intake Business Case dataloader
    18. Business Case Estimated Lifecycle Costs dataloader
  • Allows for adding numerous closed intakes/TRB requests in the seed data since this is a big part of the slowdown on the home pages.

How to test this change

  • There's a variable at the top of the seed script that can be used to generate lots of requests for local testing. Starting the app and seeding with 500 or so closed requests of each type emulates the current prod behavior pretty closely. Load either admin home page to see how long the spinner remains/the query takes to run.
  • Automated tests were also updated to work with the dataloader changes.

PR Author Checklist

  • I have provided a detailed description of the changes in this PR.
  • I have provided clear instructions on how to test the changes in this PR.
  • I have updated tests or written new tests as appropriate in this PR.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • When approving a PR, provide a reason why you're approving it
    • e.g. "Approving because I tested it locally and all functionality works as expected"
    • e.g. "Approving because the change is simple and matches the Figma design"
  • Don't be afraid to leave comments or ask questions, especially if you don't understand why something was done! (This is often a great time to suggest code comments or documentation updates)
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.

mynar7 and others added 22 commits July 12, 2024 11:06
* add funding source dataloader and refactor helpers.OneToMany and Mapper interface

* fix store method, call dataloader version in resolver
add dataloader to trb request form resolver
update notes resolver to use dataloader, add tests for notes
add dataloader to system intake actions resolver
* add dataloaders to TRB Feedback

* convert slice to postgres array

* use dataloaders in status calls

* remove unnecessary no rows error check
* add dataloaders to TRB Feedback

* convert slice to postgres array

* use dataloaders in status calls

* remove unnecessary no rows error check

* add dataloaders to advice letter and finish refactoring statuses

* sort mock users

* nil check statuses
* add dataloader to trb attendees

* add dataloader for attendees by EUA and TRB ID, update requester component resolver

* use ptr helper, add to resolver test
* refactor seed data to allow for closed request generation

* remove context value and just use a constant

* add regex to LCID test
* add a dataloader for trb documents, use dataloaders in other resolvers

* semicolons and better error handling
add dataloaders for TRB admin notes
* add dataloader for trb funding sources

* fix test to allow for empty slice

* add TODO comment
add dataloader for trb request form system intakes
@mynar7 mynar7 requested review from a team as code owners July 31, 2024 15:13
@mynar7 mynar7 requested review from ClayBenson94 and aterstriep and removed request for a team July 31, 2024 15:13
@mynar7 mynar7 requested a review from adamodd July 31, 2024 15:13
@mynar7 mynar7 requested a review from samoddball July 31, 2024 15:13
* add gov req feedback dataloader

* run gql gen
* add dataloader for documents

* use resolver to create fake documents

* update seed script

* update seed script to not fetch users multiple times for closed requests
Copy link
Collaborator

@ClayBenson94 ClayBenson94 left a comment

Choose a reason for hiding this comment

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

Changes look good! I left a few comments for a few specific areas:

  • Some clarifying code comments (nothing major)
  • Some potential updates to the GetMappingVal functions that, if they're easy to make, I think might be worthwhile!

Appreciate all the hard work on this, @mynar7!

pkg/models/cedar_system.go Show resolved Hide resolved
pkg/models/system_intake.go Show resolved Hide resolved
pkg/models/trb_request.go Show resolved Hide resolved
pkg/models/user_info.go Show resolved Hide resolved
ClayBenson94
ClayBenson94 previously approved these changes Aug 1, 2024
Copy link
Collaborator

@ClayBenson94 ClayBenson94 left a comment

Choose a reason for hiding this comment

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

Appreciate the updates on this!

* refactor business case legacy code to allow for biz case and cost line dataloaders with less friction

* cleanup comments

---------

Co-authored-by: samoddball <156127704+samoddball@users.noreply.github.com>
@mynar7 mynar7 merged commit bedaf2d into main Aug 1, 2024
12 checks passed
@mynar7 mynar7 deleted the feature/EASI-4457 branch August 1, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants