Extract logic to determine if a list of servers contains duplicates

scripts/docker/servers_from_env.sh

@@ -4,6 +4,8 @@ set -e
 
 ME=$(basename $0)
 
+# In order to allow people to pre-configure a server in their shlink-web-client instance via env vars, this function
+# dumps a servers.json file based on the values provided via env vars
 setup_single_shlink_server() {
   [ -n "$SHLINK_SERVER_URL" ] || return 0
   [ -n "$SHLINK_SERVER_API_KEY" ] || return 0

src/servers/helpers/ImportServersBtn.tsx

@@ -9,6 +9,7 @@ import { componentFactory, useDependencies } from '../../container/utils';
 import type { ServerData, ServersMap } from '../data';
 import type { ServersImporter } from '../services/ServersImporter';
 import { DuplicatedServersModal } from './DuplicatedServersModal';
+import { dedupServers } from './index';
 
 export type ImportServersBtnProps = PropsWithChildren<{
   onImport?: () => void;
@@ -26,9 +27,6 @@ type ImportServersBtnDeps = {
   ServersImporter: ServersImporter
 };
 
-const serversInclude = (servers: ServerData[], { url, apiKey }: ServerData) =>
-  servers.some((server) => server.url === url && server.apiKey === apiKey);
-
 const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBtnDeps> = ({
   createServers,
   servers,
@@ -43,7 +41,9 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
   const [duplicatedServers, setDuplicatedServers] = useState<ServerData[]>([]);
   const [isModalOpen,, showModal, hideModal] = useToggle();
 
-  const serversToCreate = useRef<ServerData[]>([]);
+  const importedServersRef = useRef<ServerData[]>([]);
+  const newServersRef = useRef<ServerData[]>([]);
+
   const create = useCallback((serversData: ServerData[]) => {
     createServers(serversData);
     onImport();
@@ -51,22 +51,21 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
   const onFile = useCallback(
     async ({ target }: ChangeEvent<HTMLInputElement>) =>
       serversImporter.importServersFromFile(target.files?.[0])
-        .then((newServers) => {
+        .then((importedServers) => {
-          serversToCreate.current = newServers;
+          const { duplicatedServers, newServers } = dedupServers(servers, importedServers);
 
-          const existingServers = Object.values(servers);
+          importedServersRef.current = importedServers;
-          const dupServers = newServers.filter((server) => serversInclude(existingServers, server));
+          newServersRef.current = newServers;
-          const hasDuplicatedServers = !!dupServers.length;
 
-          if (!hasDuplicatedServers) {
+          if (duplicatedServers.length === 0) {
-            create(newServers);
+            create(importedServers);
           } else {
-            setDuplicatedServers(dupServers);
+            setDuplicatedServers(duplicatedServers);
             showModal();
           }
         })
         .then(() => {
-          // Reset input after processing file
+          // Reset file input after processing file
           (target as { value: string | null }).value = null;
         })
         .catch(onImportError),
@@ -74,13 +73,13 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
   );
 
   const createAllServers = useCallback(() => {
-    create(serversToCreate.current);
+    create(importedServersRef.current);
     hideModal();
-  }, [create, hideModal, serversToCreate]);
+  }, [create, hideModal]);
   const createNonDuplicatedServers = useCallback(() => {
-    create(serversToCreate.current.filter((server) => !serversInclude(duplicatedServers, server)));
+    create(newServersRef.current);
     hideModal();
-  }, [create, duplicatedServers, hideModal]);
+  }, [create, hideModal]);
 
   return (
     <>
@@ -91,7 +90,15 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
         You can create servers by importing a CSV file with <b>name</b>, <b>apiKey</b> and <b>url</b> columns.
       </UncontrolledTooltip>
 
-      <input type="file" accept=".csv" className="d-none" ref={ref} onChange={onFile} aria-hidden />
+      <input
+        type="file"
+        accept=".csv"
+        className="d-none"
+        aria-hidden
+        ref={ref}
+        onChange={onFile}
+        data-testid="csv-file-input"
+      />
 
       <DuplicatedServersModal
         isOpen={isModalOpen}

src/servers/helpers/index.ts

@@ -0,0 +1,50 @@
+import { groupBy } from '@shlinkio/data-manipulation';
+import type { ServerData, ServersMap, ServerWithId } from '../data';
+
+/**
+ * Builds a potentially unique ID for a server, based on concatenating their name and the hostname of their domain, all
+ * in lowercase and replacing invalid URL characters with hyphens.
+ */
+function idForServer(server: ServerData): string {
+  const url = new URL(server.url);
+  return `${server.name} ${url.host}`.toLowerCase().replace(/[^a-zA-Z0-9-_.~]/g, '-');
+}
+
+export function serverWithId(server: ServerWithId | ServerData): ServerWithId {
+  if ('id' in server) {
+    return server;
+  }
+
+  const id = idForServer(server);
+  return { ...server, id };
+}
+
+export function serversListToMap(servers: ServerWithId[]): ServersMap {
+  return servers.reduce<ServersMap>(
+    (acc, server) => ({ ...acc, [server.id]: server }),
+    {},
+  );
+}
+
+const serversInclude = (serversList: ServerData[], { url, apiKey }: ServerData) =>
+  serversList.some((server) => server.url === url && server.apiKey === apiKey);
+
+export type DedupServersResult = {
+  /** Servers which already exist in the reference list */
+  duplicatedServers: ServerData[];
+  /** Servers which are new based on a reference list */
+  newServers: ServerData[];
+};
+
+/**
+ * Given a list of new servers, checks which of them already exist in a servers map, and which don't
+ */
+export function dedupServers(servers: ServersMap, serversToAdd: ServerData[]): DedupServersResult {
+  const serversList = Object.values(servers);
+  const { duplicatedServers = [], newServers = [] } = groupBy(
+    serversToAdd,
+    (server) => serversInclude(serversList, server) ? 'duplicatedServers' : 'newServers',
+  );
+
+  return { duplicatedServers, newServers };
+}

src/servers/reducers/servers.ts

@@ -1,7 +1,7 @@
 import type { PayloadAction } from '@reduxjs/toolkit';
 import { createSlice } from '@reduxjs/toolkit';
-import { randomUUID } from '../../utils/utils';
 import type { ServerData, ServersMap, ServerWithId } from '../data';
+import { serversListToMap, serverWithId } from '../helpers';
 
 interface EditServer {
   serverId: string;
@@ -15,19 +15,6 @@ interface SetAutoConnect {
 
 const initialState: ServersMap = {};
 
-const serverWithId = (server: ServerWithId | ServerData): ServerWithId => {
-  if ('id' in server) {
-    return server;
-  }
-
-  return { ...server, id: randomUUID() };
-};
-
-const serversListToMap = (servers: ServerWithId[]): ServersMap => servers.reduce<ServersMap>(
-  (acc, server) => ({ ...acc, [server.id]: server }),
-  {},
-);
-
 export const { actions, reducer } = createSlice({
   name: 'shlink/servers',
   initialState,

test/servers/helpers/ImportServersBtn.test.tsx

@@ -1,4 +1,4 @@
-import { fireEvent, screen, waitFor } from '@testing-library/react';
+import { screen, waitFor } from '@testing-library/react';
 import { fromPartial } from '@total-typescript/shoehorn';
 import type { ServersMap, ServerWithId } from '../../../src/servers/data';
 import type {
@@ -9,6 +9,7 @@ import { checkAccessibility } from '../../__helpers__/accessibility';
 import { renderWithEvents } from '../../__helpers__/setUpTest';
 
 describe('<ImportServersBtn />', () => {
+  const csvFile = new File([''], 'servers.csv', { type: 'text/csv' });
   const onImportMock = vi.fn();
   const createServersMock = vi.fn();
   const importServersFromFile = vi.fn().mockResolvedValue([]);
@@ -54,14 +55,13 @@ describe('<ImportServersBtn />', () => {
   });
 
   it('imports servers when file input changes', async () => {
-    const { container } = setUp();
+    const { user } = setUp();
-    const input = container.querySelector('[type=file]');
+
+    const input = screen.getByTestId('csv-file-input');
+    await user.upload(input, csvFile);
 
-    if (input) {
-      fireEvent.change(input, { target: { files: [''] } });
-    }
     expect(importServersFromFile).toHaveBeenCalledTimes(1);
-    await waitFor(() => expect(createServersMock).toHaveBeenCalledTimes(1));
+    expect(createServersMock).toHaveBeenCalledTimes(1);
   });
 
   it.each([
@@ -70,15 +70,14 @@ describe('<ImportServersBtn />', () => {
   ])('creates expected servers depending on selected option in modal', async (btnName, savesDuplicatedServers) => {
     const existingServer = fromPartial<ServerWithId>({ id: 'abc', url: 'existingUrl', apiKey: 'existingApiKey' });
     const newServer = fromPartial<ServerWithId>({ url: 'newUrl', apiKey: 'newApiKey' });
-    const { container, user } = setUp({}, { abc: existingServer });
+    const { user } = setUp({}, { abc: existingServer });
-    const input = container.querySelector('[type=file]');
+    const input = screen.getByTestId('csv-file-input');
+
     importServersFromFile.mockResolvedValue([existingServer, newServer]);
 
     expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
-    if (input) {
+    await user.upload(input, csvFile);
-      fireEvent.change(input, { target: { files: [''] } });
+    expect(screen.getByRole('dialog')).toBeInTheDocument();
-    }
-    await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument());
     await user.click(screen.getByRole('button', { name: btnName }));
 
     expect(createServersMock).toHaveBeenCalledWith(savesDuplicatedServers ? [existingServer, newServer] : [newServer]);