PSP-8317: Lease and Licence Edit/View Fee Denomination forms #4187
Conversation
|
✅ No secrets were detected in the code. |
|
✅ No secrets were detected in the code. |
| @@ -114,6 +114,11 @@ export const AddLeaseYupSchema = Yup.object().shape({ | |||
| primaryArbitrationCity: Yup.string() | |||
| .nullable() | |||
| .max(200, 'Primary arbitration city must be at most ${max} characters'), | |||
| isPublicBenefit: Yup.string().nullable(), | |||
There was a problem hiding this comment.
As far as I can tell, these are defined as booleans in the frontend model.
There was a problem hiding this comment.
Taken out the nullable validation on the yup schema since is not needed.
|
|
||
| export default FeeDeterminationSubForm; | ||
|
|
||
| const MediumTextArea = styled(TextArea)` |
There was a problem hiding this comment.
Why is this necessary? I'm surprised that you needed to manually specify the height here.
There was a problem hiding this comment.
Changed this since height is not needed
| }, | ||
| }, | ||
| ], | ||
| amount: 1, |
There was a problem hiding this comment.
when writing tests, please try and keep the test data as minimal as "focused" as possible on the parameters necessary to evaluate the component under test. For example, for this test, a large number of these fields will not have an effect on what is rendered, and can be removed.
There was a problem hiding this comment.
Reduced the amount of variable required for unit testing.
| expect(getByDisplayValue('Yes')).toBeVisible(); | ||
| }); | ||
|
|
||
| it('renders the suggested Fee field', () => { |
There was a problem hiding this comment.
since there is a calculation, I'd actually rather see more tests for this then the other fields. In general we don't need to write one test for field as that doesn't scale, but we want to write tests for non-trivial logic or business rules.
There was a problem hiding this comment.
There was a problem hiding this comment.
I deleted the fields tests and increase the number of unit tests regarding the calculation of the suggested fee.
| @@ -31,7 +31,9 @@ describe('Compensation Detail View Component', () => { | |||
| <CompensationRequisitionDetailView | |||
| acquisitionFile={renderOptions?.props?.acquisitionFile ?? mockAcquisitionFileResponse()} | |||
| compensation={renderOptions?.props?.compensation ?? getMockApiDefaultCompensation()} | |||
| compensationProperties={renderOptions?.props?.compensationProperties ?? getMockCompensationPropertiesReq()} | |||
| compensationProperties={ | |||
There was a problem hiding this comment.
I think this is just a lint thing. I no longer see this file on my PR update.
|
@stairaku please ask Julian what he means by this: (from confluence) |
|
✅ No secrets were detected in the code. |
|
✅ No secrets were detected in the code. |
No description provided.