fix(dashboard): Always trust the URL for the environment selection #7223

SokratisVidros · 2024-12-05T14:22:34Z

What changed? Why was the change needed?

Pick an environment based on the URL and avoid race conditions using the local storage as we did in the old web app
Allow multiple environments to open in separate browser tabs
Unify conventions around API-related hooks. That is:

Name all of them similarly using the use-verb-resource pattern.
Unify their return types and add stricter types to hook arguments

What this PR doesn't contain?

-[] Unify the return types to always follow the { resourceName: data, ...restOfReactQuery } return type. This will be done separately in the future.

netlify · 2024-12-05T14:22:55Z

✅ Deploy Preview for dev-web-novu ready!

Name	Link
🔨 Latest commit	`ac08790`
🔍 Latest deploy log	https://app.netlify.com/sites/dev-web-novu/deploys/675314cc4ab4f90008ef6ea5
😎 Deploy Preview	https://deploy-preview-7223.dashboard.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

netlify · 2024-12-05T14:22:56Z

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

Name	Link
🔨 Latest commit	`ac08790`
🔍 Latest deploy log	https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/675314ccc6d1670008eed336
😎 Deploy Preview	https://deploy-preview-7223.dashboard-v2.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

pkg-pr-new · 2024-12-05T21:50:33Z

Open in Stackblitz

@novu/client

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

@novu/framework

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

@novu/node

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

@novu/notification-center

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

@novu/headless

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

novu

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

@novu/providers

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

@novu/shared

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

commit: ac08790

SokratisVidros · 2024-12-05T21:52:30Z

apps/dashboard/src/api/api.client.ts

@@ -17,28 +17,28 @@ type HttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH';
 const request = async <T>(
  endpoint: string,
  options?: {
-    data?: unknown;
+    environment?: IEnvironment;


Inject the environment to API calls

SokratisVidros · 2024-12-05T21:52:42Z

apps/dashboard/src/api/billing.ts

-export async function getBillingSubscription() {
-  const { data } = await get<{ data: GetSubscriptionDto }>('/billing/subscription');
-
+export async function getSubscription({ environment }: { environment: IEnvironment }) {


Simplify naming

SokratisVidros · 2024-12-05T21:53:22Z

apps/dashboard/src/components/auth/inbox-playground.tsx

@@ -1,25 +1,26 @@
-import { useEffect, useState } from 'react';


Sort alphabetically to have it nice and tidy

SokratisVidros · 2024-12-05T21:54:13Z

apps/dashboard/src/components/auth/questionnaire-form.tsx

@@ -9,7 +9,7 @@ import { useForm, Controller } from 'react-hook-form';
 import { updateClerkOrgMetadata } from '../../api/organization';
 import { hubspotCookie } from '../../utils/cookies';
 import { identifyUser } from '../../api/telemetry';
-import { useTelemetry } from '../../hooks';
+import { useTelemetry } from '../../hooks/use-telemetry';


We should avoid the index.ts barrel file. It destroys tree shaking, chunking and introduces an extra import and file resolution.

SokratisVidros · 2024-12-05T21:55:36Z

apps/dashboard/src/hooks/index.ts

@@ -1,9 +0,0 @@
-export * from './use-billing-subscription';


https://tkdodo.eu/blog/please-stop-using-barrel-files

SokratisVidros · 2024-12-05T22:15:15Z

apps/dashboard/src/components/create-workflow-button.tsx

  const { mutateAsync, isPending } = useMutation({
-    mutationFn: async (data: CreateWorkflowDto) => createWorkflow(data),
+    mutationFn: async (workflow: CreateWorkflowDto) => createWorkflow({ environment: currentEnvironment!, workflow }),


In a follow-up PR, currentEnvironment will never be undefined or null to avoid the ! or ?._id discomfort.

SokratisVidros · 2024-12-05T22:29:10Z

apps/dashboard/src/hooks/use-fetch-workflows.ts

    placeholderData: keepPreviousData,
+    enabled: !!currentEnvironment?._id,
+    refetchOnWindowFocus: true,


This caters to the use case where customers will have their dev and prod environments side by side, and they will sync workflows between environments. They should be able to see the update without a full page reload.

- Pick an environment based on the URL and avoid race conditions using the local storage as we did in the old web app - Allow multiple environments open in separate browser tabs

This caters to the use case where customers will have their dev and prod environments side by side, and they will sync workflows between environments. They should be able to see the update without a full page reload.

LetItRock

lgtm 🙌

SokratisVidros requested review from LetItRock, scopsy and djabarovgeorge December 5, 2024 14:22

SokratisVidros self-assigned this Dec 5, 2024

SokratisVidros temporarily deployed to Linting December 5, 2024 14:22 — with GitHub Actions Inactive

SokratisVidros temporarily deployed to Linting December 5, 2024 21:45 — with GitHub Actions Inactive

SokratisVidros had a problem deploying to Linting December 5, 2024 21:45 — with GitHub Actions Error

SokratisVidros temporarily deployed to Linting December 5, 2024 21:46 — with GitHub Actions Inactive

github-actions bot added the @novu/shared label Dec 5, 2024

SokratisVidros force-pushed the env_switching branch from 0acbdcd to 119fb32 Compare December 5, 2024 21:46

SokratisVidros temporarily deployed to Linting December 5, 2024 21:46 — with GitHub Actions Inactive

SokratisVidros force-pushed the env_switching branch from 119fb32 to ae30083 Compare December 5, 2024 21:48

SokratisVidros temporarily deployed to Linting December 5, 2024 21:49 — with GitHub Actions Inactive

SokratisVidros commented Dec 5, 2024

View reviewed changes

scopsy approved these changes Dec 5, 2024

View reviewed changes

SokratisVidros commented Dec 5, 2024

View reviewed changes

SokratisVidros temporarily deployed to Linting December 5, 2024 22:27 — with GitHub Actions Inactive

SokratisVidros commented Dec 5, 2024

View reviewed changes

fix(dashboard): Always trust the URL for the environment selection

e7cee78

SokratisVidros added 2 commits December 6, 2024 16:22

chore(dashboard): Inject current environment across all hooks

36709f8

- Pick an environment based on the URL and avoid race conditions using the local storage as we did in the old web app - Allow multiple environments open in separate browser tabs

fix(dashboard): Refetch workflow list on window focus

a9be679

This caters to the use case where customers will have their dev and prod environments side by side, and they will sync workflows between environments. They should be able to see the update without a full page reload.

SokratisVidros force-pushed the env_switching branch from c5e7fc6 to a9be679 Compare December 6, 2024 14:22

SokratisVidros temporarily deployed to Linting December 6, 2024 14:22 — with GitHub Actions Inactive

SokratisVidros added 2 commits December 6, 2024 16:34

fixup! chore(dashboard): Inject current environment across all hooks

9f3c5bf

fixup! fix(dashboard): Refetch workflow list on window focus

ac08790

SokratisVidros temporarily deployed to Linting December 6, 2024 15:14 — with GitHub Actions Inactive

LetItRock approved these changes Dec 6, 2024

View reviewed changes

SokratisVidros merged commit ba519c2 into next Dec 6, 2024
43 of 44 checks passed

SokratisVidros deleted the env_switching branch December 6, 2024 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dashboard): Always trust the URL for the environment selection #7223

fix(dashboard): Always trust the URL for the environment selection #7223

SokratisVidros commented Dec 5, 2024 •

edited

Loading

netlify bot commented Dec 5, 2024 •

edited

Loading

netlify bot commented Dec 5, 2024 •

edited

Loading

pkg-pr-new bot commented Dec 5, 2024 •

edited

Loading

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

SokratisVidros Dec 5, 2024

LetItRock left a comment

		@@ -1,25 +1,26 @@
		import { useEffect, useState } from 'react';

fix(dashboard): Always trust the URL for the environment selection #7223

fix(dashboard): Always trust the URL for the environment selection #7223

Conversation

SokratisVidros commented Dec 5, 2024 • edited Loading

What changed? Why was the change needed?

What this PR doesn't contain?

netlify bot commented Dec 5, 2024 • edited Loading

✅ Deploy Preview for dev-web-novu ready!

netlify bot commented Dec 5, 2024 • edited Loading

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

pkg-pr-new bot commented Dec 5, 2024 • edited Loading

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 5, 2024

Choose a reason for hiding this comment

LetItRock left a comment

Choose a reason for hiding this comment

SokratisVidros commented Dec 5, 2024 •

edited

Loading

netlify bot commented Dec 5, 2024 •

edited

Loading

netlify bot commented Dec 5, 2024 •

edited

Loading

pkg-pr-new bot commented Dec 5, 2024 •

edited

Loading