Skip to content

PIN-8688 move 'how to get a voucher' section in developer tools#1584

Open
antofo wants to merge 13 commits intodevfrom
feature/pin-8688-move-how-to-get-voucher
Open

PIN-8688 move 'how to get a voucher' section in developer tools#1584
antofo wants to merge 13 commits intodevfrom
feature/pin-8688-move-how-to-get-voucher

Conversation

@antofo
Copy link
Collaborator

@antofo antofo commented Dec 10, 2025

  • Create new page for voucher simulation
  • Move voucher instructions related components
  • Implement client selection in step 1 of voucher instructions

action: () =>
navigate(
clientKind === 'API' ? 'SIMULATE_GET_VOUCHER_API' : 'SIMULATE_GET_VOUCHER_CONSUMER'
),
Copy link
Contributor

Choose a reason for hiding this comment

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

can i suggest to move singleAction like this ?

 const voucherSimulationAction: ActionItemButton = {
    action: () =>
      navigate(clientKind === 'API' ? 'SIMULATE_GET_VOUCHER_API' : 'SIMULATE_GET_VOUCHER_CONSUMER'),
    label: actionT('simulateVoucher'),
    variant: 'contained',
  }

])

return (
<FormProvider {...methods}>
Copy link
Contributor

Choose a reason for hiding this comment

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

formMethods

Suggested change
<FormProvider {...methods}>
<FormProvider {...formMethods}>

loading={isFetchingClients}
/>
</FormControl>
{clientKind === 'CONSUMER' ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

[general]: Do you think it makes sense to abstract this component? I see a lot of "if" related to clientKind. Maybe an idea could be have two component within this.

@@ -44,9 +48,9 @@ export const VoucherInstructionsStep3: React.FC = () => {
<Stack spacing={2}>
<InformationContainer
label={t('step3.requestBody.clientIdField.label')}
content={clientId}
Copy link
Contributor

Choose a reason for hiding this comment

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

@Carminepo2
Copy link
Contributor

A couple of considerations:

  • Empty string is still used as a values that represent the absence of user choice, which is not correct, let's use null;
  • We have the clientId, etc..., states contained in two different parts, the context and the form, those states are synced with an useEffect. Using the ´useEffect´ to sync internal react states is not a good practice. My suggestion is to remove the state inside the context, and only use the form state. Then use the watch method to subscribe to form states to handle side effects which is writing the state in the url params and reset the other states. You can see an example of using watch to subscribe to form changes in /src/hooks/useRiskAnalysisForm.ts

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
41.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants