feat(web): select primary integration modal #3869

LetItRock · 2023-07-27T16:36:37Z

What change does this PR introduce?

Implemented the Select Primary Integration modal that will be shown when after the user creates the integration if there are already any integrations for the same env and channel.

Why was this change needed?

Multi-Channel Provider PRD

Other information (Screenshots)

linear · 2023-07-27T16:36:39Z

NV-2606 Select primary provider instance modal

LetItRock · 2023-07-27T16:38:25Z

apps/web/src/api/hooks/useMakePrimaryIntegration.ts

+import { successMessage } from '../../utils/notifications';
+import { QueryKeys } from '../query.keys';
+
+export const useMakePrimaryIntegration = (


this is the hook that is used when the user clicks the "Make primary" button in the modal...

LetItRock · 2023-07-27T16:38:55Z

apps/web/src/design-system/button/Button.tsx

@@ -21,6 +21,7 @@ interface IButtonProps extends ButtonProps {
  pulse?: boolean;
  sx?: Sx;
  iconPosition?: 'left' | 'right';
+  testId?: string;


fixed the warning in the console

LetItRock · 2023-07-27T16:40:58Z

apps/web/src/design-system/icons/general/Star.tsx

+  border: 1px solid ${colors.gradientMiddle};
+`;
+
+export const Star = (props: React.ComponentPropsWithoutRef<'svg'>) => {


The star icon, I don't know why but when I export the whole "Primary Provider Icon" from Figma - the icon doesn't have a correct viewport, that's why I just exported the star and made the wrapper circle by hands... I will ask Nikolay to take a look

LetItRock · 2023-07-27T16:41:49Z

apps/web/src/design-system/table/Table.styles.ts

+        paddingLeft: withSelection ? 10 : 30,
+        paddingRight: withSelection ? 10 : 30,


the styles for the table when it has an option to select a row... more about it below

LetItRock · 2023-07-27T16:43:19Z

apps/web/src/design-system/table/Table.tsx

+  withSelection?: boolean;
+  withRowClickSelection?: boolean;
+  initialSelectedRows?: Record<IdType<T>, boolean>;
+  onRowClick?: (row: Row<T>) => void;
+  onRowSelect?: (row: Row<T>) => void;


Added the functionality to the table that will allow us to have a radio button in the first column of the table. Only one row could be selected and a callback triggered on selection.

LetItRock · 2023-07-27T16:52:26Z

apps/web/src/pages/integrations/components/multi-provider/SelectPrimaryIntegrationModal.tsx

+  const initialSelectedIndex = useMemo(() => {
+    const primary = findPrimaryIntegration(integrationsByEnvAndChannel ?? []);
+
+    return integrationsByEnvAndChannel?.indexOf(primary);
+  }, [integrationsByEnvAndChannel]);


find the initial index for a primary integration which will be selected in the modal

LetItRock · 2023-07-27T16:53:13Z

apps/web/src/pages/integrations/components/multi-provider/SelectPrimaryIntegrationModal.tsx

+  const onRowSelectCallback = (row: Row<ITableIntegration>) => {
+    setSelectedState({
+      selectedIntegrationId: row.original.integrationId,
+      isActive: row.original.active,
+      selectedRowId: row.id,
+    });
+  };


when the integration is selected by clicking on radio button we do store the info in the state

LetItRock · 2023-07-27T16:57:58Z

apps/web/src/pages/integrations/components/multi-provider/SelectPrimaryIntegrationModal.tsx

+        <When truthy={isLoading}>
+          <Table loading loadingItems={5} data-test-id="integrations-list-table" columns={columns} withSelection />
+        </When>
+        <When truthy={!isLoading}>
+          <Table
+            data-test-id="integrations-list-table"
+            columns={columns}
+            data={integrationsByEnvAndChannel}
+            withSelection
+            onRowSelect={onRowSelectCallback}
+            initialSelectedRows={initialSelectedIndex > -1 ? { [initialSelectedIndex]: true } : undefined}
+          />
+        </When>


the trick to have a initially selected row... in the beginning, we don't have data for integrations, so we cant calculate the selected row index... if we would set it to -1 in the begging then after the data arrives that index value would be different ex. 1, but the state won't be updated as it's just used for the init...

that's why we do show one table on loading and another when the data is ready...

LetItRock · 2023-07-27T16:58:53Z

apps/web/src/pages/integrations/components/multi-provider/UpdateProviderSidebar.tsx

+  useEffectOnce(() => {
+    openSelectPrimaryIntegrationModal({
+      environmentId: selectedProvider?.environmentId,
+      channelType: selectedProvider?.channel,
+    });
+  }, hasToSelectPrimaryProvider && !!selectedProvider);


show the modal when we have a hasToSelectPrimaryProvider flag set and the integration is loaded

LetItRock · 2023-07-27T16:59:31Z

apps/web/src/pages/integrations/utils.ts

@@ -32,6 +18,8 @@ export const mapToTableIntegration = (

  return {
    name: integration.name ?? provider?.displayName,
+    isPrimary: integration.isPrimary,


this flag should be returned from the BE to mark the integration as primary

LetItRock · 2023-08-02T11:22:15Z

apps/web/src/design-system/icons/general/StarEmpty.tsx

+export const StarEmpty = (props: React.ComponentPropsWithoutRef<'svg'>) => {
+  return (
+    <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 14 13" width={18} height={18} {...props}>
+      <path
+        fill="#fff"
+        d="M4.9 9.883 7 8.617 9.1 9.9l-.55-2.4 1.85-1.6-2.433-.217-.966-2.266-.967 2.25-2.433.216L5.45 7.5 4.9 9.883Zm-2.016 2.784 1.083-4.684-3.633-3.15 4.8-.416L7.001 0l1.866 4.417 4.8.416-3.633 3.15 1.083 4.684-4.116-2.484-4.117 2.484Z"
+      />
+    </svg>
+  );
+};


another different star shown in the mark as primary action

LetItRock · 2023-08-02T11:23:26Z

apps/web/src/pages/integrations/components/IntegrationNameCell.tsx

+      <Popover
+        opened={isPopoverOpened && original.primary}
+        withArrow
+        withinPortal
+        offset={10}
+        transitionDuration={300}
+        position="top"
+        width={230}
+        styles={{ dropdown: { minHeight: 'initial !important' } }}
+        content={
+          <Description>
+            {`The primary provider instance within the ${original.channel.toLowerCase()} ` +
+              `channel in the ${original.environment.toLowerCase()} environment.`}
+          </Description>
+        }
+        target={
+          <IconHolder onMouseEnter={() => setPopoverOpened(true)} onMouseLeave={() => setPopoverOpened(false)}>
+            <Image src={original.logoFileName[`${colorScheme}`]} alt={original.name} />
+            {original.primary && <Star />}
+          </IconHolder>
+        }
+      />


the popover shown over the integration icon when it's primary

LetItRock · 2023-08-02T11:24:29Z

apps/web/src/pages/integrations/components/UpdateIntegrationSidebarHeader.tsx

+  const onDeleteClick = () => {
+    if (!provider) {
+      return;
+    }
+
+    if (provider.primary) {
+      openModal({
+        environmentId: provider.environmentId,
+        channelType: provider.channel,
+        onClose: () => {
+          deleteIntegration({
+            id: provider.integrationId,
+            name: provider.name || provider.displayName,
+          });
+        },
+      });
+
+      return;
+    }
+
+    deleteIntegration({
+      id: provider.integrationId,
+      name: provider.name || provider.displayName,
+    });
+  };


when the delete integration dropdown action is clicked we do check if the integration was primary, and if yes then the user has to pick another primary integration

LetItRock · 2023-08-02T11:25:26Z

apps/web/src/pages/integrations/components/UpdateIntegrationSidebarHeader.tsx

+              {canMarkAsPrimary && (
+                <Dropdown.Item
+                  onClick={() => {
+                    makePrimaryIntegration({ id: provider.integrationId });
+                  }}
+                  icon={<StarEmpty />}
+                  disabled={isLoading || isMarkingPrimary}
+                >
+                  Mark as primary
+                </Dropdown.Item>
+              )}


only email/sms integrations could be marked as primary and also the integration that is not already primary ;)

LetItRock · 2023-08-02T11:27:04Z

apps/web/src/pages/integrations/components/multi-provider/SelectPrimaryIntegrationModal.tsx

+  const initialSelectedIndex = useMemo(() => {
+    const primary = integrationsByEnvAndChannel.find((el) => el.primary);
+    if (primary) {
+      return integrationsByEnvAndChannel.indexOf(primary ?? {});
+    }
+
+    return -1;
+  }, [integrationsByEnvAndChannel]);


calculate the index of default selected integration, it will be the one that is a primary

LetItRock · 2023-08-02T11:29:40Z

apps/web/src/pages/integrations/components/multi-provider/UpdateProviderSidebar.tsx

+  const updateAndSelectPrimaryIntegration = async (data: IConstructIntegrationDto) => {
+    if (!selectedProvider) {
+      return;
+    }
+
+    const { channel: selectedChannel, environmentId, primary } = selectedProvider;
+    const isActiveFieldChanged = dirtyFields.active;
+    const hasSameChannelActiveIntegration = !!providers
+      .filter((el) => !NOVU_PROVIDERS.includes(el.providerId))
+      .find((el) => el.active && el.channel === selectedChannel && el.environmentId === environmentId);
+    const isChannelSupportPrimary = CHANNELS_WITH_PRIMARY.includes(selectedChannel);
+
+    if (
+      isActiveFieldChanged &&
+      isChannelSupportPrimary &&
+      ((isActive && hasSameChannelActiveIntegration) || (!isActive && primary))
+    ) {
+      openSelectPrimaryIntegrationModal({
+        environmentId: selectedProvider?.environmentId,
+        channelType: selectedProvider?.channel,
+        onClose: () => {
+          updateIntegration(data);
+        },
+      });
+
+      return;
+    }
+
+    updateIntegration(data);
+  };


when the update integration button is clicked we do check if:

integration active field has changed and the integration channel supports primary (email/sms)

and is active and there are other active for the same channel

or is not active and is a primary

then we do show the modal to pick a new primary

djabarovgeorge

small question in this cycle we did not want to touch the workflow providers tab? asking regarding the star in the provider list there.

Do we need to present the deleted provider in the modal?
https://www.loom.com/share/f68a5a64702e4232973d85b02056bcd5

i am not sure if this state is possible but, another thing i encountered in the empty state where i could not add new provider, i was either redirected to workflow page or i was missing the create sidebar.
could be something in my machine tho, the flickering, in the end, is page refresh
https://www.loom.com/share/36a0bf4fea8640f782fec1770fde08f5

Another possible not possible state is the following
https://www.loom.com/share/68da5da165ce4bcc9721927ce5d30904
if it is possible should show the modal at all?

the thing i did not had the time to check is analytics and backward compatibility.

other than that your code looks as clean as always ✨

LetItRock · 2023-08-04T15:13:20Z

small question in this cycle we did not want to touch the workflow providers tab? asking regarding the star in the provider list there.

no in the next cycle, we won't work on the provider's tab

Do we need to present the deleted provider in the modal? https://www.loom.com/share/f68a5a64702e4232973d85b02056bcd5

yes, totally agree with you, I'll exclude it from the modal

i am not sure if this state is possible but, another thing i encountered in the empty state where i could not add new provider, i was either redirected to workflow page or i was missing the create sidebar. could be something in my machine tho, the flickering, in the end, is page refresh https://www.loom.com/share/36a0bf4fea8640f782fec1770fde08f5

I will investigate this case, it's weird.

Another possible not possible state is the following https://www.loom.com/share/68da5da165ce4bcc9721927ce5d30904 if it is possible should show the modal at all?

yes, I will fix this one too

the thing i did not had the time to check is analytics and backward compatibility.

We totally missed the analytics for the multi-channel provider's feature and for the new integrations list page, thank you for noticing that, I'll add it to our backlog.

other than that your code looks as clean as always ✨

thank you @djabarovgeorge for these findings and all your time spend on this feature 🙂 you really helped me a lot! 🤝

LetItRock · 2023-08-07T08:41:09Z

@djabarovgeorge all the things you've found have been fixed :)

djabarovgeorge

🚀

the last thing that is left for us is the integrations related tests that are failing.

LetItRock added 3 commits July 27, 2023 18:30

feat(web): select primary integration modal

006acad

Merge branch 'next' into nv-2606-select-primary-integration-modal

c4d6776

chore(web): removed unused var

77199d7

LetItRock self-assigned this Jul 27, 2023

github-actions bot added the @novu/web label Jul 27, 2023

LetItRock commented Jul 27, 2023

View reviewed changes

LetItRock changed the title ~~feat(web): select primary integration modal~~ [WIP] 🚧 feat(web): select primary integration modal Jul 28, 2023

feat(web): select primary integration modal logic on active integration

b646be8

LetItRock changed the title ~~[WIP] 🚧 feat(web): select primary integration modal~~ feat(web): select primary integration modal Aug 2, 2023

LetItRock commented Aug 2, 2023

View reviewed changes

LetItRock added 3 commits August 2, 2023 13:33

Merge branch 'next' into nv-2606-select-primary-integration-modal

cfadc82

Merge branch 'next' into nv-2606-select-primary-integration-modal

5ef164f

Merge branch 'next' into nv-2606-select-primary-integration-modal

baea095

github-actions bot added the @novu/shared label Aug 3, 2023

djabarovgeorge reviewed Aug 3, 2023

View reviewed changes

chore(web): fixed failing e2e tests

47b6c20

LetItRock added a commit that referenced this pull request Aug 7, 2023

chore(web): suggestions from the pr #3869

95f7058

LetItRock added 2 commits August 7, 2023 10:29

chore(web): suggestions from the pr #3869

f67e28b

Merge branch 'next' into nv-2606-select-primary-integration-modal

e0e902a

LetItRock requested a review from djabarovgeorge August 7, 2023 08:40

djabarovgeorge approved these changes Aug 7, 2023

View reviewed changes

LetItRock added 5 commits August 7, 2023 23:33

chore(web): fixed e2e tests for the integrations list

7e7ff5b

chore(web): fixed integration sidebar header for test provider

c28cce7

Merge branch 'next' into nv-2606-select-primary-integration-modal

6817e23

chore(web): fixed the alignment issue for table component and add button

9b1c10c

chore(web): trying to fix the brand e2e tests

3eecfde

LetItRock added this pull request to the merge queue Aug 8, 2023

Merged via the queue into next with commit d5303b3 Aug 8, 2023
30 checks passed

LetItRock deleted the nv-2606-select-primary-integration-modal branch August 8, 2023 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): select primary integration modal #3869

feat(web): select primary integration modal #3869

LetItRock commented Jul 27, 2023 •

edited

Loading

linear bot commented Jul 27, 2023

LetItRock Jul 27, 2023 •

edited

Loading

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Jul 27, 2023

LetItRock Aug 2, 2023

LetItRock Aug 2, 2023

LetItRock Aug 2, 2023

LetItRock Aug 2, 2023

LetItRock Aug 2, 2023

LetItRock Aug 2, 2023

djabarovgeorge left a comment

LetItRock commented Aug 4, 2023

LetItRock commented Aug 7, 2023

djabarovgeorge left a comment

		paddingLeft: withSelection ? 10 : 30,
		paddingRight: withSelection ? 10 : 30,

feat(web): select primary integration modal #3869

feat(web): select primary integration modal #3869

Conversation

LetItRock commented Jul 27, 2023 • edited Loading

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

linear bot commented Jul 27, 2023

LetItRock Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djabarovgeorge left a comment

Choose a reason for hiding this comment

LetItRock commented Aug 4, 2023

LetItRock commented Aug 7, 2023

djabarovgeorge left a comment

Choose a reason for hiding this comment

LetItRock commented Jul 27, 2023 •

edited

Loading

LetItRock Jul 27, 2023 •

edited

Loading