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

fix(api): step naming #7140

Merged
merged 6 commits into from
Dec 4, 2024
Merged

fix(api): step naming #7140

merged 6 commits into from
Dec 4, 2024

Conversation

djabarovgeorge
Copy link
Contributor

What changed? Why was the change needed?

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

# Conflicts:
#	apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts
Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for novu-stg-vite-dashboard-poc ready!

Name Link
🔨 Latest commit 768f671
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/674c83ece4ffe700085221da
😎 Deploy Preview https://deploy-preview-7140.dashboard-v2.novu-staging.co
📱 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.

@djabarovgeorge djabarovgeorge marked this pull request as draft November 27, 2024 10:26
@SokratisVidros
Copy link
Contributor

@djabarovgeorge Defining the IdentifierOrInternalId type as a string doesn't add any value, so let's just drop it. Moreover, this PR is about alignment. So the naming should be the same between workflows and steps. That is workflowIdOrInternalId and stepIdOrInternalId.

# Conflicts:
#	apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts
Copy link

netlify bot commented Dec 1, 2024

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit 768f671
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/674c83ecc89c760008e2c5c7
😎 Deploy Preview https://deploy-preview-7140.dashboard.novu-staging.co
📱 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 Dec 1, 2024

Deploy Preview for dashboard-v2-novu-staging ready!

Name Link
🔨 Latest commit 768f671
🔍 Latest deploy log https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/674c83ece2d9ba00097b3003
😎 Deploy Preview https://deploy-preview-7140--dashboard-v2-novu-staging.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.

@djabarovgeorge
Copy link
Contributor Author

djabarovgeorge commented Dec 1, 2024

@djabarovgeorge Defining the IdentifierOrInternalId type as a string doesn't add any value, so let's just drop it. Moreover, this PR is about alignment. So the naming should be the same between workflows and steps. That is workflowIdOrInternalId and stepIdOrInternalId.

Thanks for the feedback! @SokratisVidros
I’ve updated all the endpoints to explicitly use workflowIdOrInternalId and stepIdOrInternalId. The same changes were applied to nested use cases as well. This should make it clearer in nested cases which ID we’re working with.

Copy link

pkg-pr-new bot commented Dec 1, 2024

Open in Stackblitz

@novu/client

npm i https://pkg.pr.new/novuhq/novu/@novu/client@7140

@novu/framework

npm i https://pkg.pr.new/novuhq/novu/@novu/framework@7140

@novu/js

npm i https://pkg.pr.new/novuhq/novu/@novu/js@7140

@novu/headless

npm i https://pkg.pr.new/novuhq/novu/@novu/headless@7140

@novu/nextjs

npm i https://pkg.pr.new/novuhq/novu/@novu/nextjs@7140

@novu/node

npm i https://pkg.pr.new/novuhq/novu/@novu/node@7140

@novu/notification-center

npm i https://pkg.pr.new/novuhq/novu/@novu/notification-center@7140

novu

npm i https://pkg.pr.new/novuhq/novu@7140

@novu/providers

npm i https://pkg.pr.new/novuhq/novu/@novu/providers@7140

@novu/react

npm i https://pkg.pr.new/novuhq/novu/@novu/react@7140

@novu/react-native

npm i https://pkg.pr.new/novuhq/novu/@novu/react-native@7140

@novu/shared

npm i https://pkg.pr.new/novuhq/novu/@novu/shared@7140

commit: 768f671

@djabarovgeorge djabarovgeorge merged commit 780c05c into next Dec 4, 2024
48 checks passed
@djabarovgeorge djabarovgeorge deleted the fix-step-naming branch December 4, 2024 17:55
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