feat: add addition params for onboarding setup #2886
Conversation
| function updateCodeSnipped(codeSnippet: string, environmentIdentifier: string, apiKey: string) { | ||
| return codeSnippet | ||
| .replace(APPLICATION_IDENTIFIER, environmentIdentifier) | ||
| .replace(API_KEY, apiKey ?? '') |
There was a problem hiding this comment.
Added API key in order to pass it to the demo app.
| return codeSnippet | ||
| .replace(APPLICATION_IDENTIFIER, environmentIdentifier) | ||
| .replace(API_KEY, apiKey ?? '') | ||
| .replace(ENVIRONMENT, process.env.REACT_APP_API_URL === 'https://api.novu.co' ? '' : 'dev'); |
There was a problem hiding this comment.
We need this one in order to provide a more fluent flow for the product team so they won't need to update the API and WS URLs on the demo app.
I was not sure what indication to add here at the moment I am using the API URL, if some one have any other suggestion i would love to hear about them :)
| type: ChannelCTATypeEnum.REDIRECT, | ||
| data: { url: `/templates/edit/${notificationTemplate?._id}` }, | ||
| }, | ||
| content: 'Welcome to Novu! <b>visit the cloud admin panel</b> managing this message', |
There was a problem hiding this comment.
I removed the CTA because the snippets do not have the click handlers, @scopsy let me know what you think.
| .replace(BACKEND_API_URL, concatUrls ? process.env.REACT_APP_API_URL || 'http://localhost:3000' : '') | ||
| .replace(BACKEND_SOCKET_URL, concatUrls ? process.env.REACT_APP_WS_URL || 'http://localhost:3002' : ''); |
There was a problem hiding this comment.
Shouldn't we use window._env_.REACT_APP_API_URL and window._env_.REACT_APP_WS_URL to avoid to have spread all the Web app the URL calculation?
novu/apps/web/src/config/index.ts
Line 12 in 6e5b03e
If for any reason they are not the right variables to use here, maybe centralising in the config the calculation would be a good idea to have all that isolated in one place and not through the codebase. What do you think?
There was a problem hiding this comment.
I feel we should take values from window._env_ as these are set by our pipelines...
There was a problem hiding this comment.
Good catch, I will add it to the config file. Thanks!
…nts' into onboarding-demo-trigger-adjustments
What change does this PR introduce?
Adds addition params to run on the onboarding script.
Why was this change needed?
In order to trigger event on the demo app.
Other information (Screenshots)