feat(dal): update integration entity schema for multi provider #3625
Conversation
NV-2411 Update integration entity schema
What?Update the integration entity schema based on the decisions that were taken in the tech doc: https://www.notion.so/novuhq/Technical-working-process-2ece7a460b3447bd8b6c1a11770a73f5?pvs=4#b93679d09da84266b01d4e73a269f344 The condition rules are out of scope. Why? (Context)Definition of Done
|
|
|
||
| identifier?: string; | ||
|
|
||
| kind?: ProviderKindEnum; |
There was a problem hiding this comment.
Typing as ProviderKindEnum in case in the future we have kinds in other entities to help the readability and searcheability.
There was a problem hiding this comment.
I am not sure about 'kind'. What do you think about 'category' ? Both don't feel completely right🤷🏼♀️
There was a problem hiding this comment.
A small question regarding the kind property, if we are doing the "conditions" array, doesn't it mean that the kinds can be combined in a more dynamic way inside the the conditions array. Meaning that you can have the following dynamic conditions:
- TenantId = 123
- AND
- Environment = production
- AND
- Workflow = 123
So in this case it's either the "kind" or "conditions" approach, not the two of them combined. What are your thoughts on this, maybe I'm missing something
There was a problem hiding this comment.
@ainouzgali kind and category are synonims in a way. In the technical design I chose kind as it is used to define entities in places like GCP and I come from working a lot with GCP.
There was a problem hiding this comment.
A small question regarding the
kindproperty, if we are doing the "conditions" array, doesn't it mean that the kinds can be combined in a more dynamic way inside the theconditionsarray. Meaning that you can have the following dynamic conditions:
- TenantId = 123
- AND
- Environment = production
- AND
- Workflow = 123
So in this case it's either the "kind" or "conditions" approach, not the two of them combined. What are your thoughts on this, maybe I'm missing something
Ok, I wasn't aware that we totally ditched the kind property. Makes no sense if we are going to go conditions approach.
| export enum ProviderKindEnum { | ||
| ENVIRONMENT = 'environment', | ||
| TENANT = 'tenant', | ||
| WORKFLOW = 'workflow', | ||
| } |
There was a problem hiding this comment.
So far we will only use the environment kind but ready for the future.
There was a problem hiding this comment.
I'm probably jumping ahead - but maybe already add this type to shared ? As we would use it everywhere for sure
There was a problem hiding this comment.
Good point. But going to delete it as we do not need it until we create the conditions in the future.
| name: Schema.Types.String, | ||
| identifier: Schema.Types.String, | ||
| kind: Schema.Types.String, | ||
| kindId: Schema.Types.ObjectId, |
There was a problem hiding this comment.
Should we already add conditions? But as an empty array for now?
To make it easier for next phase
There was a problem hiding this comment.
Better wait for now. If we then change our mind we might forget to clean it.
| export enum ProviderKindEnum { | ||
| ENVIRONMENT = 'environment', | ||
| TENANT = 'tenant', | ||
| WORKFLOW = 'workflow', | ||
| } |
There was a problem hiding this comment.
I'm probably jumping ahead - but maybe already add this type to shared ? As we would use it everywhere for sure
|
|
||
| identifier?: string; | ||
|
|
||
| kind?: ProviderKindEnum; |
There was a problem hiding this comment.
I am not sure about 'kind'. What do you think about 'category' ? Both don't feel completely right🤷🏼♀️
|
Not sure i followed how is the new |
|
Oh @p-fernandez , a question. We now don't have a field to save the environment the user chooses for the instance. |
But that looks like a presentational thing. At the end we are storing the environmentId in the provider. I think. |
OK. I think we can go that way until we decide on the conditions structure |
What change does this PR introduce?
Update integration entity schema...
Why was this change needed?
...to support multi provider configuration.
Other information (Screenshots)