Fix project renumber (#1597)

* fix: generateUniqueFileName

* loadProject should not generate new names

* update comments

* extract and test getProjectNumber

* spell

* finish jsdoc

* use getProjectNumber

* cleanup loadProject

* create a `incrementProjectNumber` function

* spelling

Co-authored-by: Carlos Valente <34649812+cpvalente@users.noreply.github.com>

---------

Co-authored-by: Carlos Valente <34649812+cpvalente@users.noreply.github.com>

apps/server/src/services/project-service/ProjectService.ts

@@ -80,14 +80,13 @@ export async function getCurrentProject(): Promise<{ filename: string; pathToFil
 
 /**
  * Private function loads a project file and handles necessary side effects
+ * @param projectData
+ * @param fileName file name of the project including the extension
  */
-async function loadProject(projectData: DatabaseModel, projectName: string) {
+async function loadProject(projectData: DatabaseModel, fileName: string) {
-  // we need to make sure the file name is unique in the projects directory
-  const pathToNewFile = generateUniqueFileName(publicDir.projectsDir, projectName);
-
   // change LowDB to point to new file
-  await initPersistence(getPathToProject(pathToNewFile), projectData);
+  await initPersistence(getPathToProject(fileName), projectData);
-  logger.info(LogOrigin.Server, `Loaded project ${projectName}`);
+  logger.info(LogOrigin.Server, `Loaded project ${fileName}`);
 
   // stop the runtime service
   runtimeService.stop();
@@ -97,23 +96,22 @@ async function loadProject(projectData: DatabaseModel, projectName: string) {
   await initRundown(firstRundown, projectData.customFields);
 
   // persist the project selection
-  const newName = getFileNameFromPath(pathToNewFile);
+  await setLastLoadedProject(fileName);
-  await setLastLoadedProject(newName);
 
   // update the service state
   currentProjectState = {
     status: 'INITIALIZED',
-    currentProjectName: newName,
+    currentProjectName: fileName,
   };
 
-  return newName;
+  return fileName;
 }
 
 /**
  * Loads the demo project
  */
 export async function loadDemoProject(): Promise<string> {
-  return loadProject(demoDb, config.demoProject);
+  return createProject(config.demoProject, demoDb);
 }
 
 /**
@@ -121,12 +119,14 @@ export async function loadDemoProject(): Promise<string> {
  * to be composed in the loading functions
  */
 async function loadNewProject(): Promise<string> {
-  return loadProject(dbModel, config.newProject);
+  return createProject(config.newProject, dbModel);
 }
 
 /**
  * Private function handles side effects on corrupted files
  * Corrupted files in this context contain data that failed domain validation
+ * @param filePath path to the project type include the fileName and extension
+ * @param fileName as extracted from filePath, includes extension
  */
 async function handleCorruptedFile(filePath: string, fileName: string): Promise<string> {
   // copy file to corrupted folder
@@ -173,9 +173,10 @@ export async function initialiseProject(): Promise<string> {
 
 /**
  * Loads a data from a file into the runtime
+ * @param fileName file name of the project including the extension
  */
-export async function loadProjectFile(name: string): Promise<string> {
+export async function loadProjectFile(fileName: string): Promise<string> {
-  const filePath = doesProjectExist(name);
+  const filePath = doesProjectExist(fileName);
   if (filePath === null) {
     throw new Error('Project file not found');
   }
@@ -183,11 +184,11 @@ export async function loadProjectFile(name: string): Promise<string> {
   // when loading a project file, we allow parsing to fail and interrupt the process
   const fileData = await parseJsonFile(filePath);
   const result = parseDatabaseModel(fileData);
-  let parsedFileName = name;
+  let parsedFileName = fileName;
 
   if (result.errors.length > 0) {
     logger.warning(LogOrigin.Server, 'Project loaded with errors');
-    parsedFileName = await handleCorruptedFile(filePath, name);
+    parsedFileName = await handleCorruptedFile(filePath, fileName);
   }
 
   const projectName = await loadProject(result.data, parsedFileName);
@@ -256,13 +257,14 @@ export async function renameProjectFile(originalFile: string, newFilename: strin
 
 /**
  * Creates a new project file and applies its result
+ * @param fileName file name of the project including the extension
+ * @param initialData db to initialize the project with
  */
-export async function createProject(filename: string, initialData: Partial<DatabaseModel>): Promise<string> {
+export async function createProject(fileName: string, initialData: Partial<DatabaseModel>): Promise<string> {
   const data = safeMerge(dbModel, initialData);
-
+  const fileNameWithExtension = generateUniqueFileName(publicDir.projectsDir, ensureJsonExtension(fileName));
-  const fileNameWithExtension = ensureJsonExtension(filename);
+  await loadProject(data, fileNameWithExtension);
-  const newFilename = await loadProject(data, fileNameWithExtension);
+  return fileNameWithExtension;
-  return newFilename;
 }
 
 /**

apps/server/src/utils/__tests__/fileManagement.test.ts

@@ -1,7 +1,12 @@
 import { describe, it, expect, Mock } from 'vitest';
 import * as fs from 'fs';
 
-import { appendToName, ensureJsonExtension, generateUniqueFileName } from '../fileManagement.js';
+import {
+  appendToName,
+  ensureJsonExtension,
+  generateUniqueFileName,
+  incrementProjectNumber,
+} from '../fileManagement.js';
 
 // Mock fs.existsSync to control the test environment
 vi.mock('fs', () => ({
@@ -90,3 +95,25 @@ describe('generateUniqueFileName', () => {
     expect(uniqueFilename).toBe(expectedFilename);
   });
 });
+
+describe('file index', () => {
+  it('sets index to 1 when there is no index', () => {
+    expect(incrementProjectNumber('test file.json')).toBe('test file (1).json');
+  });
+
+  it('increments to 2 when index is 1', () => {
+    expect(incrementProjectNumber('test file (1).json')).toBe('test file (2).json');
+  });
+
+  it('does not count number not wrapped in parenthesis', () => {
+    expect(incrementProjectNumber('test file 1.json')).toBe('test file 1 (1).json');
+  });
+
+  it('does not count number if there is not a space', () => {
+    expect(incrementProjectNumber('test file(1).json')).toBe('test file(1) (1).json');
+  });
+
+  it('counts multi digit numbers', () => {
+    expect(incrementProjectNumber('test file (890).json')).toBe('test file (891).json');
+  });
+});

apps/server/src/utils/fileManagement.ts

@@ -1,6 +1,6 @@
 import { existsSync, mkdirSync, PathLike } from 'fs';
 import { readdir, copyFile, unlink } from 'fs/promises';
-import { basename, extname, join, parse } from 'path';
+import { basename, join, parse } from 'path';
 
 /**
  * @description Creates a directory if it doesn't exist
@@ -53,16 +53,11 @@ export function appendToName(filePath: string, append: string): string {
  * If a file with the same name already exists, appends a counter to the filename.
  */
 export function generateUniqueFileName(directory: string, filename: string): string {
-  const extension = extname(filename);
-  const baseName = basename(filename, extension);
-
-  let counter = 0;
   let uniqueFilename = filename;
 
   while (fileExists(uniqueFilename)) {
-    counter++;
     // Append counter to filename if the file exists.
-    uniqueFilename = `${baseName} (${counter})${extension}`;
+    uniqueFilename = incrementProjectNumber(uniqueFilename);
   }
 
   return uniqueFilename;
@@ -114,3 +109,23 @@ export async function dockerSafeRename(oldPath: PathLike, newPath: PathLike) {
   await copyFile(oldPath, newPath);
   await unlink(oldPath);
 }
+
+/**
+ * finds potential file index number in our (*) format and increments
+ * the number section (*) must be separated from the name by a space
+ * @example incrementProjectNumber('test(1).json') -> 'test(1).json'
+  * @example incrementProjectNumber('test (1).json') -> 'test(2).json'
+ */
+export function incrementProjectNumber(path: string): string {
+  const { dir, name, ext } = parse(path);
+
+  if (!name.endsWith(')')) return join(dir, `${name} (1)${ext}`);
+
+  const openingParenIndex = name.lastIndexOf(' (');
+  if (openingParenIndex === -1) return join(dir, `${name} (1)${ext}`);
+
+  const maybeNumber = Number(name.slice(openingParenIndex + 2, -1));
+  if (isNaN(maybeNumber)) return join(dir, `${name} (1)${ext}`);
+
+  return join(dir, `${name.slice(0, openingParenIndex)} (${maybeNumber + 1})${ext}`);
+}