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: add ability to set channels during trigger #5541

Merged
merged 3 commits into from May 12, 2024

Conversation

scopsy
Copy link
Contributor

@scopsy scopsy commented May 10, 2024

What changed? Why was the change needed?

  • Requested by multiple customers
  • Currently users are required to have cronjobs that perform 2 API requests: Create or Update Subscriber, and set channel credentials

We already support creating a subscriber during trigger, this change allows creating it with the channels object for push and chat channels similarly to how ew allow doing that with email and phone today.

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

if (
subscriber &&
!subscriberNeedUpdate(subscriber, subscriberPayload) &&
!subscriberPayload.channels
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 don't entirely like this, but this is for now until we have a better diff logic happening here

Copy link

netlify bot commented May 10, 2024

Deploy Preview for novu-design ready!

Name Link
🔨 Latest commit 653dfda
🔍 Latest deploy log https://app.netlify.com/sites/novu-design/deploys/663debcb815550000842942c
😎 Deploy Preview https://deploy-preview-5541--novu-design.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 10, 2024

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit 653dfda
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/663debcb47caee00085697f8
😎 Deploy Preview https://deploy-preview-5541--dev-web-novu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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.

LGTM :)

</envs>
<ui />
<extra-mocha-options>--timeout 30000 --require ts-node/register --exit --file e2e/setup.ts</extra-mocha-options>
<extra-mocha-options>--require ts-node/register --exit --file e2e/setup.ts</extra-mocha-options>
Copy link
Contributor

Choose a reason for hiding this comment

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

i wonder what is the default timeout, it does not exist it would be better to have it defined here

@scopsy scopsy added the run-e2e label May 10, 2024
@scopsy scopsy merged commit e20024d into next May 12, 2024
40 checks passed
@scopsy scopsy deleted the add-channels-during-trigger branch May 12, 2024 08:02
SokratisVidros pushed a commit that referenced this pull request May 13, 2024
* feat: add ability to set channels during trigger

* fix: tests

* fix: remove duplicate check for subscriber needs update
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

2 participants