Update CreateServer logic so that it ensures a unique human-friendly ID is set

src/servers/helpers/ImportServersBtn.tsx

@@ -6,10 +6,10 @@ import { useCallback, useRef, useState } from 'react';
 import { Button, UncontrolledTooltip } from 'reactstrap';
 import type { FCWithDeps } from '../../container/utils';
 import { componentFactory, useDependencies } from '../../container/utils';
-import type { ServerData, ServersMap } from '../data';
+import type { ServerData, ServersMap, ServerWithId } from '../data';
 import type { ServersImporter } from '../services/ServersImporter';
 import { DuplicatedServersModal } from './DuplicatedServersModal';
-import { dedupServers } from './index';
+import { dedupServers, ensureUniqueIds } from './index';
 
 export type ImportServersBtnProps = PropsWithChildren<{
   onImport?: () => void;
@@ -19,7 +19,7 @@ export type ImportServersBtnProps = PropsWithChildren<{
 }>;
 
 type ImportServersBtnConnectProps = ImportServersBtnProps & {
-  createServers: (servers: ServerData[]) => void;
+  createServers: (servers: ServerWithId[]) => void;
   servers: ServersMap;
 };
 
@@ -41,10 +41,10 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
   const [duplicatedServers, setDuplicatedServers] = useState<ServerData[]>([]);
   const [isModalOpen,, showModal, hideModal] = useToggle();
 
-  const importedServersRef = useRef<ServerData[]>([]);
-  const newServersRef = useRef<ServerData[]>([]);
+  const importedServersRef = useRef<ServerWithId[]>([]);
+  const newServersRef = useRef<ServerWithId[]>([]);
 
-  const create = useCallback((serversData: ServerData[]) => {
+  const create = useCallback((serversData: ServerWithId[]) => {
     createServers(serversData);
     onImport();
   }, [createServers, onImport]);
@@ -54,11 +54,11 @@ const ImportServersBtn: FCWithDeps<ImportServersBtnConnectProps, ImportServersBt
         .then((importedServers) => {
           const { duplicatedServers, newServers } = dedupServers(servers, importedServers);
 
-          importedServersRef.current = importedServers;
-          newServersRef.current = newServers;
+          importedServersRef.current = ensureUniqueIds(servers, importedServers);
+          newServersRef.current = ensureUniqueIds(servers, newServers);
 
           if (duplicatedServers.length === 0) {
-            create(importedServers);
+            create(importedServersRef.current);
           } else {
             setDuplicatedServers(duplicatedServers);
             showModal();

src/servers/helpers/index.ts

@@ -6,24 +6,18 @@ import type { ServerData, ServersMap, ServerWithId } from '../data';
  * in lowercase and replacing invalid URL characters with hyphens.
  */
 function idForServer(server: ServerData): string {
+  // TODO Handle invalid URLs. If not valid url, use the value as is
   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 serversMap: ServersMap = {};
+  servers.forEach((server) => {
+    serversMap[server.id] = server;
+  });
+
+  return serversMap;
 }
 
 const serversInclude = (serversList: ServerData[], { url, apiKey }: ServerData) =>
@@ -48,3 +42,30 @@ export function dedupServers(servers: ServersMap, serversToAdd: ServerData[]): D
 
   return { duplicatedServers, newServers };
 }
+
+/**
+ * Given a servers map and a list of servers, return the same list of servers but all with an ID, ensuring the ID is
+ * unique both among all those servers and existing ones
+ */
+export function ensureUniqueIds(existingServers: ServersMap, serversList: ServerData[]): ServerWithId[] {
+  const existingIds = new Set(Object.keys(existingServers));
+  const serversWithId: ServerWithId[] = [];
+
+  serversList.forEach((server) => {
+    const baseId = idForServer(server);
+
+    let id = baseId;
+    let iterations = 1;
+    while (existingIds.has(id)) {
+      id = `${baseId}-${iterations}`;
+      iterations++;
+    }
+
+    serversWithId.push({ id, ...server });
+
+    // Add this server's ID to the list, so that it is taken into consideration for the next ones
+    existingIds.add(id);
+  });
+
+  return serversWithId;
+}