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

feat(dal): add relationship between message template and layout #2444

Merged
merged 1 commit into from
Jan 9, 2023

Conversation

p-fernandez
Copy link
Contributor

What change does this PR introduce?

Adds the relationship at DAL level for Message Template and Layout. Also adds missing test in message template creation to validate the change.

Why was this change needed?

Another step forward for the DAL for Layouts feature.

Other information (Screenshots)

@p-fernandez p-fernandez self-assigned this Jan 6, 2023
@linear
Copy link

linear bot commented Jan 6, 2023

NV-1422 Feature: add relationship between Message Template and Layout

DAL relationship to MessageTemplate and Layout.

@p-fernandez p-fernandez force-pushed the nv-1422-feature-add-relationship-between-message branch from 281d85f to 6ed8aea Compare January 6, 2023 17:43
@p-fernandez p-fernandez force-pushed the nv-1422-feature-add-relationship-between-message branch from 6ed8aea to 00f9ce4 Compare January 6, 2023 17:45
@@ -11,6 +11,9 @@ export class MessageTemplateEntity {

_creatorId: string;

// TODO: Due a circular dependency I can't import LayoutId from Layout.
_layoutId: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line export { IEmailBlock, ITemplateVariable } from '../message-template'; shouldn't be exported from the libs/dal/src/repositories/layout/types.ts. Just import it directly, because this way it creates a circular dependency.
ex. file libs/dal/src/repositories/layout/layout.entity.ts should import IEmailBlock from ../message-template or another suggestion to move these types to some shared folder...

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 want to review the types and do some tidy up. Will do it in a different PR and I will address this problem. Mainly I was facing that message-template and layout are dependant on each other (because sharing properties and the relationship through LayoutId). Tidying up types should help for that. 🙂

@p-fernandez p-fernandez merged commit 82e98ab into next Jan 9, 2023
@p-fernandez p-fernandez deleted the nv-1422-feature-add-relationship-between-message branch January 9, 2023 12:52
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

3 participants