NV-2332 - Template Store - Add blueprint module #3432
Conversation
djabarovgeorge
changed the title
apps/api/src/app/blueprint/e2e/create-blueprint-notification-templates.e2e.ts
Show resolved
Hide resolved
| @Get('/:templateId/blueprint') | ||
| getNotificationTemplateBlueprintById( | ||
| @UserSession() user: IJwtPayload, | ||
| @Param('templateId') templateId: string | ||
| ): Promise<NotificationTemplateResponse> { | ||
| return this.getBlueprintNotificationTemplate.execute( | ||
| GetBlueprintNotificationTemplateCommand.create({ | ||
| environmentId: user.environmentId, | ||
| organizationId: user.organizationId, | ||
| userId: user._id, | ||
| templateId, | ||
| }) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
What we are trying to achieve moving them to an own module? As far as I remember when @davidsoderberg implemented this, blueprints will be base notification templates, so it felt right to live in the notifications template module and its API.
There was a problem hiding this comment.
I tend to agree, the blueprints are just the copy of the original notification template from another org. So I feel like there is no sense to move it to a separate module and make the breaking changes in the public API (not sure if anyone is using it).
There was a problem hiding this comment.
So the change was made for several reasons, you guys are right to say that the blueprint is in a sense are notification- template, but logically they are not. I was not sure what to name this new module as well
I wanted to separate those two because maybe tomorrow we will get this data from separate CMS and then it won't make sense to keep it in the notification-template namespace.
The most crucial part of why I created a separate module was the blueprint storage
but in the end, it was not relevant but I still decided to keep it separate.
Let me know what you think at the current state I am not tied in this decision and I can revert it if you think it is less intuitive.
@p-fernandez @LetItRock
There was a problem hiding this comment.
@djabarovgeorge
I'm ok with leaving it, but all the reasons are not valid anymore as you said.
So my approach is to not overengineer if not needed and if you are not sure because the future changes might never happen. But anyway, this is just my personal opinion, and I feel it's ok to leave it if you already did a big chunk of work.
There was a problem hiding this comment.
The revert should not be too much work, I will merge for now and revisit this decision in middle of this week
What change does this PR introduce?
Add a dedicated blueprint module to fetch blueprints.
Why was this change needed?
Separate the blueprint logic from the notification-templates.
Other information (Screenshots)