Word service update return type (#4150)

Author: imnasnainaec

Backend.Tests/Controllers/LiftControllerTests.cs

@@ -413,10 +413,10 @@ public async Task TestDeletedWordsExportToLift()
             // Create untouched word.
             await _wordRepo.Create(secondWord);
 
-            word.Id = "";
+            word.Id = wordToUpdate.Id;
             word.Vernacular = "updated";
 
-            await _wordService.Update(_projId, UserId, wordToUpdate.Id, word);
+            await _wordService.Update(UserId, word);
             await _wordService.DeleteFrontierWord(_projId, UserId, wordToDelete.Id);
 
             _liftService.SetExportInProgress(UserId, ExportId);

Backend.Tests/Services/WordServiceTests.cs

@@ -82,7 +82,7 @@ public void TestDeleteAudio()
         [Test]
         public void TestUpdateNotInFrontierNull()
         {
-            Assert.That(_wordService.Update(ProjId, UserId, WordId, new Word()).Result, Is.Null);
+            Assert.That(_wordService.Update(UserId, new Word() { Id = WordId, ProjectId = ProjId }).Result, Is.Null);
         }
 
         [Test]
@@ -92,7 +92,7 @@ public void TestUpdateReplacesFrontierWord()
             Assert.That(word, Is.Not.Null);
             var oldId = word.Id;
             word.Vernacular = "NewVern";
-            Assert.That(_wordService.Update(ProjId, UserId, oldId, word).Result, Is.EqualTo(word.Id));
+            Assert.That(_wordService.Update(UserId, word).Result!.Guid, Is.EqualTo(word.Guid));
             var frontier = _wordRepo.GetFrontier(ProjId).Result;
             Assert.That(frontier, Has.Count.EqualTo(1));
             var newWord = frontier.First();
@@ -110,13 +110,13 @@ public void TestUpdateUsingCitationForm()
 
             // Update something other than Vernacular and make sure UsingCitationForm is still true.
             word.Note = new() { Text = "change word's note" };
-            _ = _wordService.Update(ProjId, UserId, word.Id, word).Result;
-            Assert.That(word.UsingCitationForm, Is.True);
+            var nonVernUpdate = _wordService.Update(UserId, word).Result;
+            Assert.That(nonVernUpdate!.UsingCitationForm, Is.True);
 
             // Update the Vernacular and make sure UsingCitationForm is false.
-            word.Vernacular = "change word's vernacular form";
-            _ = _wordService.Update(ProjId, UserId, word.Id, word).Result;
-            Assert.That(word.UsingCitationForm, Is.False);
+            nonVernUpdate.Vernacular = "change word's vernacular form";
+            var vernUpdate = _wordService.Update(UserId, nonVernUpdate).Result;
+            Assert.That(vernUpdate!.UsingCitationForm, Is.False);
         }
 
         [Test]

Backend/Controllers/AudioController.cs

@@ -13,21 +13,15 @@ namespace BackendFramework.Controllers
     [Authorize]
     [Produces("application/json")]
     [Route("v1/projects/{projectId}/words/{wordId}/audio")]
-    public class AudioController : Controller
+    public class AudioController(
+        IWordRepository wordRepo, IWordService wordService, IPermissionService permissionService) : Controller
     {
-        private readonly IWordRepository _wordRepo;
-        private readonly IPermissionService _permissionService;
-        private readonly IWordService _wordService;
+        private readonly IWordRepository _wordRepo = wordRepo;
+        private readonly IPermissionService _permissionService = permissionService;
+        private readonly IWordService _wordService = wordService;
 
         private const string otelTagName = "otel.AudioController";
 
-        public AudioController(IWordRepository repo, IWordService wordService, IPermissionService permissionService)
-        {
-            _wordRepo = repo;
-            _permissionService = permissionService;
-            _wordService = wordService;
-        }
-
         /// <summary> Gets the audio file in the form of a stream from disk. </summary>
         /// <returns> Audio file stream. </returns>
         [AllowAnonymous]
@@ -144,9 +138,9 @@ public async Task<IActionResult> UploadAudioFile(
             word.Audio.Add(audio);
 
             // Update the word with new audio file
-            await _wordService.Update(projectId, userId, wordId, word);
+            string? updatedId = (await _wordService.Update(userId, word))?.Id;
 
-            return Ok(word.Id);
+            return updatedId is null ? NotFound($"wordId: {wordId}") : Ok(updatedId);
         }
 
         /// <summary> Deletes audio in <see cref="Word"/> with specified ID </summary>
@@ -177,12 +171,8 @@ public async Task<IActionResult> DeleteAudioFile(string projectId, string wordId
                 return new UnsupportedMediaTypeResult();
             }
 
-            var newWord = await _wordService.DeleteAudio(projectId, userId, wordId, fileName);
-            if (newWord is not null)
-            {
-                return Ok(newWord.Id);
-            }
-            return NotFound($"wordId: {wordId}; fileName: {fileName}");
+            string? newId = (await _wordService.DeleteAudio(projectId, userId, wordId, fileName))?.Id;
+            return newId is null ? NotFound($"wordId: {wordId}; fileName: {fileName}") : Ok(newId);
         }
     }
 }

Backend/Controllers/WordController.cs

@@ -13,21 +13,15 @@ namespace BackendFramework.Controllers
     [Authorize]
     [Produces("application/json")]
     [Route("v1/projects/{projectId}/words")]
-    public class WordController : Controller
+    public class WordController(
+        IWordRepository wordRepo, IWordService wordService, IPermissionService permissionService) : Controller
     {
-        private readonly IWordRepository _wordRepo;
-        private readonly IPermissionService _permissionService;
-        private readonly IWordService _wordService;
+        private readonly IWordRepository _wordRepo = wordRepo;
+        private readonly IPermissionService _permissionService = permissionService;
+        private readonly IWordService _wordService = wordService;
 
         private const string otelTagName = "otel.WordController";
 
-        public WordController(IWordRepository repo, IWordService wordService, IPermissionService permissionService)
-        {
-            _wordRepo = repo;
-            _permissionService = permissionService;
-            _wordService = wordService;
-        }
-
         /// <summary> Deletes specified Frontier <see cref="Word"/>. </summary>
         [HttpDelete("frontier/{wordId}", Name = "DeleteFrontierWord")]
         [ProducesResponseType(StatusCodes.Status200OK)]
@@ -43,8 +37,8 @@ public async Task<IActionResult> DeleteFrontierWord(string projectId, string wor
             }
             var userId = _permissionService.GetUserId(HttpContext);
 
-            var deletedWordId = await _wordService.DeleteFrontierWord(projectId, userId, wordId);
-            return deletedWordId is null ? NotFound() : Ok();
+            var deleted = await _wordService.DeleteFrontierWord(projectId, userId, wordId);
+            return deleted is null ? NotFound() : Ok();
         }
 
         /// <summary> Returns <see cref="Word"/> with specified id. </summary>
@@ -60,12 +54,9 @@ public async Task<IActionResult> GetWord(string projectId, string wordId)
             {
                 return Forbid();
             }
-            var word = await _wordRepo.GetWord(projectId, wordId);
-            if (word is null)
-            {
-                return NotFound();
-            }
-            return Ok(word);
+
+            Word? word = await _wordRepo.GetWord(projectId, wordId);
+            return word is null ? NotFound() : Ok(word);
         }
 
         /// <summary> Checks if Frontier for specified <see cref="Project"/> has any words. </summary>
@@ -80,6 +71,7 @@ public async Task<IActionResult> HasFrontierWords(string projectId)
             {
                 return Forbid();
             }
+
             return Ok(await _wordRepo.HasFrontierWords(projectId));
         }
 
@@ -95,6 +87,7 @@ public async Task<IActionResult> GetFrontierCount(string projectId)
             {
                 return Forbid();
             }
+
             return Ok(await _wordRepo.GetFrontierCount(projectId));
         }
 
@@ -110,6 +103,7 @@ public async Task<IActionResult> GetProjectFrontierWords(string projectId)
             {
                 return Forbid();
             }
+
             return Ok(await _wordRepo.GetFrontier(projectId));
         }
 
@@ -125,6 +119,7 @@ public async Task<IActionResult> IsInFrontier(string projectId, string wordId)
             {
                 return Forbid();
             }
+
             return Ok(await _wordRepo.IsInFrontier(projectId, wordId));
         }
 
@@ -170,7 +165,8 @@ public async Task<IActionResult> GetDuplicateId(string projectId, [FromBody, Bin
             }
             word.ProjectId = projectId;
 
-            return Ok(await _wordService.FindContainingWord(word) ?? "");
+            string? containingId = await _wordService.FindContainingWord(word);
+            return Ok(containingId ?? "");
         }
 
         /// <summary> Combines a <see cref="Word"/> into the existing duplicate with specified wordId. </summary>
@@ -203,9 +199,8 @@ public async Task<IActionResult> UpdateDuplicate(
                 return Conflict();
             }
 
-            await _wordService.Update(duplicatedWord.ProjectId, userId, duplicatedWord.Id, duplicatedWord);
-
-            return Ok(duplicatedWord.Id);
+            string? newId = (await _wordService.Update(userId, duplicatedWord))?.Id;
+            return newId is null ? NotFound() : Ok(newId);
         }
 
         /// <summary> Creates a <see cref="Word"/>. </summary>
@@ -221,9 +216,11 @@ public async Task<IActionResult> CreateWord(string projectId, [FromBody, BindReq
             {
                 return Forbid();
             }
+
             word.ProjectId = projectId;
-            var userId = _permissionService.GetUserId(HttpContext);
-            return Ok((await _wordService.Create(userId, word)).Id);
+
+            string newId = (await _wordService.Create(_permissionService.GetUserId(HttpContext), word)).Id;
+            return Ok(newId);
         }
 
         /// <summary> Updates a <see cref="Word"/>. </summary>
@@ -241,17 +238,13 @@ public async Task<IActionResult> UpdateWord(
             {
                 return Forbid();
             }
-            var document = await _wordRepo.GetWord(projectId, wordId);
-            if (document is null)
-            {
-                return NotFound();
-            }
 
-            // Add the found id to the updated word.
-            word.Id = document.Id;
-            var userId = _permissionService.GetUserId(HttpContext);
-            await _wordService.Update(projectId, userId, wordId, word);
-            return Ok(word.Id);
+            // Don't allow changing project or manually setting the Id.
+            word.ProjectId = projectId;
+            word.Id = wordId;
+
+            string? newId = (await _wordService.Update(_permissionService.GetUserId(HttpContext), word))?.Id;
+            return newId is null ? NotFound() : Ok(newId);
         }
 
         /// <summary> Restore a deleted <see cref="Word"/>. </summary>
@@ -276,7 +269,7 @@ public async Task<IActionResult> RestoreWord(string projectId, string wordId)
             return Ok(await _wordService.RestoreFrontierWords(projectId, [wordId]));
         }
 
-        /// <summary> Revert words from an dictionary of word ids (key: to revert to; value: from frontier). </summary>
+        /// <summary> Revert words from a dictionary of word ids (key: to revert to; value: from frontier). </summary>
         /// <returns> Id dictionary of all words successfully updated (key: was in frontier; value: new id). </returns>
         [HttpPost("revertwords", Name = "RevertWords")]
         [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(Dictionary<string, string>))]
@@ -296,11 +289,15 @@ public async Task<IActionResult> RevertWords(
             foreach (var kv in wordIds)
             {
                 var idToRevert = kv.Value;
-                var word = await _wordRepo.GetWord(projectId, kv.Key);
-                if (word is not null && await _wordRepo.IsInFrontier(projectId, idToRevert))
+                var priorWord = await _wordRepo.GetWord(projectId, kv.Key);
+                if (priorWord is not null)
                 {
-                    await _wordService.Update(projectId, userId, idToRevert, word);
-                    updates[idToRevert] = word.Id;
+                    priorWord.Id = idToRevert;
+                    var newId = (await _wordService.Update(userId, priorWord))?.Id;
+                    if (newId is not null)
+                    {
+                        updates[idToRevert] = newId;
+                    }
                 }
             }
             return Ok(updates);

Backend/Interfaces/IWordService.cs

@@ -8,7 +8,7 @@ public interface IWordService
     {
         Task<Word> Create(string userId, Word word);
         Task<List<Word>> Create(string userId, List<Word> words);
-        Task<string?> Update(string projectId, string userId, string wordId, Word word);
+        Task<Word?> Update(string userId, Word word);
         Task<Word?> DeleteAudio(string projectId, string userId, string wordId, string fileName);
         Task<string?> DeleteFrontierWord(string projectId, string userId, string wordId);
         Task<bool> RestoreFrontierWords(string projectId, List<string> wordIds);

Backend/Services/WordService.cs

@@ -113,26 +113,33 @@ public async Task<bool> RestoreFrontierWords(string projectId, List<string> word
         }
 
         /// <summary> Makes a new word in the Frontier with changes made </summary>
-        /// <returns> Id of updated word, or null if not found </returns>
-        public async Task<string?> Update(string projectId, string userId, string wordId, Word word)
+        /// <returns> Updated word, or null if word-to-update not found </returns>
+        public async Task<Word?> Update(string userId, Word word)
         {
             using var activity = OtelService.StartActivityWithTag(otelTagName, "updating a word in Frontier");
 
-            // We only want to update words that are in the frontier
-            var oldWord = await _wordRepo.DeleteFrontier(projectId, wordId);
-            if (oldWord is null)
+            var oldWordId = word.Id; // Capture the id in case of changes.
+            var oldWord = await _wordRepo.GetWord(word.ProjectId, oldWordId);
+            if (oldWord is null || !await _wordRepo.IsInFrontier(word.ProjectId, oldWordId))
             {
                 return null;
             }
 
+            word.Created = oldWord.Created;
+            if (!word.History.Contains(oldWordId))
+            {
+                word.History.Add(oldWordId);
+            }
             // If an imported word was using the citation form for its Vernacular,
             // only keep UsingCitationForm true if the Vernacular hasn't changed.
             word.UsingCitationForm &= word.Vernacular == oldWord.Vernacular;
 
-            word.ProjectId = projectId;
-            word.History.Add(wordId);
+            var newWord = await Create(userId, word);
+
+            // Don't remove the old Frontier word until the new word is successfully created.
+            await _wordRepo.DeleteFrontier(word.ProjectId, oldWordId);
 
-            return (await Create(userId, word)).Id;
+            return newWord;
         }
 
         /// <summary> Checks if a word being added is a duplicate of a preexisting word. </summary>