-
Notifications
You must be signed in to change notification settings - Fork 92
fix: pass event document to conditionals #11057
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
fix: pass event document to conditionals #11057
Conversation
This comment has been minimized.
This comment has been minimized.
|
@tahmidrahman-dsi can you tell me, what git issue is this related to and what release please? |
@euanmillar I dont think there is a ticket yet. As part of my farajaland configuration work, I had to configure |
makelicious
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you write a test case for the form and to conditionals that uses this. Otherwise we don't know if it breaks again.
|
@tahmidrahman-dsi @makelicious it turns out that Madagascar also require this. I guess it could be triaged into 1.9.2 scope to ensure it makes it into the "to be created" release-v1.9.2 branch? But we need an associated issue. FYI @SyedaAfrida @ThierryMyshepherd @pankaj-pant @Zangetsu101 @rikukissa Can you create one? I'm just passing on information here and dont know the specifics |
| setFormData={(data) => setFormValues(data)} | ||
| showReviewButton={searchParams.from === 'review'} | ||
| validatorContext={validatorContext} | ||
| validatorContext={{ ...validatorContext, event }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be needed if you already are passing event in useValidatorContext, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, good catch, thanks @pankaj-pant ! forgot to update this, I will do it
makelicious
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you write a test case for the form and to conditionals that uses this. Otherwise we don't know if it breaks again.
| ]} | ||
| id="my-form" | ||
| validatorContext={getTestValidatorContext()} | ||
| onChange={() => noop()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might just pass args since you define them above args: { onChange: fn() },
| msw: { | ||
| handlers: { | ||
| event: [ | ||
| tRPCMsw.event.config.get.query(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might not need this? By default, mocks come from packages/client/.storybook/default-request-handlers.ts
| const StyledFormFieldGenerator = styled(FormFieldGenerator)` | ||
| width: 400px; | ||
| ` | ||
| export const Default: StoryObj<typeof FormFieldGenerator> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not have any interaction. Consider separate file if needed.
| import { getTestValidatorContext } from '../../../../../.storybook/decorators' | ||
|
|
||
| const meta: Meta<typeof FormFieldGenerator> = { | ||
| title: 'Inputs/AlphaPrintButton', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider separating interaction tests from others. e.g.
const meta: Meta<typeof Pages> = {
title: 'Declare/Interaction',
| parameters: { | ||
| reactRouter: { | ||
| router: { | ||
| path: ROUTES.V2.EVENTS.DECLARE.REVIEW.buildPath({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might not need this? could you define the parameters under meta if they are all the same
| role: TestUserRole | ||
| fullEvent: EventDocument | ||
| }> = { | ||
| parameters: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by giving userRole: estUserRole.Enum.LOCAL_REGISTRAR mocks the endpoints and makes it explicit what you are testing against. It defaults to Registration Agent, that's why it works.
| const StyledFormFieldGenerator = styled(FormFieldGenerator)` | ||
| width: 400px; | ||
| ` | ||
| function createAlphaPrintButtonStoryParameters( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@makelicious @Zangetsu101 I wanted to render FormFieldGenerator with different conditional and validatorContext. But could not work it out.
This is as far as I could go to make the tests functional, now tests run fine. But wonder if there is a more storybook way of doing it
|
Your environment is deployed to https://fixpass-event-document-to-cond.e2e-k8s.opencrvs.dev |
Zangetsu101
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Let's just show this in action by using it for some field (or a new field) in tennis club membership event
To use conditionals like:
event.hasAction(ActionType.DECLARE)in forms and review pages