[EASI-4457] TRB Request Funding Sources dataloader #2715
Conversation
mynar7
requested review from
a team and
samoddball
and removed request for
a team
| // should have no sources initially | ||
| s.Len(fetched, 0) | ||
| // current GQL schema returns null instead of an empty array for no funding sources | ||
| s.Nil(fetched) |
There was a problem hiding this comment.
For some reason the previous version of the tests worked with NotNil, but the schema dictates a null return and that causes the resolver to pass . The functions return empty slices as they should, so I'm confused why it's returning nil instead of an empty slice[] instead of null on the main branch.
There was a problem hiding this comment.
After local testing to double check this, it appears we're returning an empty slice currently despite the schema saying it's possible we can return nil. I went ahead and fixed this test to allow for an empty slice or nil here: a8ecd66
The front-end is already typed for null or [], so actually returning null like the schema specifies shouldn't be an issue. I also tested the FE locally to double check this assumption and didn't see any issues.
There was a problem hiding this comment.
I'd say this might be a good spot to just add a TODO comment in the gql schema to make the array non-nullable -- I would expect the schema to define:
# This
fundingSources: [TRBFundingSource!]!
# Instead of this, which is our current schema:
fundingSources: [TRBFundingSource!]
| if err != nil { | ||
| appcontext.ZLogger(ctx).Error( | ||
| "Failed to fetch TRB request funding sources", | ||
| zap.Error(err), | ||
| zap.String("trbRequestID", trbRequestID.String()), | ||
| ) | ||
| return nil, err | ||
| return nil, &apperrors.QueryError{ |
There was a problem hiding this comment.
are we aiming to get rid of all the apperrors stuff? i was under the impression it's a tech-debt goal
There was a problem hiding this comment.
My bad -- didn't catch this 🤦
I noted in Slack, but for posterity I will share my thoughts here as well!
I think I typically would prefer standard go errors rather than trying to (or having to...) wrap errors all the time. Perhaps we could take a look at customizing error types in some other way, but I'd be good to move in the direction away from these error types!
There was a problem hiding this comment.
I wouldn't hold up a PR for it because they're not really harmful, but maybe we could rip them out of the codebase at some point!
…p into EASI-4457/trb-request-funding-sources-dataloader
* [EASI-4457] Funding sources dataloader and mapper refactor (#2692) * add funding source dataloader and refactor helpers.OneToMany and Mapper interface * fix store method, call dataloader version in resolver * [EASI-4457] Add dataloader to TRB request form resolver (#2695) add dataloader to trb request form resolver * [EASI-4457] System intake notes dataloader (#2693) update notes resolver to use dataloader, add tests for notes * [EASI-4457] System Intake Actions dataloader (#2694) add dataloader to system intake actions resolver * [EASI 4457] - TRB Request Feedback dataloaders (#2700) * add dataloaders to TRB Feedback * convert slice to postgres array * use dataloaders in status calls * remove unnecessary no rows error check * [EASI-4457] TRB Advice letter dataloader (#2702) * 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 * [EASI-4457] TRB attendees dataloaders (#2705) * 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 * [EASI-4457] Add closed requests for testing (#2709) * refactor seed data to allow for closed request generation * remove context value and just use a constant * add regex to LCID test * [EASI-4457] TRB documents dataloaders (#2706) * add a dataloader for trb documents, use dataloaders in other resolvers * semicolons and better error handling * [EASI-4457] TRB Admin Notes dataloader (#2714) add dataloaders for TRB admin notes * [EASI-4457] TRB Request Funding Sources dataloader (#2715) * add dataloader for trb funding sources * fix test to allow for empty slice * add TODO comment * [EASI-4457] TRB Form System Intakes dataloader (#2722) add dataloader for trb request form system intakes * [EASI-4457] Add gov req feedback dataloader (#2731) * add gov req feedback dataloader * run gql gen * [EASI-4457] intake documents dataloader (#2732) * 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 * add comments * [EASI-4457] Business Case dataloaders (#2729) * 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> --------- Co-authored-by: samoddball <156127704+samoddball@users.noreply.github.com>
EASI-4457
Description
How to test this change
form/fundingSourceson TRB Requests.PR Author Checklist
PR Reviewer Guidelines