-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add IP Pool Name In Integration #3647
Conversation
@@ -115,4 +116,24 @@ export class CredentialsDto { | |||
@IsString() | |||
@IsOptional() | |||
webhookUrl?: string; | |||
|
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.
missing attributes that i located due the implements ICredentials
(in a hacky way)
|
||
export class CredentialsDto { | ||
export class CredentialsDto implements ICredentials { |
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 addition implements:
- is not helping with the compilation time with missing attributes because all of the attributes in ICredentials are optional.
- it will alert in case we provide an invalid type.
@@ -328,32 +329,6 @@ export interface IIntegratedProvider { | |||
novu?: boolean; | |||
} | |||
|
|||
export interface ICredentials { |
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.
🧹
@@ -152,34 +153,6 @@ export interface IIntegratedProvider { | |||
novu?: boolean; | |||
} | |||
|
|||
export interface ICredentials { |
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.
🧹
|
||
import type { EnvironmentId } from '../environment'; | ||
import type { OrganizationId } from '../organization'; | ||
import { ChangePropsValueType } from '../../types/helpers'; | ||
|
||
export interface ICredentials { |
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.
🧹
@@ -1,30 +1,6 @@ | |||
export interface ICredentialsDto { |
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.
🧹
@@ -59,4 +30,6 @@ export class IntegrationEntity { | |||
deletedBy: string; | |||
} | |||
|
|||
export type ICredentialsEntity = ICredentials; |
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.
was not sure what to call this one :/
because it is basically a type of interface, i left the I for now.
any thoughts about it?
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.
Decided to keep the "Entity" separation so we will have it in code. because one can change while the other one may not.
@@ -1,4 +1,4 @@ | |||
import { ICredentials } from '@novu/dal'; | |||
import { ICredentials } from '@novu/shared'; |
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.
I thought that the shared one will do the work in the provider's packages.
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.
The naming and location of some of the interfaces can be done in the future. This PR is big enough already.
🌟
What change does this PR introduce?
Add ip pool name into the send grid integration.
Why was this change needed?
in order to provide the user the ability to add/update it.
Other information (Screenshots)
i refactored(moved to shared) the ICredentials by mistake we had too many duplications of it mostly mine fult.
i created an ICredentials in shared.
deleted some of the duplications.
currently created
type
of the interface because they were the same, it this type will need to change we change to update it to interface andimplement
the shared ICredentials.