#1487: replaced usage of type any with specific types #2130
Conversation
There was a problem hiding this comment.
This is looking good! Could you create some named interfaces for the payloads (look to the tasks hooks file for examples)? Also, I think your TS check is failing because you need to pull from develop (git pull origin develop).
The TS checks run when you do yarn prisma:reset so you can check it that way
…cing/FinishLine into #1487-remove-any-change-request.hooks pull develop
| @@ -103,41 +111,63 @@ export const useCreateStandardChangeRequest = () => { | |||
| ); | |||
| }; | |||
|
|
|||
| export interface Payload { | |||
There was a problem hiding this comment.
Can you please give this a more descriptive name regarding what it's for?
There was a problem hiding this comment.
The two payloads for activation and stage gate change requests should actually be different. You don't see issues here, because we have it set up such that the hooks only use the fields they need, but if you look to where the hooks are implemented, you can see the objects that are passed as arguments do not follow the one payload. You should create two payloads based on the fields passed where the hooks are used.
| ['change requests', 'review'], | ||
| async (crReviewPayload: { userIds: number[] }) => { | ||
| async (crReviewPayload) => { |
There was a problem hiding this comment.
Define the type of crReviewPayload in the async call with the new type you created
| @@ -47,7 +47,8 @@ const ProposedSolutionsList: React.FC<ProposedSolutionsListProps> = ({ proposedS | |||
| description, | |||
| scopeImpact, | |||
| timelineImpact, | |||
| budgetImpact | |||
| budgetImpact, | |||
| type: '' | |||
There was a problem hiding this comment.
Since ProposedSolution type is just a placeholder value you can just remove type from the CreateProposeSolutionPayload type and call mutateAsync without that field
| projectLeadId, | ||
| projectManagerId, | ||
| startDate, | ||
| projectLeadId: projectLeadId || 0, |
There was a problem hiding this comment.
Instead of doing projectLeadId || 0 in the case that projectLeadId is undefined, use an if statement like above to check if projectLeadId is defined and throw an error if not
| projectManagerId, | ||
| startDate, | ||
| projectLeadId: projectLeadId || 0, | ||
| projectManagerId: projectManagerId || 0, |
| @@ -41,14 +41,20 @@ const ActivateWorkPackageModalContainer: React.FC<ActivateWorkPackageModalContai | |||
| const handleConfirm = async ({ projectLeadId, projectManagerId, startDate, confirmDetails }: FormInput) => { | |||
| handleClose(); | |||
| if (auth.user?.userId === undefined) throw new Error('Cannot create activation change request without being logged in'); | |||
| if (projectLeadId === undefined) { | |||
There was a problem hiding this comment.
You can just use the shorthand of !projectLeadId instead of projectLeadId === undefined
| if (projectLeadId === undefined) { | ||
| throw new Error('Project Lead Id must be defined to create an activation change request'); | ||
| } | ||
| if (projectManagerId === undefined) { |
Changes
i went through the file and replaced occurrences of any with the appropriate types. I would declare certain variables as number, boolean, string, etc and could delete the any keywords.
Notes
No notes, but would like to make sure I did it correctly or in the format you are looking for
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes #1487