feat(api-service): polish action steps #7389
Conversation
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| export const createStep = (type: StepTypeEnum): StepCreateDto => { | ||
| const controlValue: Record<string, unknown> = {}; | ||
|
|
||
| if (type === StepTypeEnum.DIGEST) { | ||
| controlValue.amount = DEFAULT_CONTROL_DIGEST_AMOUNT; | ||
| controlValue.unit = DEFAULT_CONTROL_DIGEST_UNIT; | ||
| controlValue.digestKey = DEFAULT_CONTROL_DIGEST_DIGEST_KEY; | ||
| controlValue.cron = DEFAULT_CONTROL_DIGEST_CRON; | ||
| } | ||
|
|
||
| if (type === StepTypeEnum.DELAY) { | ||
| controlValue.amount = DEFAULT_CONTROL_DELAY_AMOUNT; | ||
| controlValue.unit = DEFAULT_CONTROL_DELAY_UNIT; | ||
| controlValue.type = DEFAULT_CONTROL_DELAY_TYPE; | ||
| } | ||
|
|
||
| return { | ||
| name: STEP_TYPE_LABELS[type] + ' Step', | ||
| type, | ||
| controlValues: controlValue, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
The summary of the findings of the best approach, why the change is necessary, and what is happening in the current implementation.
The main reason for this change is that we cannot create a step with control values during step creation. This limitation causes unexpected behavior, like the bug we are experiencing.
Currently, we cannot move schemas with default values to the Shared package and reuse them in the Dashboard because these schemas depend on Zod and Zod-to-JSON-Schema.
The solution we originally wanted—to move default values from the UiSchema to the dataSchema—is not supported natively. The solution should be Zod schema with default values (used by the Dashboard) and make certain attributes, like body, required for generating issues. However, this approach fails because zod-to-json-schema removes the required attributes if there is a default value meaning we will need to manipulate the schema manually.
This solution is reducing the amount of code. In addition once we store the default values during step creation, we wouldn't need to handle parsing them (placeholders/defaults) in the Dashboard or maintain "placeholders" or default values in the API.
Next Steps
- Remove placeholders from the API.
- Eliminate placeholder parsing in the
Dashboard.
| skip: controlValues.skip || undefined, | ||
| }; | ||
|
|
||
| return filterNullishValues(mappedValues); | ||
| } | ||
|
|
||
| function parseAmount(amount?: unknown) { |
There was a problem hiding this comment.
safe parse just in case
| amount: controlValues.amount || 0, | ||
| unit: controlValues.unit || TimeUnitEnum.SECONDS, | ||
| // Cast to trigger Ajv validation errors - possible undefined | ||
| ...(parseAmount(controlValues) as { amount?: number }), // TODO FIX THIS - should be controlValues.amount |
There was a problem hiding this comment.
self note: execute the todo
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
| @Transform(({ value }) => (value ? Number(value) : value)) | ||
| @IsNumber() | ||
| amount?: number; |
There was a problem hiding this comment.
this should be a number as we use it as number in the usecase
| type, | ||
| _id: crypto.randomUUID(), | ||
| }); | ||
| export const createStep = (type: StepTypeEnum): StepCreateDto => { |
There was a problem hiding this comment.
we used the wrong type here, this is why we passed redundant data to the API such as slug prefix
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer