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(api): set integration as primary and priority system #3873

Conversation

LetItRock
Copy link
Contributor

What change does this PR introduce?

Set integration as "primary" endpoint and priority system logic:

  1. One integration per channel could be marked as primary.
  2. Only Email and SMS integrations could be marked as primary and will apply priority to the integrations. For the Chat and Push integrations messages are delivered for each channel, potentially different integrations, meaning that we can't have a "primary" integration concept or priority system. For In-App we only have one provider rn, but in future, we might have many per application, so there also we can't apply these concepts.
  3. When we do create or update the first active integration (excluding Novu providers) for the Email or SMS channel, that integration will become automatically primary.
  4. I will describe all the other logic in comments on the code.

Why was this change needed?

Other information (Screenshots)

@LetItRock LetItRock self-assigned this Jul 28, 2023
@linear
Copy link

linear bot commented Jul 28, 2023

@LetItRock LetItRock changed the title [WIP] 🚧 feat(api): set integration as primary and priority system feat(api): set integration as primary and priority system Aug 2, 2023
@LetItRock LetItRock marked this pull request as ready for review August 2, 2023 10:53
@github-actions github-actions bot removed the @novu/web label Aug 2, 2023
LetItRock and others added 2 commits August 2, 2023 13:38
…api-update-integrations-logic-for-primary-and-private-fields
…api-update-integrations-logic-for-primary-and-private-fields
…api-update-integrations-logic-for-primary-and-private-fields
Copy link
Contributor

@djabarovgeorge djabarovgeorge left a comment

Choose a reason for hiding this comment

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

Everything looks great! :) I just had a couple of questions sorry about it, if my concerns are by design I can approve it just let me know

another small thing i think create and update usecase shares some logic maybe we could extract it as well but totally could we for future refactoring.

LetItRock and others added 4 commits August 7, 2023 10:39
…api-update-integrations-logic-for-primary-and-private-fields
…api-update-integrations-logic-for-primary-and-private-fields
…api-update-integrations-logic-for-primary-and-private-fields
Base automatically changed from nv-2606-select-primary-integration-modal to next August 8, 2023 10:55
Copy link
Contributor

@djabarovgeorge djabarovgeorge left a comment

Choose a reason for hiding this comment

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

Yeah looks great to me awesome work :)

@LetItRock LetItRock added this pull request to the merge queue Aug 8, 2023
Merged via the queue into next with commit 70eb4c7 Aug 8, 2023
@LetItRock LetItRock deleted the nv-2607-api-update-integrations-logic-for-primary-and-private-fields branch August 8, 2023 16:47
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.

2 participants