Skip to content
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

NV-2335 - Template Store - Template store refactor web on blueprint creation to use #3467

Conversation

djabarovgeorge
Copy link
Contributor

What change does this PR introduce?

At the moment we are using the /notification-template/:templateId/blueprint POST, we need to update this flow to create the template using the regular /notification-template POST request that is used by the client.

The additional step that we will need to refactor is instead of sending the id of the blueprint we should pass the template snapshot.

Why was this change needed?

  1. Reuse the existing route.
  2. In case we get calls from different environments (US/EU...) the current logic won't work because not all the environments we have the blueprint stored, in order to solve this issue we can send a snapshot of the template, the same way we do in the /notification-template POST request. In order to simplify the flow we can use the same route.

Other information (Screenshots)

@linear
Copy link

linear bot commented May 21, 2023

NV-2335 [Template store] Refactor WEB on blueprint creation to use notification-template POST

At the moment we are using the /notification-template/:templateId/blueprint POST, we need to update this flow to create the template using the regular /notification-template POST request that is used by the client.

The additional step that we will need to refactor is instead of sending the id of the blueprint we should pass the template snapshot.

}
});

export async function createTemplateFromBlueprint({
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this one should be moved to some util?

apps/web/src/pages/templates/components/BlueprintModal.tsx Outdated Show resolved Hide resolved
@@ -14,7 +14,7 @@ export class CreateNotificationTemplateRequestDto implements ICreateNotification
@ApiProperty()
@IsString()
@IsDefined({
message: 'Notification group must be provided',
message: 'Notification group must be provided ',
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need that space?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found that if we won't have it that the next argument will miss a space, so I added it here but if it is persistent in all cases maybe looking at how to fix it globally will be a better idea.

Base automatically changed from NV-2338/template-store-invalidate-the-cache-on-blueprints-update to next May 22, 2023 09:53
@djabarovgeorge djabarovgeorge added this pull request to the merge queue May 22, 2023
@djabarovgeorge djabarovgeorge removed this pull request from the merge queue due to a manual request May 22, 2023
@djabarovgeorge djabarovgeorge added this pull request to the merge queue May 22, 2023
Merged via the queue into next with commit 1355006 May 22, 2023
33 checks passed
@djabarovgeorge djabarovgeorge deleted the NV-2335/template-store-refactor-web-on-blueprint-creation-to-use branch May 22, 2023 11:05
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.

None yet

2 participants