From 7eb18f7551f62e662bba33d37124f693862602c9 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 28 Mar 2024 16:29:28 -0400 Subject: [PATCH 01/14] [Backend] Prevent duplicate speaker names --- .../Controllers/SpeakerControllerTests.cs | 15 ++++++++++++++- Backend.Tests/Mocks/SpeakerRepositoryMock.cs | 5 +++++ Backend/Controllers/SpeakerController.cs | 12 ++++++++++++ Backend/Interfaces/ISpeakerRepository.cs | 1 + Backend/Repositories/SpeakerRepository.cs | 15 ++++++++++----- 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/Backend.Tests/Controllers/SpeakerControllerTests.cs b/Backend.Tests/Controllers/SpeakerControllerTests.cs index a53218b4b2..205556c36e 100644 --- a/Backend.Tests/Controllers/SpeakerControllerTests.cs +++ b/Backend.Tests/Controllers/SpeakerControllerTests.cs @@ -127,6 +127,15 @@ public void TestCreateSpeakerUnauthorized() Assert.That(result, Is.InstanceOf()); } + [Test] + public void TestCreateNameTaken() + { + var oldCount = _speakerRepo.GetAllSpeakers(ProjId).Result.Count; + var result = _speakerController.CreateSpeaker(ProjId, Name).Result; + Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); + Assert.That(_speakerRepo.GetAllSpeakers(ProjId).Result, Has.Count.EqualTo(oldCount)); + } + [Test] public void TestCreateSpeaker() { @@ -212,10 +221,14 @@ public void TestUpdateSpeakerNameNoSpeaker() } [Test] - public void TestUpdateSpeakerNameSameName() + public void TestUpdateSpeakerNameNameTaken() { var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, Name).Result; Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); + + var idOfNewSpeaker = ((ObjectResult)_speakerController.CreateSpeaker(ProjId, "Ms. Other").Result).Value as string; + result = _speakerController.UpdateSpeakerName(ProjId, idOfNewSpeaker!, Name).Result; + Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); } [Test] diff --git a/Backend.Tests/Mocks/SpeakerRepositoryMock.cs b/Backend.Tests/Mocks/SpeakerRepositoryMock.cs index 1ef2aaba85..907024447f 100644 --- a/Backend.Tests/Mocks/SpeakerRepositoryMock.cs +++ b/Backend.Tests/Mocks/SpeakerRepositoryMock.cs @@ -77,5 +77,10 @@ public Task Update(string speakerId, Speaker speaker) _speakers.Add(speaker.Clone()); return Task.FromResult(ResultOfUpdate.Updated); } + + public Task IsSpeakerNameInProject(string projectId, string name) + { + return Task.FromResult(_speakers.Any(s => s.ProjectId == projectId && s.Name == name)); + } } } diff --git a/Backend/Controllers/SpeakerController.cs b/Backend/Controllers/SpeakerController.cs index 54a0f57e8d..49f8b30d4a 100644 --- a/Backend/Controllers/SpeakerController.cs +++ b/Backend/Controllers/SpeakerController.cs @@ -92,6 +92,12 @@ public async Task CreateSpeaker(string projectId, string name) return Forbid(); } + // Ensure the name isn't taken + if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) + { + return StatusCode(StatusCodes.Status304NotModified, $"Name taken: {name}"); + } + // Create speaker and return id var speaker = new Speaker { Name = name, ProjectId = projectId }; return Ok((await _speakerRepo.Create(speaker)).Id); @@ -188,6 +194,12 @@ public async Task UpdateSpeakerName(string projectId, string spea return NotFound(speakerId); } + // Ensure the new name isn't taken + if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) + { + return StatusCode(StatusCodes.Status304NotModified, $"Name taken: {name}"); + } + // Update name and return result with id speaker.Name = name; return await _speakerRepo.Update(speakerId, speaker) switch diff --git a/Backend/Interfaces/ISpeakerRepository.cs b/Backend/Interfaces/ISpeakerRepository.cs index f0ccf1bcff..dbce48abfe 100644 --- a/Backend/Interfaces/ISpeakerRepository.cs +++ b/Backend/Interfaces/ISpeakerRepository.cs @@ -13,5 +13,6 @@ public interface ISpeakerRepository Task Delete(string projectId, string speakerId); Task DeleteAllSpeakers(string projectId); Task Update(string speakerId, Speaker speaker); + Task IsSpeakerNameInProject(string projectId, string name); } } diff --git a/Backend/Repositories/SpeakerRepository.cs b/Backend/Repositories/SpeakerRepository.cs index c06dfca5d4..f017762b2f 100644 --- a/Backend/Repositories/SpeakerRepository.cs +++ b/Backend/Repositories/SpeakerRepository.cs @@ -37,8 +37,7 @@ public async Task DeleteAllSpeakers(string projectId) public async Task GetSpeaker(string projectId, string speakerId) { var filterDef = new FilterDefinitionBuilder(); - var filter = filterDef.And(filterDef.Eq( - x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId)); + var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId)); var speakerList = await _speakerDatabase.Speakers.FindAsync(filter); try @@ -64,9 +63,7 @@ public async Task Create(Speaker speaker) public async Task Delete(string projectId, string speakerId) { var filterDef = new FilterDefinitionBuilder(); - var filter = filterDef.And( - filterDef.Eq(x => x.ProjectId, projectId), - filterDef.Eq(x => x.Id, speakerId)); + var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId)); return (await _speakerDatabase.Speakers.DeleteOneAsync(filter)).DeletedCount > 0; } @@ -88,5 +85,13 @@ public async Task Update(string speakerId, Speaker speaker) ? ResultOfUpdate.Updated : ResultOfUpdate.NoChange; } + + /// Check if with specified name is already in project + public async Task IsSpeakerNameInProject(string projectId, string name) + { + var filterDef = new FilterDefinitionBuilder(); + var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Name, name)); + return await _speakerDatabase.Speakers.CountDocumentsAsync(filter) > 0; + } } } From 947c13e191eb61cac348581ae215643f88e4d7d1 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 28 Mar 2024 16:38:07 -0400 Subject: [PATCH 02/14] Prevent terminal whitespace and empty names --- .../Controllers/SpeakerControllerTests.cs | 13 ++++++++++--- Backend/Controllers/SpeakerController.cs | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/Backend.Tests/Controllers/SpeakerControllerTests.cs b/Backend.Tests/Controllers/SpeakerControllerTests.cs index 205556c36e..ef01079033 100644 --- a/Backend.Tests/Controllers/SpeakerControllerTests.cs +++ b/Backend.Tests/Controllers/SpeakerControllerTests.cs @@ -127,11 +127,18 @@ public void TestCreateSpeakerUnauthorized() Assert.That(result, Is.InstanceOf()); } + [Test] + public void TestCreateEmptyName() + { + var result = _speakerController.CreateSpeaker(ProjId, " \n\t").Result; + Assert.That(result, Is.InstanceOf()); + } + [Test] public void TestCreateNameTaken() { var oldCount = _speakerRepo.GetAllSpeakers(ProjId).Result.Count; - var result = _speakerController.CreateSpeaker(ProjId, Name).Result; + var result = _speakerController.CreateSpeaker(ProjId, $"\n{Name} ").Result; Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); Assert.That(_speakerRepo.GetAllSpeakers(ProjId).Result, Has.Count.EqualTo(oldCount)); } @@ -223,11 +230,11 @@ public void TestUpdateSpeakerNameNoSpeaker() [Test] public void TestUpdateSpeakerNameNameTaken() { - var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, Name).Result; + var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, $" {Name}\t").Result; Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); var idOfNewSpeaker = ((ObjectResult)_speakerController.CreateSpeaker(ProjId, "Ms. Other").Result).Value as string; - result = _speakerController.UpdateSpeakerName(ProjId, idOfNewSpeaker!, Name).Result; + result = _speakerController.UpdateSpeakerName(ProjId, idOfNewSpeaker!, $"\t{Name}\n").Result; Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); } diff --git a/Backend/Controllers/SpeakerController.cs b/Backend/Controllers/SpeakerController.cs index 49f8b30d4a..03baa652d9 100644 --- a/Backend/Controllers/SpeakerController.cs +++ b/Backend/Controllers/SpeakerController.cs @@ -92,6 +92,13 @@ public async Task CreateSpeaker(string projectId, string name) return Forbid(); } + // Trim whitespace + name = name.Trim(); + if (string.IsNullOrEmpty(name)) + { + return BadRequest(name); + } + // Ensure the name isn't taken if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) { @@ -187,6 +194,13 @@ public async Task UpdateSpeakerName(string projectId, string spea return Forbid(); } + // Trim whitespace + name = name.Trim(); + if (string.IsNullOrEmpty(name)) + { + return BadRequest(name); + } + // Ensure the speaker exists var speaker = await _speakerRepo.GetSpeaker(projectId, speakerId); if (speaker is null) From 990f4d57ce26415b47a43c9fdeb0fffdd76e5bad Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 28 Mar 2024 17:18:07 -0400 Subject: [PATCH 03/14] Export consent with speaker name rather than speaker id --- Backend/Helper/Sanitization.cs | 2 +- Backend/Services/LiftService.cs | 41 ++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Backend/Helper/Sanitization.cs b/Backend/Helper/Sanitization.cs index 0c1288f817..3491f78b3f 100644 --- a/Backend/Helper/Sanitization.cs +++ b/Backend/Helper/Sanitization.cs @@ -88,12 +88,12 @@ public static string MakeFriendlyForPath(string name, string fallback = "") case UnicodeCategory.LowercaseLetter: case UnicodeCategory.UppercaseLetter: case UnicodeCategory.DecimalDigitNumber: + case UnicodeCategory.OtherLetter: stringBuilder.Append(c); break; case UnicodeCategory.DashPunctuation: case UnicodeCategory.CurrencySymbol: case UnicodeCategory.MathSymbol: - case UnicodeCategory.OtherLetter: case UnicodeCategory.OtherSymbol: stringBuilder.Append('-'); break; diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index 670daa2282..ea8ed67f2c 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -541,14 +541,32 @@ private static async Task AddAudio(LexEntry entry, List pronuncia { foreach (var audio in pronunciations) { - var lexPhonetic = new LexPhonetic(); var src = FileStorage.GenerateAudioFilePath(projectId, audio.FileName); - var dest = Path.Combine(path, audio.FileName); + if (!File.Exists(src)) + { + continue; + }; - if (!File.Exists(src)) continue; - if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase)) + var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId); + // If audio has speaker, use speaker name as file name + var safeName = speaker is not null ? Sanitization.MakeFriendlyForPath(speaker.Name) : ""; + var fileName = safeName == "" ? Path.GetFileNameWithoutExtension(audio.FileName) : safeName; + + var fileExt = Path.GetExtension(audio.FileName); + var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase); + fileExt = convertToWav ? ".wav" : fileExt; + var dest = Path.ChangeExtension(Path.Combine(path, fileName), fileExt); + + // Prevent collisions resulting from name sanitization + var duplicate = 0; + while (File.Exists(dest)) + { + duplicate++; + dest = Path.ChangeExtension(Path.Combine(path, $"{fileName}{duplicate}"), fileExt); + } + + if (convertToWav) { - dest = Path.ChangeExtension(dest, ".wav"); await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\""); } else @@ -556,16 +574,13 @@ private static async Task AddAudio(LexEntry entry, List pronuncia File.Copy(src, dest, true); } + var lexPhonetic = new LexPhonetic(); lexPhonetic.MergeIn(MultiText.Create(new LiftMultiText { { "href", dest } })); - // If audio has speaker, include speaker info as a pronunciation label - if (!audio.Protected && !string.IsNullOrEmpty(audio.SpeakerId)) + // If audio has speaker, include speaker name as a pronunciation label + if (speaker is not null) { - var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId); - if (speaker is not null) - { - var text = new LiftMultiText { { "en", $"Speaker #{speaker.Id}: {speaker.Name}" } }; - lexPhonetic.MergeIn(MultiText.Create(text)); - } + var text = new LiftMultiText { { "en", $"Speaker: {speaker.Name}" } }; + lexPhonetic.MergeIn(MultiText.Create(text)); } entry.Pronunciations.Add(lexPhonetic); } From 37b81f803ebb41de4a0fde5b6fb14e03b19d8f86 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 10:18:33 -0400 Subject: [PATCH 04/14] Update non-Latin sanitization tests --- Backend.Tests/Helper/SanitizationTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Backend.Tests/Helper/SanitizationTests.cs b/Backend.Tests/Helper/SanitizationTests.cs index 9cae32a0b7..16f034bd96 100644 --- a/Backend.Tests/Helper/SanitizationTests.cs +++ b/Backend.Tests/Helper/SanitizationTests.cs @@ -104,11 +104,12 @@ public void TestInvalidFileNames(string fileName) private static List> _namesUnfriendlyFriendly = new() { new List { "A1phaNum3ricN0Change", "A1phaNum3ricN0Change" }, + new List { "こんにちは", "こんにちは" }, new List { "RémöveOrRèpláceÄccênts", "RemoveOrReplaceAccents" }, + new List { "алтайча", "алтаича"}, new List { "math+and=currency$to { "make spaces underscores", "make_spaces_underscores" }, new List { "(){}[]", "()()()" }, - new List { "こんにちは", "-----" }, new List { "⁇⸘¡", ",,," } }; [TestCaseSource(nameof(_namesUnfriendlyFriendly))] From bee5a5602af6cd1ad8e6d1cbc5a1170023f048cd Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 12:05:27 -0400 Subject: [PATCH 05/14] Feed user error for empty/taken speaker name --- .../Controllers/SpeakerControllerTests.cs | 13 +++++++-- Backend/Controllers/SpeakerController.cs | 25 +++++++---------- public/locales/en/translation.json | 2 ++ src/backend/index.ts | 10 +++++-- .../ProjectUsers/ProjectSpeakersList.tsx | 28 ++++++++++++++++--- 5 files changed, 53 insertions(+), 25 deletions(-) diff --git a/Backend.Tests/Controllers/SpeakerControllerTests.cs b/Backend.Tests/Controllers/SpeakerControllerTests.cs index ef01079033..3086010e96 100644 --- a/Backend.Tests/Controllers/SpeakerControllerTests.cs +++ b/Backend.Tests/Controllers/SpeakerControllerTests.cs @@ -139,7 +139,7 @@ public void TestCreateNameTaken() { var oldCount = _speakerRepo.GetAllSpeakers(ProjId).Result.Count; var result = _speakerController.CreateSpeaker(ProjId, $"\n{Name} ").Result; - Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); + Assert.That(result, Is.InstanceOf()); Assert.That(_speakerRepo.GetAllSpeakers(ProjId).Result, Has.Count.EqualTo(oldCount)); } @@ -227,15 +227,22 @@ public void TestUpdateSpeakerNameNoSpeaker() Assert.That(result, Is.InstanceOf()); } + [Test] + public void TestUpdateSpeakerNameEmptyName() + { + var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, " \n\t").Result; + Assert.That(result, Is.InstanceOf()); + } + [Test] public void TestUpdateSpeakerNameNameTaken() { var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, $" {Name}\t").Result; - Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); + Assert.That(result, Is.InstanceOf()); var idOfNewSpeaker = ((ObjectResult)_speakerController.CreateSpeaker(ProjId, "Ms. Other").Result).Value as string; result = _speakerController.UpdateSpeakerName(ProjId, idOfNewSpeaker!, $"\t{Name}\n").Result; - Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified)); + Assert.That(result, Is.InstanceOf()); } [Test] diff --git a/Backend/Controllers/SpeakerController.cs b/Backend/Controllers/SpeakerController.cs index 03baa652d9..34c6140207 100644 --- a/Backend/Controllers/SpeakerController.cs +++ b/Backend/Controllers/SpeakerController.cs @@ -92,17 +92,15 @@ public async Task CreateSpeaker(string projectId, string name) return Forbid(); } - // Trim whitespace + // Ensure the new name is valid name = name.Trim(); if (string.IsNullOrEmpty(name)) { - return BadRequest(name); + return BadRequest("projectSettings.speaker.nameEmpty"); } - - // Ensure the name isn't taken if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) { - return StatusCode(StatusCodes.Status304NotModified, $"Name taken: {name}"); + return BadRequest("projectSettings.speaker.nameTaken"); } // Create speaker and return id @@ -193,14 +191,6 @@ public async Task UpdateSpeakerName(string projectId, string spea { return Forbid(); } - - // Trim whitespace - name = name.Trim(); - if (string.IsNullOrEmpty(name)) - { - return BadRequest(name); - } - // Ensure the speaker exists var speaker = await _speakerRepo.GetSpeaker(projectId, speakerId); if (speaker is null) @@ -208,10 +198,15 @@ public async Task UpdateSpeakerName(string projectId, string spea return NotFound(speakerId); } - // Ensure the new name isn't taken + // Ensure the new name is valid + name = name.Trim(); + if (string.IsNullOrEmpty(name)) + { + return BadRequest("projectSettings.speaker.nameEmpty"); + } if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) { - return StatusCode(StatusCodes.Status304NotModified, $"Name taken: {name}"); + return BadRequest("projectSettings.speaker.nameTaken"); } // Update name and return result with id diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index b235a6f1c6..72e53d489f 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -201,6 +201,8 @@ "label": "Manage speakers", "add": "Add a speaker", "enterName": "Enter the name of a new speaker", + "nameEmpty": "Speaker name is empty", + "nameTaken": "Speaker name is already taken", "delete": "Delete this speaker", "edit": "Edit speaker's name", "consent": { diff --git a/src/backend/index.ts b/src/backend/index.ts index c2d425c921..2478f6f966 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -41,9 +41,13 @@ const apiBaseURL = `${baseURL}/v1`; const config_parameters: Api.ConfigurationParameters = { basePath: baseURL }; const config = new Api.Configuration(config_parameters); -/** A list of URL suffixes for which the frontend explicitly handles errors +/** A list of URL patterns for which the frontend explicitly handles errors * and the blanket error pop ups should be suppressed.*/ -const whiteListedErrorUrls = ["users/authenticate"]; +const whiteListedErrorUrls = [ + "users/authenticate", + "/speakers/create/", + "/speakers/update/", +]; // Create an axios instance to allow for attaching interceptors to it. const axiosInstance = axios.create({ baseURL: apiBaseURL }); @@ -67,7 +71,7 @@ axiosInstance.interceptors.response.use(undefined, (err: AxiosError) => { status <= StatusCodes.NETWORK_AUTHENTICATION_REQUIRED ) { // Suppress error pop-ups for URLs the frontend already explicitly handles. - if (url && whiteListedErrorUrls.some((u) => url.endsWith(u))) { + if (url && whiteListedErrorUrls.some((u) => url.includes(u))) { return Promise.reject(err); } diff --git a/src/components/ProjectUsers/ProjectSpeakersList.tsx b/src/components/ProjectUsers/ProjectSpeakersList.tsx index 6b522729c7..89067689cf 100644 --- a/src/components/ProjectUsers/ProjectSpeakersList.tsx +++ b/src/components/ProjectUsers/ProjectSpeakersList.tsx @@ -1,6 +1,8 @@ import { Add, Edit } from "@mui/icons-material"; import { List, ListItem, ListItemIcon, ListItemText } from "@mui/material"; import { ReactElement, useCallback, useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { toast } from "react-toastify"; import { ConsentType, Speaker } from "api/models"; import { @@ -70,9 +72,18 @@ export function SpeakerListItem(props: ProjSpeakerProps): ReactElement { function EditSpeakerNameListItemIcon(props: ProjSpeakerProps): ReactElement { const [open, setOpen] = useState(false); + const { t } = useTranslation(); + const handleUpdateText = async (name: string): Promise => { - await updateSpeakerName(props.speaker.id, name, props.projectId); - await props.refresh(); + name = name.trim(); + if (!name) { + return Promise.reject(t("projectSettings.speaker.nameEmpty")); + } + await updateSpeakerName(props.speaker.id, name, props.projectId) + .then(async () => { + await props.refresh(); + }) + .catch((err) => toast.error(t(err.response?.data ?? err.message))); }; return ( @@ -129,9 +140,18 @@ interface AddSpeakerProps { export function AddSpeakerListItem(props: AddSpeakerProps): ReactElement { const [open, setOpen] = useState(false); + const { t } = useTranslation(); + const handleSubmitText = async (name: string): Promise => { - await createSpeaker(name, props.projectId); - await props.refresh(); + name = name.trim(); + if (!name) { + return Promise.reject(t("projectSettings.speaker.nameEmpty")); + } + await createSpeaker(name, props.projectId) + .then(async () => { + await props.refresh(); + }) + .catch((err) => toast.error(t(err.response?.data ?? err.message))); }; return ( From a8e939ac550ce6f13f1d4aa83785dc0e757e5e00 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 12:07:59 -0400 Subject: [PATCH 06/14] Restore newline --- Backend/Controllers/SpeakerController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Backend/Controllers/SpeakerController.cs b/Backend/Controllers/SpeakerController.cs index 34c6140207..5403acc964 100644 --- a/Backend/Controllers/SpeakerController.cs +++ b/Backend/Controllers/SpeakerController.cs @@ -191,6 +191,7 @@ public async Task UpdateSpeakerName(string projectId, string spea { return Forbid(); } + // Ensure the speaker exists var speaker = await _speakerRepo.GetSpeaker(projectId, speakerId); if (speaker is null) From 4010d96c4adfefdadcef38e1a69b6d7abad89276 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 12:11:54 -0400 Subject: [PATCH 07/14] Add missing space --- Backend.Tests/Helper/SanitizationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Backend.Tests/Helper/SanitizationTests.cs b/Backend.Tests/Helper/SanitizationTests.cs index 16f034bd96..d31c04d0a8 100644 --- a/Backend.Tests/Helper/SanitizationTests.cs +++ b/Backend.Tests/Helper/SanitizationTests.cs @@ -106,7 +106,7 @@ public void TestInvalidFileNames(string fileName) new List { "A1phaNum3ricN0Change", "A1phaNum3ricN0Change" }, new List { "こんにちは", "こんにちは" }, new List { "RémöveOrRèpláceÄccênts", "RemoveOrReplaceAccents" }, - new List { "алтайча", "алтаича"}, + new List { "алтайча", "алтаича" }, new List { "math+and=currency$to { "make spaces underscores", "make_spaces_underscores" }, new List { "(){}[]", "()()()" }, From 8f9c30b80e9d4547c7e657b81cf002411fd88618 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 15:15:30 -0400 Subject: [PATCH 08/14] Move file-name refactor to correct place --- Backend/Helper/Sanitization.cs | 4 +- Backend/Services/LiftService.cs | 70 +++++++++++++++++---------------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/Backend/Helper/Sanitization.cs b/Backend/Helper/Sanitization.cs index 3491f78b3f..bb9bfe0dc7 100644 --- a/Backend/Helper/Sanitization.cs +++ b/Backend/Helper/Sanitization.cs @@ -72,7 +72,7 @@ public static string SanitizeFileName(string fileName) /// /// Convert a string (e.g., a project name), into one friendly to use in a path. - /// Uses alphanumeric and '-' '_' ',' '(' ')'. + /// Uses international alphanumeric and '-' '_' ',' '(' ')'. /// /// Converted string, unless length 0, then returns fallback. public static string MakeFriendlyForPath(string name, string fallback = "") @@ -87,8 +87,8 @@ public static string MakeFriendlyForPath(string name, string fallback = "") { case UnicodeCategory.LowercaseLetter: case UnicodeCategory.UppercaseLetter: - case UnicodeCategory.DecimalDigitNumber: case UnicodeCategory.OtherLetter: + case UnicodeCategory.DecimalDigitNumber: stringBuilder.Append(c); break; case UnicodeCategory.DashPunctuation: diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index ea8ed67f2c..aaa5da1624 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -347,22 +347,39 @@ public async Task LiftExport( // Add consent files to export directory foreach (var speaker in projSpeakers) { - if (speaker.Consent != ConsentType.None) + if (speaker.Consent == ConsentType.None) { - var src = FileStorage.GetConsentFilePath(speaker.Id); - if (src is not null) - { - var dest = Path.Combine(consentDir, Path.GetFileName(src)); - if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase)) - { - dest = Path.ChangeExtension(dest, ".wav"); - await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\""); - } - else - { - File.Copy(src, dest); - } - } + continue; + } + + var src = FileStorage.GetConsentFilePath(speaker.Id); + if (src is null || !File.Exists(src)) + { + continue; + }; + + var safeName = Sanitization.MakeFriendlyForPath(speaker.Name); + var fileName = safeName == "" ? Path.GetFileNameWithoutExtension(src) : safeName; + var fileExt = Path.GetExtension(src); + var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase); + fileExt = convertToWav ? ".wav" : fileExt; + var dest = Path.ChangeExtension(Path.Combine(consentDir, fileName), fileExt); + + // Prevent collisions resulting from name sanitization + var duplicate = 0; + while (File.Exists(dest)) + { + duplicate++; + dest = Path.ChangeExtension(Path.Combine(consentDir, $"{fileName}{duplicate}"), fileExt); + } + + if (convertToWav) + { + await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\""); + } + else + { + File.Copy(src, dest); } } @@ -547,26 +564,10 @@ private static async Task AddAudio(LexEntry entry, List pronuncia continue; }; - var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId); - // If audio has speaker, use speaker name as file name - var safeName = speaker is not null ? Sanitization.MakeFriendlyForPath(speaker.Name) : ""; - var fileName = safeName == "" ? Path.GetFileNameWithoutExtension(audio.FileName) : safeName; - - var fileExt = Path.GetExtension(audio.FileName); - var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase); - fileExt = convertToWav ? ".wav" : fileExt; - var dest = Path.ChangeExtension(Path.Combine(path, fileName), fileExt); - - // Prevent collisions resulting from name sanitization - var duplicate = 0; - while (File.Exists(dest)) - { - duplicate++; - dest = Path.ChangeExtension(Path.Combine(path, $"{fileName}{duplicate}"), fileExt); - } - - if (convertToWav) + var dest = Path.Combine(path, Path.GetFileName(audio.FileName)); + if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase)) { + dest = Path.ChangeExtension(dest, ".wav"); await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\""); } else @@ -577,6 +578,7 @@ private static async Task AddAudio(LexEntry entry, List pronuncia var lexPhonetic = new LexPhonetic(); lexPhonetic.MergeIn(MultiText.Create(new LiftMultiText { { "href", dest } })); // If audio has speaker, include speaker name as a pronunciation label + var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId); if (speaker is not null) { var text = new LiftMultiText { { "en", $"Speaker: {speaker.Name}" } }; From 271856f829be71d67a98da6960fecc42992d0c07 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 1 Apr 2024 15:22:43 -0400 Subject: [PATCH 09/14] Remove superfluous path function --- Backend/Services/LiftService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index aaa5da1624..1932ef290e 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -564,7 +564,7 @@ private static async Task AddAudio(LexEntry entry, List pronuncia continue; }; - var dest = Path.Combine(path, Path.GetFileName(audio.FileName)); + var dest = Path.Combine(path, audio.FileName); if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase)) { dest = Path.ChangeExtension(dest, ".wav"); From 2a7e36614891472fc9fb3923bf3b73ccb07c1540 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 2 Apr 2024 11:59:50 -0400 Subject: [PATCH 10/14] [ProjectSpeakers] Add unit tests --- .../ProjectUsers/ProjectSpeakersList.tsx | 56 +++++++--- .../tests/ProjectSpeakersList.test.tsx | 105 +++++++++++++++++- 2 files changed, 139 insertions(+), 22 deletions(-) diff --git a/src/components/ProjectUsers/ProjectSpeakersList.tsx b/src/components/ProjectUsers/ProjectSpeakersList.tsx index 89067689cf..40046d322b 100644 --- a/src/components/ProjectUsers/ProjectSpeakersList.tsx +++ b/src/components/ProjectUsers/ProjectSpeakersList.tsx @@ -18,6 +18,20 @@ import { import { EditTextDialog, SubmitTextDialog } from "components/Dialogs"; import SpeakerConsentListItemIcon from "components/ProjectUsers/SpeakerConsentListItemIcon"; +export enum ProjectSpeakersId { + ButtonAdd = "speaker-add-button", + ButtonAddCancel = "speaker-add-cancel-button", + ButtonAddConfirm = "speaker-add-confirm-button", + ButtonDeleteCancel = "speaker-delete-cancel-button", + ButtonDeleteConfirm = "speaker-delete-confirm-button", + ButtonDeletePrefix = "speaker-delete-button-", + ButtonEditCancel = "speaker-edit-cancel-button", + ButtonEditConfirm = "speaker-edit-confirm-button", + ButtonEditPrefix = "speaker-edit-button-", + TextFieldAdd = "speaker-add-textfield", + TextFieldEdit = "speaker-edit-textfield", +} + export default function ProjectSpeakersList(props: { projectId: string; }): ReactElement { @@ -79,6 +93,10 @@ function EditSpeakerNameListItemIcon(props: ProjSpeakerProps): ReactElement { if (!name) { return Promise.reject(t("projectSettings.speaker.nameEmpty")); } + if (name === props.speaker.name) { + return; + } + await updateSpeakerName(props.speaker.id, name, props.projectId) .then(async () => { await props.refresh(); @@ -89,21 +107,23 @@ function EditSpeakerNameListItemIcon(props: ProjSpeakerProps): ReactElement { return ( } onClick={() => setOpen(true)} textId="projectSettings.speaker.edit" /> - setOpen(false)} - open={open} - text={props.speaker.name} - textFieldId="project-speakers-edit-name" - titleId="projectSettings.speaker.edit" - updateText={handleUpdateText} - /> + {open && ( + setOpen(false)} + open={open} + text={props.speaker.name} + textFieldId={ProjectSpeakersId.TextFieldEdit} + titleId="projectSettings.speaker.edit" + updateText={handleUpdateText} + /> + )} ); } @@ -117,9 +137,9 @@ function DeleteSpeakerListItemIcon(props: ProjSpeakerProps): ReactElement { return ( } onClick={() => setOpen(true)} textId="projectSettings.speaker.add" /> setOpen(false)} open={open} submitText={handleSubmitText} - textFieldId="project-speakers-add-name" + textFieldId={ProjectSpeakersId.TextFieldAdd} titleId="projectSettings.speaker.enterName" /> diff --git a/src/components/ProjectUsers/tests/ProjectSpeakersList.test.tsx b/src/components/ProjectUsers/tests/ProjectSpeakersList.test.tsx index d2fc4e82b7..9698b420d5 100644 --- a/src/components/ProjectUsers/tests/ProjectSpeakersList.test.tsx +++ b/src/components/ProjectUsers/tests/ProjectSpeakersList.test.tsx @@ -2,16 +2,27 @@ import renderer from "react-test-renderer"; import ProjectSpeakersList, { AddSpeakerListItem, + ProjectSpeakersId, SpeakerListItem, } from "components/ProjectUsers/ProjectSpeakersList"; import { randomSpeaker } from "types/project"; +// Dialog uses portals, which are not supported in react-test-renderer. +jest.mock("@mui/material/Dialog", () => + jest.requireActual("@mui/material/Container") +); + jest.mock("backend", () => ({ - createSpeaker: (args: any[]) => mockCreateSpeaker(...args), - deleteSpeaker: (args: any[]) => mockDeleteSpeaker(...args), + createSpeaker: (name: string, projectId?: string) => + mockCreateSpeaker(name, projectId), + deleteSpeaker: (speakerId: string, projectId?: string) => + mockDeleteSpeaker(speakerId, projectId), getAllSpeakers: (projectId?: string) => mockGetAllSpeakers(projectId), - updateSpeakerName: (args: any[]) => mockUpdateSpeakerName(...args), + updateSpeakerName: (speakerId: string, name: string, projectId?: string) => + mockUpdateSpeakerName(speakerId, name, projectId), })); +// Mock "i18n", else `Error: connect ECONNREFUSED ::1:80` +jest.mock("i18n", () => ({})); const mockCreateSpeaker = jest.fn(); const mockDeleteSpeaker = jest.fn(); @@ -33,15 +44,101 @@ const renderProjectSpeakersList = async ( beforeEach(() => { jest.resetAllMocks(); + mockCreateSpeaker.mockResolvedValue(""); + mockGetAllSpeakers.mockResolvedValue(mockSpeakers); + mockUpdateSpeakerName.mockResolvedValue(""); }); describe("ProjectSpeakersList", () => { it("shows right number of speakers and an item to add a speaker", async () => { - mockGetAllSpeakers.mockResolvedValue(mockSpeakers); await renderProjectSpeakersList(); expect(testRenderer.root.findAllByType(SpeakerListItem)).toHaveLength( mockSpeakers.length ); expect(testRenderer.root.findByType(AddSpeakerListItem)).toBeTruthy(); }); + + it("updates speaker name if changed", async () => { + await renderProjectSpeakersList(); + + // Click the button to edit speaker + const editButton = testRenderer.root.findByProps({ + id: `${ProjectSpeakersId.ButtonEditPrefix}${mockSpeakers[0].id}`, + }); + await renderer.act(() => { + editButton.props.onClick(); + }); + + // Submit the current name with extra whitespace + const mockEvent = { + preventDefault: jest.fn(), + target: { value: `\t\t${mockSpeakers[0].name} ` }, + }; + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.TextFieldEdit }) + .props.onChange(mockEvent); + }); + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.ButtonEditConfirm }) + .props.onClick(); + }); + + // Ensure no name update was submitted + expect(mockUpdateSpeakerName).not.toHaveBeenCalled(); + + // Click the button to edit speaker + await renderer.act(() => { + editButton.props.onClick(); + }); + + // Submit a new name + const name = "Mr. Different"; + mockEvent.target.value = name; + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.TextFieldEdit }) + .props.onChange(mockEvent); + }); + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.ButtonEditConfirm }) + .props.onClick(); + }); + + // Ensure the name update was submitted + expect(mockUpdateSpeakerName.mock.calls[0][1]).toEqual(name); + }); + + it("trims whitespace when adding a speaker", async () => { + await renderProjectSpeakersList(); + + // Click the button to add a speaker + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.ButtonAdd }) + .props.onClick(); + }); + + // Submit the name of the speaker with extra whitespace + const name = "Ms. Nym"; + const mockEvent = { + preventDefault: jest.fn(), + target: { value: ` ${name}\t ` }, + }; + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.TextFieldAdd }) + .props.onChange(mockEvent); + }); + await renderer.act(() => { + testRenderer.root + .findByProps({ id: ProjectSpeakersId.ButtonAddConfirm }) + .props.onClick(); + }); + + // Ensure new speaker was submitted with trimmed name + expect(mockCreateSpeaker.mock.calls[0][0]).toEqual(name); + }); }); From 0d823fd5dafa363e11ac246dbd551c5b178dfbe4 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 5 Apr 2024 08:05:34 -0400 Subject: [PATCH 11/14] [ListControllerTests] Add test for exported consent files --- .../Controllers/LiftControllerTests.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/Backend.Tests/Controllers/LiftControllerTests.cs b/Backend.Tests/Controllers/LiftControllerTests.cs index f5ceda8534..cd093c72a1 100644 --- a/Backend.Tests/Controllers/LiftControllerTests.cs +++ b/Backend.Tests/Controllers/LiftControllerTests.cs @@ -454,6 +454,63 @@ public async Task TestDeletedWordsExportToLift() Assert.That(notFoundResult, Is.TypeOf()); } + [Test] + public async Task TestExportConsentFileWithSpeakerName() + { + // Add word so there's something to export + await _wordRepo.Create(Util.RandomWord(_projId)); + + // Add speakers to project with names that will collide when sanitized + await _speakerRepo.Create(new Speaker { Name = "No consent!", ProjectId = _projId }); + + var nameNoExt = "Underscored_name"; + var speakerNoExt = await _speakerRepo.Create( + new Speaker { Name = nameNoExt, ProjectId = _projId, Consent = ConsentType.Image }); + var speakerNoExt1 = await _speakerRepo.Create( + new Speaker { Name = "Underscored name", ProjectId = _projId, Consent = ConsentType.Image }); + var speakerNoExt2 = await _speakerRepo.Create( + new Speaker { Name = "Underscored_náme", ProjectId = _projId, Consent = ConsentType.Image }); + + var nameExts = "Imagination"; + var speakerExts = await _speakerRepo.Create( + new Speaker { Name = nameExts, ProjectId = _projId, Consent = ConsentType.Image }); + var speakerExts1 = await _speakerRepo.Create( + new Speaker { Name = "Imágination", ProjectId = _projId, Consent = ConsentType.Image }); + + // Create mock consent files tied to speaker ids + var pathNoExt = FileStorage.GenerateConsentFilePath(speakerNoExt.Id); + var pathNoExt1 = FileStorage.GenerateConsentFilePath(speakerNoExt1.Id); + var pathNoExt2 = FileStorage.GenerateConsentFilePath(speakerNoExt2.Id); + var expectedFileNames = new List { nameNoExt, $"{nameNoExt}1", $"{nameNoExt}2" }; + + var ext = ".png"; + var pathExt = FileStorage.GenerateConsentFilePath(speakerExts.Id, ext); + var pathExt1 = FileStorage.GenerateConsentFilePath(speakerExts1.Id, ext); + expectedFileNames.AddRange(new List { $"{nameExts}{ext}", $"{nameExts}1{ext}" }); + + var mockFiles = new List { pathNoExt, pathNoExt1, pathNoExt2, pathExt, pathExt1 }; + mockFiles.ForEach(path => File.Create(path).Dispose()); + + // Export the project + var exportedFilePath = await _liftController.CreateLiftExport(_projId); + var exportedDirectory = FileOperations.ExtractZipFile(exportedFilePath, null); + var exportedProjDir = Directory.GetDirectories(exportedDirectory).First(); + + // Verify all consent files were copied over with speaker names + var consentFiles = Directory.GetFiles(Path.Combine(exportedProjDir, "consent")); + var consentFileNames = consentFiles.Select(path => Path.GetFileName(path)).ToList(); + Assert.That(consentFileNames, Has.Count.EqualTo(expectedFileNames.Count)); + foreach (var file in expectedFileNames) + { + Assert.That(consentFileNames.Contains(file)); + }; + + // Delete everything + mockFiles.ForEach(path => File.Delete(path)); + File.Delete(exportedFilePath); + Directory.Delete(exportedDirectory, true); + } + private static RoundTripObj[] _roundTripCases = { new("Gusillaay.zip", "gsl-Qaaa-x-orth", new List(), 8045), From 90fd6006a463380a27ed6bfbaff6d670d3debccf Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 5 Apr 2024 08:47:50 -0400 Subject: [PATCH 12/14] Fix ChangeExtension use on Linux --- Backend/Helper/FileStorage.cs | 7 ++++++- Backend/Services/LiftService.cs | 13 +++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Backend/Helper/FileStorage.cs b/Backend/Helper/FileStorage.cs index 124c658978..602a50dec5 100644 --- a/Backend/Helper/FileStorage.cs +++ b/Backend/Helper/FileStorage.cs @@ -99,7 +99,12 @@ public static string GenerateAvatarFilePath(string userId) /// Throws when id invalid. public static string GenerateConsentFilePath(string speakerId, string? extension = null) { - var fileName = Path.ChangeExtension(Sanitization.SanitizeId(speakerId), extension); + var fileName = Sanitization.SanitizeId(speakerId); + // On Linux, `Path.ChangeExtension` adds an unwanted final . if `extension` is null/empty. + if (!string.IsNullOrEmpty(extension)) + { + fileName = Path.ChangeExtension(fileName, extension); + } return GenerateFilePath(ConsentDir, fileName); } diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index 1932ef290e..dd2b6f5c57 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -363,14 +363,23 @@ public async Task LiftExport( var fileExt = Path.GetExtension(src); var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase); fileExt = convertToWav ? ".wav" : fileExt; - var dest = Path.ChangeExtension(Path.Combine(consentDir, fileName), fileExt); + var dest = Path.Combine(consentDir, fileName); + // On Linux, `Path.ChangeExtension` adds an unwanted final . if `extension` is null/empty. + if (!string.IsNullOrEmpty(fileExt)) + { + dest = Path.ChangeExtension(dest, fileExt); + } // Prevent collisions resulting from name sanitization var duplicate = 0; while (File.Exists(dest)) { duplicate++; - dest = Path.ChangeExtension(Path.Combine(consentDir, $"{fileName}{duplicate}"), fileExt); + dest = Path.Combine(consentDir, $"{fileName}{duplicate}"); + if (!string.IsNullOrEmpty(fileExt)) + { + dest = Path.ChangeExtension(dest, fileExt); + } } if (convertToWav) From 71e89c05c030c49e889f21eb2b50b82da9aec380 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 16 Apr 2024 17:04:09 -0400 Subject: [PATCH 13/14] Update speaker name error handling --- Backend/Controllers/SpeakerController.cs | 33 +++++++++++++++--------- public/locales/en/translation.json | 2 +- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/Backend/Controllers/SpeakerController.cs b/Backend/Controllers/SpeakerController.cs index 5403acc964..06cb2e090a 100644 --- a/Backend/Controllers/SpeakerController.cs +++ b/Backend/Controllers/SpeakerController.cs @@ -79,6 +79,21 @@ public async Task GetSpeaker(string projectId, string speakerId) return Ok(speaker); } + /// Checks if given speaker name is valid for the project with given id. + /// null if valid; a BadRequestObjectResult if invalid. + private async Task CheckSpeakerName(string projectId, string name) + { + if (string.IsNullOrEmpty(name)) + { + return BadRequest("projectSettings.speaker.nameEmpty"); + } + if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) + { + return BadRequest("projectSettings.speaker.nameTaken"); + } + return null; + } + /// Creates a for the specified projectId /// Id of created Speaker [HttpGet("create/{name}", Name = "CreateSpeaker")] @@ -94,13 +109,10 @@ public async Task CreateSpeaker(string projectId, string name) // Ensure the new name is valid name = name.Trim(); - if (string.IsNullOrEmpty(name)) + var nameError = await CheckSpeakerName(projectId, name); + if (nameError is not null) { - return BadRequest("projectSettings.speaker.nameEmpty"); - } - if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) - { - return BadRequest("projectSettings.speaker.nameTaken"); + return nameError; } // Create speaker and return id @@ -201,13 +213,10 @@ public async Task UpdateSpeakerName(string projectId, string spea // Ensure the new name is valid name = name.Trim(); - if (string.IsNullOrEmpty(name)) + var nameError = await CheckSpeakerName(projectId, name); + if (nameError is not null) { - return BadRequest("projectSettings.speaker.nameEmpty"); - } - if (await _speakerRepo.IsSpeakerNameInProject(projectId, name)) - { - return BadRequest("projectSettings.speaker.nameTaken"); + return nameError; } // Update name and return result with id diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index 2140311073..3741480be4 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -202,7 +202,7 @@ "add": "Add a speaker", "enterName": "Enter the name of a new speaker", "nameEmpty": "Speaker name is empty", - "nameTaken": "Speaker name is already taken", + "nameTaken": "Speaker name is already taken in this project", "delete": "Delete this speaker", "edit": "Edit speaker's name", "consent": { From 8f5564a973c89cd5e852ca628e2d34d4843cb050 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 16 Apr 2024 17:18:08 -0400 Subject: [PATCH 14/14] Extract Path.ChangeExtension wrapper --- Backend/Helper/FileOperations.cs | 10 ++++++++++ Backend/Helper/FileStorage.cs | 7 +------ Backend/Services/LiftService.cs | 13 ++----------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/Backend/Helper/FileOperations.cs b/Backend/Helper/FileOperations.cs index 4af9c2eb86..4846afe93c 100644 --- a/Backend/Helper/FileOperations.cs +++ b/Backend/Helper/FileOperations.cs @@ -124,5 +124,15 @@ public static List FindFilesWithExtension(string dir, string ext, bool r } return files; } + + /// Like Path.ChangeExtension, but doesn't add a . for empty-string extension. + public static string ChangeExtension(string path, string? extension) + { + if (extension == "") + { + extension = null; + } + return Path.ChangeExtension(path, extension); + } } } diff --git a/Backend/Helper/FileStorage.cs b/Backend/Helper/FileStorage.cs index 602a50dec5..e584f9eb81 100644 --- a/Backend/Helper/FileStorage.cs +++ b/Backend/Helper/FileStorage.cs @@ -99,12 +99,7 @@ public static string GenerateAvatarFilePath(string userId) /// Throws when id invalid. public static string GenerateConsentFilePath(string speakerId, string? extension = null) { - var fileName = Sanitization.SanitizeId(speakerId); - // On Linux, `Path.ChangeExtension` adds an unwanted final . if `extension` is null/empty. - if (!string.IsNullOrEmpty(extension)) - { - fileName = Path.ChangeExtension(fileName, extension); - } + var fileName = FileOperations.ChangeExtension(Sanitization.SanitizeId(speakerId), extension); return GenerateFilePath(ConsentDir, fileName); } diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs index dd2b6f5c57..750599ade4 100644 --- a/Backend/Services/LiftService.cs +++ b/Backend/Services/LiftService.cs @@ -363,23 +363,14 @@ public async Task LiftExport( var fileExt = Path.GetExtension(src); var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase); fileExt = convertToWav ? ".wav" : fileExt; - var dest = Path.Combine(consentDir, fileName); - // On Linux, `Path.ChangeExtension` adds an unwanted final . if `extension` is null/empty. - if (!string.IsNullOrEmpty(fileExt)) - { - dest = Path.ChangeExtension(dest, fileExt); - } + var dest = FileOperations.ChangeExtension(Path.Combine(consentDir, fileName), fileExt); // Prevent collisions resulting from name sanitization var duplicate = 0; while (File.Exists(dest)) { duplicate++; - dest = Path.Combine(consentDir, $"{fileName}{duplicate}"); - if (!string.IsNullOrEmpty(fileExt)) - { - dest = Path.ChangeExtension(dest, fileExt); - } + dest = FileOperations.ChangeExtension(Path.Combine(consentDir, $"{fileName}{duplicate}"), fileExt); } if (convertToWav)