From c0b2bcf596e2123d880301a00e355cdf42ca0f4e Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Mon, 9 Oct 2023 15:01:58 -0400 Subject: [PATCH 01/11] [ReviewEntriesActions] Fix flag not saving; Add tests for note and flag (#2668) --- .../Redux/ReviewEntriesActions.ts | 1 + .../tests/ReviewEntriesActions.test.tsx | 67 ++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/goals/ReviewEntries/ReviewEntriesComponent/Redux/ReviewEntriesActions.ts b/src/goals/ReviewEntries/ReviewEntriesComponent/Redux/ReviewEntriesActions.ts index 34e3d6ffa5..7fb58f09ca 100644 --- a/src/goals/ReviewEntries/ReviewEntriesComponent/Redux/ReviewEntriesActions.ts +++ b/src/goals/ReviewEntries/ReviewEntriesComponent/Redux/ReviewEntriesActions.ts @@ -146,6 +146,7 @@ export function updateFrontierWord( getSenseFromEditSense(s, editWord.senses) ); editWord.note = newNote(editSource.noteText, editWord.note?.language); + editWord.flag = { ...editSource.flag }; // Update the word in the backend, and retrieve the id. editSource.id = (await backend.updateWord(editWord)).id; diff --git a/src/goals/ReviewEntries/ReviewEntriesComponent/tests/ReviewEntriesActions.test.tsx b/src/goals/ReviewEntries/ReviewEntriesComponent/tests/ReviewEntriesActions.test.tsx index fbee89406a..a391f8f7e1 100644 --- a/src/goals/ReviewEntries/ReviewEntriesComponent/tests/ReviewEntriesActions.test.tsx +++ b/src/goals/ReviewEntries/ReviewEntriesComponent/tests/ReviewEntriesActions.test.tsx @@ -12,7 +12,7 @@ import { ReviewEntriesWord, } from "goals/ReviewEntries/ReviewEntriesComponent/ReviewEntriesTypes"; import { newSemanticDomain } from "types/semanticDomain"; -import { newGloss, newSense, newWord } from "types/word"; +import { newFlag, newGloss, newNote, newSense, newWord } from "types/word"; import { Bcp47Code } from "types/writingSystem"; const mockGetWord = jest.fn(); @@ -100,7 +100,7 @@ describe("ReviewEntriesActions", () => { expect(mockUpdateWord.mock.calls[0][0]).toEqual(newFrontierWord); } - describe("Adds data", () => { + describe("Changes data", () => { it("Changes the vernacular.", async () => { const newRevWord = mockReviewEntriesWord("foo2"); const newFrontierWord = mockFrontierWord("foo2"); @@ -109,6 +109,44 @@ describe("ReviewEntriesActions", () => { checkResultantData(newFrontierWord); }); + it("Changes the note.", async () => { + const oldNoteText = "old-note"; + const oldRevWord = mockReviewEntriesWord(); + oldRevWord.noteText = oldNoteText; + const oldFrontierWord = mockFrontierWord(); + oldFrontierWord.note = newNote(oldNoteText, Bcp47Code.Pt); + + const newNoteText = "new-note"; + const newRevWord = mockReviewEntriesWord(); + newRevWord.noteText = newNoteText; + const newFrontierWord = mockFrontierWord(); + newFrontierWord.note = newNote(newNoteText, Bcp47Code.Pt); + + mockGetWordResolve(oldFrontierWord); + await makeDispatch(newRevWord, oldRevWord); + checkResultantData(newFrontierWord); + }); + + it("Changes the flag.", async () => { + const oldFlagText = "old-flag"; + const oldRevWord = mockReviewEntriesWord(); + oldRevWord.flag = newFlag(oldFlagText); + const oldFrontierWord = mockFrontierWord(); + oldFrontierWord.flag = newFlag(oldFlagText); + + const newFlagText = "new-flag"; + const newRevWord = mockReviewEntriesWord(); + newRevWord.flag = newFlag(newFlagText); + const newFrontierWord = mockFrontierWord(); + newFrontierWord.flag = newFlag(newFlagText); + + mockGetWordResolve(oldFrontierWord); + await makeDispatch(newRevWord, oldRevWord); + checkResultantData(newFrontierWord); + }); + }); + + describe("Adds data", () => { it("Adds a gloss to an extant sense.", async () => { const newRevWord = mockReviewEntriesWord(); newRevWord.senses[0].glosses.push(gloss1); @@ -141,6 +179,17 @@ describe("ReviewEntriesActions", () => { await makeDispatch(newRevWord, mockReviewEntriesWord()); checkResultantData(newFrontierWord); }); + + it("Adds a flag.", async () => { + const newFlagText = "new-flag"; + const newRevWord = mockReviewEntriesWord(); + newRevWord.flag = newFlag(newFlagText); + const newFrontierWord = mockFrontierWord(); + newFrontierWord.flag = newFlag(newFlagText); + + await makeDispatch(newRevWord, mockReviewEntriesWord()); + checkResultantData(newFrontierWord); + }); }); describe("Removes data", () => { @@ -153,6 +202,7 @@ describe("ReviewEntriesActions", () => { }); const newRevWord = mockReviewEntriesWord(); newRevWord.senses.push(oldSense); + const oldFrontierWord = mockFrontierWord(); const oldFrontierSense = sense1(); oldFrontierWord.senses.push({ @@ -176,6 +226,7 @@ describe("ReviewEntriesActions", () => { }); const newRevWord = mockReviewEntriesWord(); newRevWord.senses.push(oldSense); + const oldFrontierWord = mockFrontierWord(); const oldFrontierSense = sense1(); oldFrontierWord.senses.push({ @@ -200,6 +251,18 @@ describe("ReviewEntriesActions", () => { await makeDispatch(mockReviewEntriesWord(), oldRevWord); checkResultantData(mockFrontierWord()); }); + + it("Removes the flag.", async () => { + const oldFlagText = "old-flag"; + const oldRevWord = mockReviewEntriesWord(); + oldRevWord.flag = newFlag(oldFlagText); + const oldFrontierWord = mockFrontierWord(); + oldRevWord.flag = newFlag(oldFlagText); + + mockGetWordResolve(oldFrontierWord); + await makeDispatch(mockReviewEntriesWord(), oldRevWord); + checkResultantData(mockFrontierWord()); + }); }); describe("Circumvents bad data", () => { From 47537c4ff833a1efc0514209f08739de7f49cdad Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 11:34:17 -0400 Subject: [PATCH 02/11] [InviteController] Clean up logic and add tests (#2663) --- .../Controllers/InviteControllerTests.cs | 165 ++++++++++++++++++ Backend/Controllers/InviteController.cs | 98 ++++++----- src/api/models/email-invite-status.ts | 2 +- .../ProjectInvite/ProjectInvite.tsx | 2 +- 4 files changed, 218 insertions(+), 49 deletions(-) create mode 100644 Backend.Tests/Controllers/InviteControllerTests.cs diff --git a/Backend.Tests/Controllers/InviteControllerTests.cs b/Backend.Tests/Controllers/InviteControllerTests.cs new file mode 100644 index 0000000000..1da286f377 --- /dev/null +++ b/Backend.Tests/Controllers/InviteControllerTests.cs @@ -0,0 +1,165 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Backend.Tests.Mocks; +using BackendFramework.Controllers; +using BackendFramework.Interfaces; +using BackendFramework.Models; +using BackendFramework.Services; +using Microsoft.AspNetCore.Mvc; +using NUnit.Framework; + +namespace Backend.Tests.Controllers +{ + public class InviteControllerTests : IDisposable + { + private IProjectRepository _projRepo = null!; + private IUserRepository _userRepo = null!; + private IInviteService _inviteService = null!; + private IPermissionService _permissionService = null!; + private InviteController _inviteController = null!; + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _inviteController?.Dispose(); + } + } + + private string _projId = null!; + private string _tokenActive = null!; + private string _tokenExpired = null!; + private const string EmailActive = "active@token.email"; + private const string EmailExpired = "expired@token.email"; + private const string MissingId = "MISSING_ID"; + + [SetUp] + public async Task Setup() + { + _projRepo = new ProjectRepositoryMock(); + _userRepo = new UserRepositoryMock(); + _permissionService = new PermissionServiceMock(); + _inviteService = new InviteService( + _projRepo, _userRepo, _permissionService, new UserRoleRepositoryMock(), new EmailServiceMock()); + _inviteController = new InviteController(_inviteService, _projRepo, _userRepo, _permissionService); + + var tokenPast = new EmailInvite(-1) { Email = EmailExpired }; + _tokenExpired = tokenPast.Token; + var tokenFuture = new EmailInvite(1) { Email = EmailActive }; + _tokenActive = tokenFuture.Token; + _projId = (await _projRepo.Create(new Project + { + Name = "InviteControllerTests", + InviteTokens = new List { tokenPast, tokenFuture } + }))!.Id; + } + + [Test] + public void TestEmailInviteToProject() + { + var data = new EmailInviteData { ProjectId = _projId }; + var result = (ObjectResult)_inviteController.EmailInviteToProject(data).Result; + Assert.That(result.Value, Is.Not.Empty); + } + + [Test] + public void TestEmailInviteToProjectUnauthorized() + { + _inviteController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext(); + var result = _inviteController.EmailInviteToProject(new EmailInviteData()).Result; + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public void TestEmailInviteToProjectNoProject() + { + var data = new EmailInviteData { ProjectId = MissingId }; + var result = _inviteController.EmailInviteToProject(data).Result; + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public void TestValidateTokenNoProject() + { + var result = _inviteController.ValidateToken(MissingId, _tokenActive).Result; + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public void TestValidateTokenNoTokenNoUser() + { + var result = _inviteController.ValidateToken(_projId, "not-a-token").Result; + Assert.That(result, Is.InstanceOf()); + var value = ((OkObjectResult)result).Value; + Assert.That(value, Is.InstanceOf()); + + var status = (EmailInviteStatus)value!; + Assert.That(status.IsTokenValid, Is.False); + Assert.That(status.IsUserValid, Is.False); + } + + [Test] + public void TestValidateTokenExpiredTokenNoUser() + { + var result = _inviteController.ValidateToken(_projId, _tokenExpired).Result; + Assert.That(result, Is.InstanceOf()); + var value = ((OkObjectResult)result).Value; + Assert.That(value, Is.InstanceOf()); + + var status = (EmailInviteStatus)value!; + Assert.That(status.IsTokenValid, Is.False); + Assert.That(status.IsUserValid, Is.False); + } + + [Test] + public void TestValidateTokenValidTokenNoUser() + { + var result = _inviteController.ValidateToken(_projId, _tokenActive).Result; + Assert.That(result, Is.InstanceOf()); + var value = ((OkObjectResult)result).Value; + Assert.That(value, Is.InstanceOf()); + + var status = (EmailInviteStatus)value!; + Assert.That(status.IsTokenValid, Is.True); + Assert.That(status.IsUserValid, Is.False); + } + + [Test] + public void TestValidateTokenValidTokenUserAlreadyInProject() + { + var roles = new Dictionary { [_projId] = "role-id" }; + _userRepo.Create(new User { Email = EmailActive, ProjectRoles = roles }); + + var result = _inviteController.ValidateToken(_projId, _tokenActive).Result; + Assert.That(result, Is.InstanceOf()); + var value = ((OkObjectResult)result).Value; + Assert.That(value, Is.InstanceOf()); + + var status = (EmailInviteStatus)value!; + Assert.That(status.IsTokenValid, Is.True); + Assert.That(status.IsUserValid, Is.False); + } + + [Test] + public void TestValidateTokenExpiredTokenUserAvailable() + { + _userRepo.Create(new User { Email = EmailExpired }); + + var result = _inviteController.ValidateToken(_projId, _tokenExpired).Result; + Assert.That(result, Is.InstanceOf()); + var value = ((OkObjectResult)result).Value; + Assert.That(value, Is.InstanceOf()); + + var status = (EmailInviteStatus)value!; + Assert.That(status.IsTokenValid, Is.False); + Assert.That(status.IsUserValid, Is.True); + } + } +} diff --git a/Backend/Controllers/InviteController.cs b/Backend/Controllers/InviteController.cs index f4f5b78d1b..5feee3c85a 100644 --- a/Backend/Controllers/InviteController.cs +++ b/Backend/Controllers/InviteController.cs @@ -73,82 +73,86 @@ public async Task ValidateToken(string projectId, string token) var tokenObj = new EmailInvite(); foreach (var tok in project.InviteTokens) { - if (tok.Token == token && DateTime.Now < tok.ExpireTime) + if (tok.Token == token) { - isTokenValid = true; tokenObj = tok; + if (DateTime.Now < tok.ExpireTime) + { + isTokenValid = true; + } break; } } var users = await _userRepo.GetAllUsers(); var currentUser = new User(); - var isUserRegistered = false; + var isUserRegisteredAndNotInProject = false; foreach (var user in users) { if (user.Email == tokenObj.Email) { currentUser = user; - isUserRegistered = true; + if (!user.ProjectRoles.ContainsKey(projectId)) + { + isUserRegisteredAndNotInProject = true; + } break; } } - var status = new EmailInviteStatus(isTokenValid, isUserRegistered); - if (isTokenValid && !isUserRegistered) + var status = new EmailInviteStatus(isTokenValid, isUserRegisteredAndNotInProject); + if (!isTokenValid || !isUserRegisteredAndNotInProject) { return Ok(status); } - if (isTokenValid && isUserRegistered - && !currentUser.ProjectRoles.ContainsKey(projectId) - && await _inviteService.RemoveTokenAndCreateUserRole(project, currentUser, tokenObj)) + if (await _inviteService.RemoveTokenAndCreateUserRole(project, currentUser, tokenObj)) { return Ok(status); } - return Ok(new EmailInviteStatus(false, false)); + return Ok(new EmailInviteStatus(false, true)); } + } - /// - /// This is used in a [FromBody] serializer, so its attributes cannot be set to readonly. - /// - public class EmailInviteData - { - [Required] - public string EmailAddress { get; set; } - [Required] - public string Message { get; set; } - [Required] - public string ProjectId { get; set; } - [Required] - public Role Role { get; set; } - [Required] - public string Domain { get; set; } + /// + /// This is used in a [FromBody] serializer, so its attributes cannot be set to readonly. + /// + public class EmailInviteData + { + [Required] + public string EmailAddress { get; set; } + [Required] + public string Message { get; set; } + [Required] + public string ProjectId { get; set; } + [Required] + public Role Role { get; set; } + [Required] + public string Domain { get; set; } - public EmailInviteData() - { - EmailAddress = ""; - Message = ""; - ProjectId = ""; - Role = Role.Harvester; - Domain = ""; - } + public EmailInviteData() + { + EmailAddress = ""; + Message = ""; + ProjectId = ""; + Role = Role.Harvester; + Domain = ""; } + } - /// - /// This is used in an OpenAPI return value serializer, so its attributes must be defined as properties. - /// - public class EmailInviteStatus - { - [Required] - public bool IsTokenValid { get; set; } - [Required] - public bool IsUserRegistered { get; set; } + /// + /// This is used in an OpenAPI return value serializer, so its attributes must be defined as properties. + /// + public class EmailInviteStatus + { + [Required] + public bool IsTokenValid { get; set; } + [Required] + public bool IsUserValid { get; set; } - public EmailInviteStatus(bool isTokenValid, bool isUserRegistered) - { - IsTokenValid = isTokenValid; - IsUserRegistered = isUserRegistered; - } + public EmailInviteStatus(bool isTokenValid, bool isUserRegistered) + { + IsTokenValid = isTokenValid; + IsUserValid = isUserRegistered; } } } diff --git a/src/api/models/email-invite-status.ts b/src/api/models/email-invite-status.ts index 51bababf90..ed840bbe91 100644 --- a/src/api/models/email-invite-status.ts +++ b/src/api/models/email-invite-status.ts @@ -29,5 +29,5 @@ export interface EmailInviteStatus { * @type {boolean} * @memberof EmailInviteStatus */ - isUserRegistered: boolean; + isUserValid: boolean; } diff --git a/src/components/ProjectInvite/ProjectInvite.tsx b/src/components/ProjectInvite/ProjectInvite.tsx index 9f30bbb738..fcbe7c3d5c 100644 --- a/src/components/ProjectInvite/ProjectInvite.tsx +++ b/src/components/ProjectInvite/ProjectInvite.tsx @@ -24,7 +24,7 @@ export default function ProjectInvite(): ReactElement { const validateLink = useCallback(async (): Promise => { if (project && token) { const status = await backend.validateLink(project, token); - if (status.isTokenValid && status.isUserRegistered) { + if (status.isTokenValid && status.isUserValid) { navigate(Path.Login); return; } From e30eac50cdd685641c1ab3ae9f92478716462cd0 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 12:31:22 -0400 Subject: [PATCH 03/11] [InviteService] Add tests (#2672) --- Backend.Tests/Services/InviteServiceTests.cs | 83 ++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 Backend.Tests/Services/InviteServiceTests.cs diff --git a/Backend.Tests/Services/InviteServiceTests.cs b/Backend.Tests/Services/InviteServiceTests.cs new file mode 100644 index 0000000000..415db5bdcd --- /dev/null +++ b/Backend.Tests/Services/InviteServiceTests.cs @@ -0,0 +1,83 @@ +using System.Linq; +using Backend.Tests.Mocks; +using BackendFramework.Interfaces; +using BackendFramework.Models; +using BackendFramework.Services; +using NUnit.Framework; + +namespace Backend.Tests.Services +{ + public class InviteServiceTests + { + private IProjectRepository _projRepo = null!; + private IUserRepository _userRepo = null!; + private IUserRoleRepository _userRoleRepo = null!; + private IEmailService _emailService = null!; + private IPermissionService _permService = null!; + private InviteService _inviteService = null!; + private const string Email = "user@domain.com"; + + + private Project _proj = null!; + private User _user = null!; + + [SetUp] + public void Setup() + { + _projRepo = new ProjectRepositoryMock(); + _userRepo = new UserRepositoryMock(); + _userRoleRepo = new UserRoleRepositoryMock(); + _emailService = new EmailServiceMock(); + _permService = new PermissionServiceMock(_userRepo); + _inviteService = new InviteService(_projRepo, _userRepo, _permService, _userRoleRepo, _emailService); + + _proj = _projRepo.Create(new Project { Name = "InviteServiceTests" }).Result!; + _user = _userRepo.Create(new User()).Result!; + } + + [Test] + public void TestCreateLinkWithToken() + { + var url = _inviteService.CreateLinkWithToken(_proj, Role.Harvester, Email).Result; + Assert.That(url, Does.Contain(Email)); + Assert.That(url, Does.Contain(_proj.Id)); + var token = _projRepo.GetProject(_proj.Id).Result!.InviteTokens.First().Token; + Assert.That(url, Does.Contain(token)); + } + + [Test] + public void TestRemoveTokenAndCreateUserRoleOwnerException() + { + var invite = new EmailInvite { Role = Role.Owner }; + Assert.That( + () => _inviteService.RemoveTokenAndCreateUserRole(_proj, _user, invite), + Throws.TypeOf()); + } + + [Test] + public void TestRemoveTokenAndCreateUserRoleAddsRole() + { + var result = _inviteService.RemoveTokenAndCreateUserRole(_proj, _user, new EmailInvite()).Result; + Assert.That(result, Is.True); + var userRoles = _userRoleRepo.GetAllUserRoles(_proj.Id).Result; + Assert.That(userRoles, Has.Count.EqualTo(1)); + var userRole = userRoles.First(); + Assert.That(_userRepo.GetUser(_user.Id).Result!.ProjectRoles[_proj.Id], Is.EqualTo(userRole.Id)); + } + + [Test] + public void TestRemoveTokenAndCreateUserRoleRemovesToken() + { + var invite = new EmailInvite(1); + _proj.InviteTokens.Add(invite); + _ = _projRepo.Update(_proj.Id, _proj).Result; + _proj = _projRepo.GetProject(_proj.Id).Result!; + Assert.That(_proj.InviteTokens, Has.Count.EqualTo(1)); + + var result = _inviteService.RemoveTokenAndCreateUserRole(_proj, _user, invite).Result; + Assert.That(result, Is.True); + _proj = _projRepo.GetProject(_proj.Id).Result!; + Assert.That(_proj.InviteTokens, Is.Empty); + } + } +} From b50bb015859e03e8f031e0c43d58693ddb2ca30b Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 13:28:52 -0400 Subject: [PATCH 04/11] When removing from graylist, don't remove subsets (#2670) --- Backend.Tests/Services/MergeServiceTests.cs | 54 ++++++++++++--------- Backend/Interfaces/IMergeService.cs | 2 +- Backend/Services/MergeService.cs | 15 +++--- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/Backend.Tests/Services/MergeServiceTests.cs b/Backend.Tests/Services/MergeServiceTests.cs index 2f0580975c..c5373b552f 100644 --- a/Backend.Tests/Services/MergeServiceTests.cs +++ b/Backend.Tests/Services/MergeServiceTests.cs @@ -199,8 +199,6 @@ public void UndoMergeMultiChildTest() [Test] public void AddMergeToBlacklistTest() { - _ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result; - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List { "1", "2" }; // Adding to blacklist should clear from graylist @@ -219,7 +217,6 @@ public void AddMergeToBlacklistTest() [Test] public void AddMergeToBlacklistErrorTest() { - _ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result; var wordIds0 = new List(); var wordIds1 = new List { "1" }; Assert.That( @@ -231,7 +228,6 @@ public void AddMergeToBlacklistErrorTest() [Test] public void IsInMergeBlacklistTest() { - _ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List { "1", "2", "3" }; var subWordIds = new List { "3", "2" }; @@ -243,7 +239,6 @@ public void IsInMergeBlacklistTest() [Test] public void IsInMergeBlacklistErrorTest() { - _ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result; var wordIds0 = new List(); var wordIds1 = new List { "1" }; Assert.That( @@ -298,7 +293,6 @@ public void UpdateMergeBlacklistTest() [Test] public void AddMergeToGraylistTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List { "1", "2" }; _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result; var graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result; @@ -307,10 +301,24 @@ public void AddMergeToGraylistTest() Assert.That(expectedEntry.ContentEquals(graylist.First()), Is.True); } + [Test] + public void AddMergeToGraylistSupersetTest() + { + var wordIds12 = new List { "1", "2" }; + var wordIds13 = new List { "1", "3" }; + var wordIds123 = new List { "1", "2", "3" }; + + _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds12).Result; + _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds13).Result; + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(2)); + + _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds123).Result; + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1)); + } + [Test] public void AddMergeToGraylistErrorTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List(); var wordIds1 = new List { "1" }; Assert.That( @@ -324,24 +332,28 @@ public void AddMergeToGraylistErrorTest() [Test] public void RemoveFromMergeGraylistTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; - var wordIds12 = new List { "1", "2" }; - var wordIds13 = new List { "1", "3" }; - _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds12).Result; - _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds13).Result; - var graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result; - Assert.That(graylist, Has.Count.EqualTo(2)); - var wordIds123 = new List { "1", "2", "3" }; - var removed = _mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds123).Result; - Assert.That(removed, Has.Count.EqualTo(2)); - graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result; - Assert.That(graylist, Is.Empty); + var wordIds = new List { "1", "2", "3" }; + _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result; + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1)); + Assert.That(_mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds).Result, Is.True); + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(0)); + } + + [Test] + public void RemoveFromMergeGraylistSupersetTest() + { + var wordIds = new List { "1", "2" }; + _ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result; + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1)); + + wordIds.Add("3"); + Assert.That(_mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds).Result, Is.False); + Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1)); } [Test] public void RemoveFromMergeGraylistErrorTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List(); var wordIds1 = new List { "1" }; Assert.That( @@ -355,7 +367,6 @@ public void RemoveFromMergeGraylistErrorTest() [Test] public void IsInMergeGraylistTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds = new List { "1", "2", "3" }; var subWordIds = new List { "3", "2" }; @@ -367,7 +378,6 @@ public void IsInMergeGraylistTest() [Test] public void IsInMergeGraylistErrorTest() { - _ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result; var wordIds0 = new List(); var wordIds1 = new List { "1" }; Assert.That( diff --git a/Backend/Interfaces/IMergeService.cs b/Backend/Interfaces/IMergeService.cs index 71e6562588..40beae11b5 100644 --- a/Backend/Interfaces/IMergeService.cs +++ b/Backend/Interfaces/IMergeService.cs @@ -10,7 +10,7 @@ public interface IMergeService Task UndoMerge(string projectId, MergeUndoIds ids); Task AddToMergeBlacklist(string projectId, string userId, List wordIds); Task AddToMergeGraylist(string projectId, string userId, List wordIds); - Task> RemoveFromMergeGraylist(string projectId, string userId, List wordIds); + Task RemoveFromMergeGraylist(string projectId, string userId, List wordIds); Task IsInMergeBlacklist(string projectId, List wordIds, string? userId = null); Task IsInMergeGraylist(string projectId, List wordIds, string? userId = null); Task UpdateMergeBlacklist(string projectId); diff --git a/Backend/Services/MergeService.cs b/Backend/Services/MergeService.cs index 9e5903dbd0..01be6ef365 100644 --- a/Backend/Services/MergeService.cs +++ b/Backend/Services/MergeService.cs @@ -148,6 +148,7 @@ public async Task AddToMergeBlacklist( public async Task AddToMergeGraylist( string projectId, string userId, List wordIds) { + wordIds = wordIds.Distinct().ToList(); if (wordIds.Count < 2) { throw new InvalidMergeWordSetException("Cannot graylist a list of fewer than 2 wordIds."); @@ -169,27 +170,27 @@ public async Task AddToMergeGraylist( /// Remove a List of wordIds from MergeGraylist of specified . /// Throws when wordIds has count less than 2. - /// List of removed ids. - public async Task> RemoveFromMergeGraylist( + /// Boolean indicating whether anything was removed. + public async Task RemoveFromMergeGraylist( string projectId, string userId, List wordIds) { + wordIds = wordIds.Distinct().ToList(); if (wordIds.Count < 2) { throw new InvalidMergeWordSetException("Cannot have a graylist entry with fewer than 2 wordIds."); } - // Remove all graylist entries fully contained in the input List. + // Remove only the set of wordIds, if in graylist (not any subsets) var graylist = await _mergeGraylistRepo.GetAllSets(projectId, userId); - var removed = new List(); foreach (var entry in graylist) { - if (entry.WordIds.All(wordIds.Contains)) + if (entry.WordIds.All(wordIds.Contains) && wordIds.Count == entry.WordIds.Count) { await _mergeGraylistRepo.Delete(projectId, entry.Id); - removed.Add(entry.Id); + return true; } } - return removed; + return false; } /// Check if List of wordIds is in MergeBlacklist for specified . From d7e36c26bd153ff4c1e0d09c506624bc451e9352 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 15:02:12 -0400 Subject: [PATCH 05/11] [User Guide] Fix typo (#2714) --- .vscode/settings.json | 1 + docs/user_guide/docs/en/admin.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 509e6f0f60..eb7a9947b2 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -49,6 +49,7 @@ "Doulos", "Dups", "endcap", + "Ethnologue", "globaltool", "graylist", "Guids", diff --git a/docs/user_guide/docs/en/admin.md b/docs/user_guide/docs/en/admin.md index de7ed8bcff..862d036d4b 100644 --- a/docs/user_guide/docs/en/admin.md +++ b/docs/user_guide/docs/en/admin.md @@ -4,7 +4,7 @@ Site administrators have one more option in the User Menu: "Site Settings". ![User Menu - Admin](images/userMenuAdmin.png){ .center } -## Project Managements +## Project Management Administrators can export or archive/restore any project. Archiving a project makes it invisible and inaccessible to all users, even the project creator, but any admin can restore the project. There is at present no way to permanently delete From 4b4eca7707c7ceb75d43dc8deed33029c2e509b7 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 15:22:25 -0400 Subject: [PATCH 06/11] Prevent Harvesters from accessing Data Cleanup (#2673) --- src/components/AppBar/NavigationButtons.tsx | 36 ++++++-- .../AppBar/tests/NavigationButtons.test.tsx | 89 ++++++++++++++++--- .../ProjectScreen/ChooseProject.tsx | 2 +- 3 files changed, 105 insertions(+), 22 deletions(-) diff --git a/src/components/AppBar/NavigationButtons.tsx b/src/components/AppBar/NavigationButtons.tsx index 5017200c9a..6f9a25d9ed 100644 --- a/src/components/AppBar/NavigationButtons.tsx +++ b/src/components/AppBar/NavigationButtons.tsx @@ -1,15 +1,19 @@ import { PlaylistAdd, Rule } from "@mui/icons-material"; import { Button, Hidden, Tooltip, Typography } from "@mui/material"; -import { ReactElement } from "react"; +import { ReactElement, useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import { useNavigate } from "react-router-dom"; +import { Permission } from "api/models"; +import { getCurrentPermissions } from "backend"; import { TabProps, appBarHeight, buttonMinHeight, tabColor, } from "components/AppBar/AppBarTypes"; +import { StoreState } from "types"; +import { useAppSelector } from "types/hooks"; import { Path } from "types/path"; import { useWindowSize } from "utilities/useWindowSize"; @@ -20,6 +24,20 @@ const navButtonMaxWidthProportion = 0.2; /** Buttons for navigating to Data Entry and Data Cleanup */ export default function NavigationButtons(props: TabProps): ReactElement { + const projectId = useAppSelector( + (state: StoreState) => state.currentProjectState.project.id + ); + const [hasGoalPermission, setHasGoalPermission] = useState(false); + + useEffect(() => { + getCurrentPermissions().then((perms) => { + setHasGoalPermission( + perms.includes(Permission.CharacterInventory) || + perms.includes(Permission.MergeAndReviewEntries) + ); + }); + }, [projectId]); + return ( <> - } - targetPath={Path.Goals} - textId="appBar.dataCleanup" - /> + {hasGoalPermission && ( + } + targetPath={Path.Goals} + textId="appBar.dataCleanup" + /> + )} ); } diff --git a/src/components/AppBar/tests/NavigationButtons.test.tsx b/src/components/AppBar/tests/NavigationButtons.test.tsx index 285205752e..2136106f80 100644 --- a/src/components/AppBar/tests/NavigationButtons.test.tsx +++ b/src/components/AppBar/tests/NavigationButtons.test.tsx @@ -1,7 +1,10 @@ +import { Provider } from "react-redux"; import renderer, { ReactTestInstance } from "react-test-renderer"; +import configureMockStore from "redux-mock-store"; import "tests/reactI18nextMock"; +import { Permission } from "api"; import NavigationButtons, { dataCleanupButtonId, dataEntryButtonId, @@ -13,38 +16,98 @@ jest.mock("react-router-dom", () => ({ useNavigate: jest.fn(), })); +jest.mock("backend", () => ({ + getCurrentPermissions: () => mockGetCurrentPermissions(), +})); + +const mockGetCurrentPermissions = jest.fn(); +const mockStore = configureMockStore()({ + currentProjectState: { project: { id: "" } }, +}); + let testRenderer: renderer.ReactTestRenderer; -let entryButton: ReactTestInstance | undefined; -let cleanButton: ReactTestInstance | undefined; +let entryButton: ReactTestInstance; +let cleanButton: ReactTestInstance; -const renderNavButtons = (path: Path): void => { - renderer.act(() => { - testRenderer = renderer.create(); +const renderNavButtons = async ( + path: Path, + permission = Permission.MergeAndReviewEntries +): Promise => { + mockGetCurrentPermissions.mockResolvedValue([permission]); + await renderer.act(async () => { + testRenderer = renderer.create( + + + + ); }); +}; + +const renderNavButtonsWithPath = async (path: Path): Promise => { + await renderNavButtons(path, Permission.MergeAndReviewEntries); entryButton = testRenderer.root.findByProps({ id: dataEntryButtonId }); cleanButton = testRenderer.root.findByProps({ id: dataCleanupButtonId }); }; +const renderNavButtonsWithPermission = async ( + perm: Permission +): Promise => { + await renderNavButtons(Path.DataEntry, perm); + entryButton = testRenderer.root.findByProps({ id: dataEntryButtonId }); + const cleanupButtons = testRenderer.root.findAllByProps({ + id: dataCleanupButtonId, + }); + if (cleanupButtons.length) { + cleanButton = cleanupButtons[0]; + } +}; + +beforeEach(() => { + jest.resetAllMocks(); +}); + describe("NavigationButtons", () => { - it("highlights the correct tab", () => { - renderNavButtons(Path.Statistics); - expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); - expect(cleanButton?.props.style.background).toEqual(themeColors.lightShade); + it("only shows the data cleanup tab for the correct permissions", async () => { + for (const perm of Object.values(Permission)) { + await renderNavButtonsWithPermission(perm); + if ( + perm === Permission.CharacterInventory || + perm === Permission.MergeAndReviewEntries + ) { + expect(cleanButton).toBeTruthy; + } else { + expect(cleanButton).toBeUndefined; + } + } + }); - renderNavButtons(Path.DataEntry); + it("highlights the correct tab", async () => { + await renderNavButtonsWithPath(Path.DataEntry); expect(entryButton?.props.style.background).toEqual(themeColors.darkShade); expect(cleanButton?.props.style.background).toEqual(themeColors.lightShade); - renderNavButtons(Path.Goals); + await renderNavButtonsWithPath(Path.Goals); expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); expect(cleanButton?.props.style.background).toEqual(themeColors.darkShade); - renderNavButtons(Path.GoalCurrent); + await renderNavButtonsWithPath(Path.GoalCurrent); expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); expect(cleanButton?.props.style.background).toEqual(themeColors.darkShade); - renderNavButtons(Path.GoalNext); + await renderNavButtonsWithPath(Path.GoalNext); expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); expect(cleanButton?.props.style.background).toEqual(themeColors.darkShade); + + await renderNavButtonsWithPath(Path.ProjSettings); + expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); + expect(cleanButton?.props.style.background).toEqual(themeColors.lightShade); + + await renderNavButtonsWithPath(Path.Statistics); + expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); + expect(cleanButton?.props.style.background).toEqual(themeColors.lightShade); + + await renderNavButtonsWithPath(Path.UserSettings); + expect(entryButton?.props.style.background).toEqual(themeColors.lightShade); + expect(cleanButton?.props.style.background).toEqual(themeColors.lightShade); }); }); diff --git a/src/components/ProjectScreen/ChooseProject.tsx b/src/components/ProjectScreen/ChooseProject.tsx index 8d7204017e..8abb790a58 100644 --- a/src/components/ProjectScreen/ChooseProject.tsx +++ b/src/components/ProjectScreen/ChooseProject.tsx @@ -36,7 +36,7 @@ export default function ChooseProject(): ReactElement { const selectProject = (project: Project): void => { dispatch(setNewCurrentProject(project)); - navigate(Path.Goals); + navigate(Path.DataEntry); }; return ( From 39518a94e107bb0b65270b5296d9d9b7608a78d0 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 15:58:44 -0400 Subject: [PATCH 07/11] [ReviewEntriesTable] Reduce max rows-per-page to 200 (#2680) --- .../ReviewEntries/ReviewEntriesComponent/ReviewEntriesTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/goals/ReviewEntries/ReviewEntriesComponent/ReviewEntriesTable.tsx b/src/goals/ReviewEntries/ReviewEntriesComponent/ReviewEntriesTable.tsx index cce194e344..a50b197069 100644 --- a/src/goals/ReviewEntries/ReviewEntriesComponent/ReviewEntriesTable.tsx +++ b/src/goals/ReviewEntries/ReviewEntriesComponent/ReviewEntriesTable.tsx @@ -39,7 +39,7 @@ function getPageState(wordCount: number): PageState { } // Constants -const ROWS_PER_PAGE = [10, 50, 250]; +const ROWS_PER_PAGE = [10, 50, 200]; const tableRef: React.RefObject = React.createRef(); export default function ReviewEntriesTable( From 7037c5c17c709afc139ac44230dbe1e5c67011bc Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Tue, 10 Oct 2023 16:27:24 -0400 Subject: [PATCH 08/11] Adjust DomainTileButton dimensions (#2683) --- .../TreeDepiction/DomainTileButton.tsx | 1 + .../TreeDepiction/TreeDepictionTypes.ts | 4 +- .../DomainTileButton.test.tsx.snap | 1 + .../tests/__snapshots__/index.test.tsx.snap | 980 ++++++++++++++++-- 4 files changed, 919 insertions(+), 67 deletions(-) diff --git a/src/components/TreeView/TreeDepiction/DomainTileButton.tsx b/src/components/TreeView/TreeDepiction/DomainTileButton.tsx index 02b115a53f..0555f4c40b 100644 --- a/src/components/TreeView/TreeDepiction/DomainTileButton.tsx +++ b/src/components/TreeView/TreeDepiction/DomainTileButton.tsx @@ -104,6 +104,7 @@ export default function DomainTileButton( width: "95%", height: "95%", margin: "2.5%", + padding: "5px", }} variant={"outlined"} > diff --git a/src/components/TreeView/TreeDepiction/TreeDepictionTypes.ts b/src/components/TreeView/TreeDepiction/TreeDepictionTypes.ts index d76ed4795a..d71b45874c 100644 --- a/src/components/TreeView/TreeDepiction/TreeDepictionTypes.ts +++ b/src/components/TreeView/TreeDepiction/TreeDepictionTypes.ts @@ -1,8 +1,8 @@ import { SemanticDomain, SemanticDomainTreeNode } from "api/models"; const MAX_COL_WIDTH = 50; // Max gap. -const MIN_COL_WIDTH = 30; // Multiply this by RATIO_TILE_TO_GAP for min tile width. -export const RATIO_TILE_TO_GAP = 3; // Must be odd. +const MIN_COL_WIDTH = 12; // Multiply this by RATIO_TILE_TO_GAP for min tile width. +export const RATIO_TILE_TO_GAP = 7; // Must be odd. export function getNumCols(numChildren: number) { return numChildren * (RATIO_TILE_TO_GAP + 1) - 1; diff --git a/src/components/TreeView/TreeDepiction/tests/__snapshots__/DomainTileButton.test.tsx.snap b/src/components/TreeView/TreeDepiction/tests/__snapshots__/DomainTileButton.test.tsx.snap index 218386f8b3..971cb6d2ee 100644 --- a/src/components/TreeView/TreeDepiction/tests/__snapshots__/DomainTileButton.test.tsx.snap +++ b/src/components/TreeView/TreeDepiction/tests/__snapshots__/DomainTileButton.test.tsx.snap @@ -24,6 +24,7 @@ exports[`DomainTileButton renders tile and matches the latest snapshot 1`] = ` "height": "95%", "left": 0, "margin": "2.5%", + "padding": "5px", "width": "95%", } } diff --git a/src/components/TreeView/TreeDepiction/tests/__snapshots__/index.test.tsx.snap b/src/components/TreeView/TreeDepiction/tests/__snapshots__/index.test.tsx.snap index 365a916bac..0648b0c075 100644 --- a/src/components/TreeView/TreeDepiction/tests/__snapshots__/index.test.tsx.snap +++ b/src/components/TreeView/TreeDepiction/tests/__snapshots__/index.test.tsx.snap @@ -49,6 +49,7 @@ Array [ "height": "95%", "left": 0, "margin": "2.5%", + "padding": "5px", "width": "95%", } } @@ -109,7 +110,7 @@ Array [ className="MuiImageListItem-img" height={60} src="parent.svg" - width={51} + width={45} /> @@ -221,11 +222,35 @@ Array [ style={ Object { "gap": 0, - "gridTemplateColumns": "repeat(11, 1fr)", - "width": 550, + "gridTemplateColumns": "repeat(23, 1fr)", + "width": 1012, } } > +
  • +
  • +
  • +
  • + span.svg +
  • +
  • + span.svg +
  • +
  • + span.svg +
  • +
  • + span.svg
  • +
  • +
  • + span.svg +
  • +
  • + span.svg +
  • +
  • + span.svg +
  • +
  • + span.svg
  • +
  • +
  • +
  • +
  • -