diff --git a/Backend.Tests/Controllers/WordControllerTests.cs b/Backend.Tests/Controllers/WordControllerTests.cs index 0c99ec1b8c..9bd23f6f44 100644 --- a/Backend.Tests/Controllers/WordControllerTests.cs +++ b/Backend.Tests/Controllers/WordControllerTests.cs @@ -49,6 +49,34 @@ public void Setup() _projId = _projRepo.Create(new Project { Name = "WordControllerTests" }).Result!.Id; } + [Test] + public async Task TestAreInFrontier() + { + var wordNotInFrontier = await _wordRepo.Add(Util.RandomWord(_projId)); + var emptyResult = await _wordController.AreInFrontier(_projId, new() { wordNotInFrontier.Id, "non-id" }); + Assert.That(((ObjectResult)emptyResult).Value, Is.Empty); + + var wordInFrontier = await _wordRepo.AddFrontier(Util.RandomWord(_projId)); + var nonemptyResult = await _wordController.AreInFrontier(_projId, new() { wordInFrontier.Id, "non-id" }); + Assert.That(((OkObjectResult)nonemptyResult).Value, Is.EqualTo(new List { wordInFrontier.Id })); + } + + [Test] + public async Task TestAreInFrontierNoPermission() + { + var wordInFrontier = await _wordRepo.AddFrontier(Util.RandomWord(_projId)); + _wordController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext(); + var result = await _wordController.AreInFrontier(_projId, new() { wordInFrontier.Id }); + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public async Task TestAreInFrontierMissingProject() + { + var result = await _wordController.AreInFrontier(MissingId, new()); + Assert.That(result, Is.InstanceOf()); + } + [Test] public async Task TestDeleteFrontierWord() { @@ -266,6 +294,49 @@ public async Task TestGetDuplicateIdMissingProject() Assert.That(result, Is.InstanceOf()); } + [Test] + public async Task TestRevertWords() + { + var frontierWord0 = await _wordRepo.Create(Util.RandomWord(_projId)); + var frontierWord1 = await _wordRepo.AddFrontier(Util.RandomWord(_projId)); + var nonFrontierWord0 = await _wordRepo.Add(Util.RandomWord(_projId)); + var nonFrontierWord1 = await _wordRepo.Add(Util.RandomWord(_projId)); + var nonFrontierWord2 = await _wordRepo.Add(Util.RandomWord(_projId)); + + var result = await _wordController.RevertWords(_projId, new() + { + ["non-id"] = frontierWord1.Id, // Cannot revert with key not a word + [nonFrontierWord1.Id] = nonFrontierWord2.Id, // Cannot revert with value not in frontier + [nonFrontierWord0.Id] = frontierWord0.Id, // Can revert + }); + var reverted = (Dictionary)((OkObjectResult)result).Value!; + Assert.That(reverted, Has.Count.EqualTo(1)); + var frontierIds = (await _wordRepo.GetFrontier(_projId)).Select(w => w.Id).ToList(); + Assert.That(frontierIds, Has.Count.EqualTo(2)); + Assert.That(frontierIds, Does.Contain(frontierWord1.Id)); + Assert.That(frontierIds, Does.Contain(reverted[frontierWord0.Id])); + } + + [Test] + public async Task TestRevertWordsNoPermission() + { + _wordController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext(); + + var oldWord = await _wordRepo.Add(Util.RandomWord(_projId)); + var newWord = await _wordRepo.Create(Util.RandomWord(_projId)); + var result = await _wordController.RevertWords(_projId, new() { [oldWord.Id] = newWord.Id }); + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public async Task TestRevertWordsMissingProject() + { + var oldWord = await _wordRepo.Add(Util.RandomWord(_projId)); + var newWord = await _wordRepo.Create(Util.RandomWord(_projId)); + var result = await _wordController.RevertWords(MissingId, new() { [oldWord.Id] = newWord.Id }); + Assert.That(result, Is.InstanceOf()); + } + [Test] public async Task TestUpdateDuplicate() { diff --git a/Backend/Controllers/WordController.cs b/Backend/Controllers/WordController.cs index 46f9862baf..dc2219c95d 100644 --- a/Backend/Controllers/WordController.cs +++ b/Backend/Controllers/WordController.cs @@ -141,6 +141,31 @@ public async Task IsInFrontier(string projectId, string wordId) return Ok(await _wordRepo.IsInFrontier(projectId, wordId)); } + /// Checks if Frontier has words in specified . + [HttpPost("areinfrontier", Name = "AreInFrontier")] + [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List))] + public async Task AreInFrontier(string projectId, [FromBody, BindRequired] List wordIds) + { + if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId)) + { + return Forbid(); + } + if ((await _projRepo.GetProject(projectId)) is null) + { + return NotFound(projectId); + } + + var idsInFrontier = new List(); + foreach (var id in wordIds) + { + if (await _wordRepo.IsInFrontier(projectId, id)) + { + idsInFrontier.Add(id); + } + } + return Ok(idsInFrontier); + } + /// /// Checks if a is a duplicate--i.e., are its primary text fields /// (Vernacular, Gloss text, Definition text) contained in a frontier entry? @@ -247,5 +272,36 @@ public async Task UpdateWord( await _wordService.Update(projectId, userId, wordId, word); return Ok(word.Id); } + + /// Revert words from an dictionary of word ids (key: to revert to; value: from frontier). + /// Id dictionary of all words successfully updated (key: was in frontier; value: new id). + [HttpPost("revertwords", Name = "RevertWords")] + [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(Dictionary))] + public async Task RevertWords( + string projectId, [FromBody, BindRequired] Dictionary wordIds) + { + if (!await _permissionService.HasProjectPermission(HttpContext, Permission.WordEntry, projectId)) + { + return Forbid(); + } + if ((await _projRepo.GetProject(projectId)) is null) + { + return NotFound(projectId); + } + + var updates = new Dictionary(); + var userId = _permissionService.GetUserId(HttpContext); + 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)) + { + await _wordService.Update(projectId, userId, idToRevert, word); + updates[idToRevert] = word.Id; + } + } + return Ok(updates); + } } } diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index 96522e60b7..b235a6f1c6 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -335,7 +335,7 @@ "undo": { "undo": "Undo Edit", "undoDialog": "Undo this edit?", - "undoDisabled": "Undo Unavailable" + "undoDisabled": "Undo unavailable" } }, "charInventory": { @@ -367,8 +367,17 @@ "no": "no" }, "changes": { - "noChanges": "No changes made in this goal.", - "more": "more" + "noCharChanges": "No changes made to character inventory.", + "more": "more", + "noWordChangesFindReplace": "No words changed by find-and-replace.", + "wordChanges": "Words changed:", + "wordChangesWithStrings": "Words changed ({{ val1 }} → {{ val2 }}):" + }, + "undo": { + "undo": "Undo find-and-replace", + "undoDialog": "Undo words changes by find-and-replace?", + "undoDisabled": "Undo unavailable", + "undoWords": "Undo available for {{ val1 }} of the {{ val2 }} changed words." } }, "mergeDups": { @@ -425,7 +434,7 @@ "undo": { "undo": "Undo Merge", "undoDialog": "Undo this merge?", - "undoDisabled": "Undo Unavailable" + "undoDisabled": "Undo unavailable" } }, "flags": { diff --git a/src/api/api/word-api.ts b/src/api/api/word-api.ts index 196a1f3ec0..e9e3423c92 100644 --- a/src/api/api/word-api.ts +++ b/src/api/api/word-api.ts @@ -46,6 +46,63 @@ export const WordApiAxiosParamCreator = function ( configuration?: Configuration ) { return { + /** + * + * @param {string} projectId + * @param {Array} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + areInFrontier: async ( + projectId: string, + requestBody: Array, + options: any = {} + ): Promise => { + // verify required parameter 'projectId' is not null or undefined + assertParamExists("areInFrontier", "projectId", projectId); + // verify required parameter 'requestBody' is not null or undefined + assertParamExists("areInFrontier", "requestBody", requestBody); + const localVarPath = + `/v1/projects/{projectId}/words/areinfrontier`.replace( + `{${"projectId"}}`, + encodeURIComponent(String(projectId)) + ); + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { + method: "POST", + ...baseOptions, + ...options, + }; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + localVarHeaderParameter["Content-Type"] = "application/json"; + + setSearchParams(localVarUrlObj, localVarQueryParameter, options.query); + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + }; + localVarRequestOptions.data = serializeDataIfNeeded( + requestBody, + localVarRequestOptions, + configuration + ); + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, /** * * @param {string} projectId @@ -440,6 +497,62 @@ export const WordApiAxiosParamCreator = function ( options: localVarRequestOptions, }; }, + /** + * + * @param {string} projectId + * @param {{ [key: string]: string; }} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + revertWords: async ( + projectId: string, + requestBody: { [key: string]: string }, + options: any = {} + ): Promise => { + // verify required parameter 'projectId' is not null or undefined + assertParamExists("revertWords", "projectId", projectId); + // verify required parameter 'requestBody' is not null or undefined + assertParamExists("revertWords", "requestBody", requestBody); + const localVarPath = `/v1/projects/{projectId}/words/revertwords`.replace( + `{${"projectId"}}`, + encodeURIComponent(String(projectId)) + ); + // use dummy base URL string because the URL constructor only accepts absolute URLs. + const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); + let baseOptions; + if (configuration) { + baseOptions = configuration.baseOptions; + } + + const localVarRequestOptions = { + method: "POST", + ...baseOptions, + ...options, + }; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + localVarHeaderParameter["Content-Type"] = "application/json"; + + setSearchParams(localVarUrlObj, localVarQueryParameter, options.query); + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + }; + localVarRequestOptions.data = serializeDataIfNeeded( + requestBody, + localVarRequestOptions, + configuration + ); + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, /** * * @param {string} projectId @@ -568,6 +681,32 @@ export const WordApiAxiosParamCreator = function ( export const WordApiFp = function (configuration?: Configuration) { const localVarAxiosParamCreator = WordApiAxiosParamCreator(configuration); return { + /** + * + * @param {string} projectId + * @param {Array} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async areInFrontier( + projectId: string, + requestBody: Array, + options?: any + ): Promise< + (axios?: AxiosInstance, basePath?: string) => AxiosPromise> + > { + const localVarAxiosArgs = await localVarAxiosParamCreator.areInFrontier( + projectId, + requestBody, + options + ); + return createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration + ); + }, /** * * @param {string} projectId @@ -767,6 +906,35 @@ export const WordApiFp = function (configuration?: Configuration) { configuration ); }, + /** + * + * @param {string} projectId + * @param {{ [key: string]: string; }} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async revertWords( + projectId: string, + requestBody: { [key: string]: string }, + options?: any + ): Promise< + ( + axios?: AxiosInstance, + basePath?: string + ) => AxiosPromise<{ [key: string]: string }> + > { + const localVarAxiosArgs = await localVarAxiosParamCreator.revertWords( + projectId, + requestBody, + options + ); + return createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration + ); + }, /** * * @param {string} projectId @@ -839,6 +1007,22 @@ export const WordApiFactory = function ( ) { const localVarFp = WordApiFp(configuration); return { + /** + * + * @param {string} projectId + * @param {Array} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + areInFrontier( + projectId: string, + requestBody: Array, + options?: any + ): AxiosPromise> { + return localVarFp + .areInFrontier(projectId, requestBody, options) + .then((request) => request(axios, basePath)); + }, /** * * @param {string} projectId @@ -961,6 +1145,22 @@ export const WordApiFactory = function ( .isInFrontier(projectId, wordId, options) .then((request) => request(axios, basePath)); }, + /** + * + * @param {string} projectId + * @param {{ [key: string]: string; }} requestBody + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + revertWords( + projectId: string, + requestBody: { [key: string]: string }, + options?: any + ): AxiosPromise<{ [key: string]: string }> { + return localVarFp + .revertWords(projectId, requestBody, options) + .then((request) => request(axios, basePath)); + }, /** * * @param {string} projectId @@ -1000,6 +1200,27 @@ export const WordApiFactory = function ( }; }; +/** + * Request parameters for areInFrontier operation in WordApi. + * @export + * @interface WordApiAreInFrontierRequest + */ +export interface WordApiAreInFrontierRequest { + /** + * + * @type {string} + * @memberof WordApiAreInFrontier + */ + readonly projectId: string; + + /** + * + * @type {Array} + * @memberof WordApiAreInFrontier + */ + readonly requestBody: Array; +} + /** * Request parameters for createWord operation in WordApi. * @export @@ -1147,6 +1368,27 @@ export interface WordApiIsInFrontierRequest { readonly wordId: string; } +/** + * Request parameters for revertWords operation in WordApi. + * @export + * @interface WordApiRevertWordsRequest + */ +export interface WordApiRevertWordsRequest { + /** + * + * @type {string} + * @memberof WordApiRevertWords + */ + readonly projectId: string; + + /** + * + * @type {{ [key: string]: string; }} + * @memberof WordApiRevertWords + */ + readonly requestBody: { [key: string]: string }; +} + /** * Request parameters for updateDuplicate operation in WordApi. * @export @@ -1210,6 +1452,26 @@ export interface WordApiUpdateWordRequest { * @extends {BaseAPI} */ export class WordApi extends BaseAPI { + /** + * + * @param {WordApiAreInFrontierRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof WordApi + */ + public areInFrontier( + requestParameters: WordApiAreInFrontierRequest, + options?: any + ) { + return WordApiFp(this.configuration) + .areInFrontier( + requestParameters.projectId, + requestParameters.requestBody, + options + ) + .then((request) => request(this.axios, this.basePath)); + } + /** * * @param {WordApiCreateWordRequest} requestParameters Request parameters. @@ -1347,6 +1609,26 @@ export class WordApi extends BaseAPI { .then((request) => request(this.axios, this.basePath)); } + /** + * + * @param {WordApiRevertWordsRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof WordApi + */ + public revertWords( + requestParameters: WordApiRevertWordsRequest, + options?: any + ) { + return WordApiFp(this.configuration) + .revertWords( + requestParameters.projectId, + requestParameters.requestBody, + options + ) + .then((request) => request(this.axios, this.basePath)); + } + /** * * @param {WordApiUpdateDuplicateRequest} requestParameters Request parameters. diff --git a/src/backend/index.ts b/src/backend/index.ts index 339fed3cc3..6837bd945d 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -788,6 +788,15 @@ export async function removeUserRole( /* WordController.cs */ +export async function areInFrontier( + wordIds: string[], + projectId?: string +): Promise { + projectId ||= LocalStorage.getProjectId(); + const params = { projectId, requestBody: wordIds }; + return (await wordApi.areInFrontier(params, defaultOptions())).data; +} + export async function createWord(word: Word): Promise { const params = { projectId: LocalStorage.getProjectId(), word }; word.id = (await wordApi.createWord(params, defaultOptions())).data; @@ -823,11 +832,20 @@ export async function isInFrontier( wordId: string, projectId?: string ): Promise { - projectId = projectId || LocalStorage.getProjectId(); + projectId ||= LocalStorage.getProjectId(); const params = { projectId, wordId }; return (await wordApi.isInFrontier(params, defaultOptions())).data; } +/** Revert word updates given in dictionary of word ids: + * - key: id of word to revert to; + * - value: id of word in frontier. */ +export async function revertWords(ids: Hash): Promise> { + const params = { projectId: LocalStorage.getProjectId(), requestBody: ids }; + const resp = await wordApi.revertWords(params, defaultOptions()); + return resp.data; +} + export async function updateDuplicate( dupId: string, word: Word diff --git a/src/components/Dialogs/CancelConfirmDialog.tsx b/src/components/Dialogs/CancelConfirmDialog.tsx index 7b7fea21b4..22d93aa9b2 100644 --- a/src/components/Dialogs/CancelConfirmDialog.tsx +++ b/src/components/Dialogs/CancelConfirmDialog.tsx @@ -32,6 +32,7 @@ export default function CancelConfirmDialog( const onConfirm = async (): Promise => { setLoading(true); await props.handleConfirm(); + setLoading(false); }; return ( diff --git a/src/components/GoalTimeline/tests/GoalRedux.test.tsx b/src/components/GoalTimeline/tests/GoalRedux.test.tsx index e825ad571a..b53e3b0ef1 100644 --- a/src/components/GoalTimeline/tests/GoalRedux.test.tsx +++ b/src/components/GoalTimeline/tests/GoalRedux.test.tsx @@ -5,10 +5,9 @@ import { Edit, MergeUndoIds, Permission, User, UserEdit } from "api/models"; import * as LocalStorage from "backend/localStorage"; import GoalTimeline from "components/GoalTimeline"; import { - CharacterChange, - CharacterStatus, CharInvChanges, CharInvData, + CharacterStatus, CreateCharInv, } from "goals/CharacterInventory/CharacterInventoryTypes"; import { @@ -83,9 +82,10 @@ const mockCompletedMerge: MergeUndoIds = { parentIds: ["1", "2"], childIds: ["3", "4"], }; -const mockCharInvChanges: CharacterChange[] = [ - ["'", CharacterStatus.Undecided, CharacterStatus.Accepted], -]; +const mockCharInvChanges: CharInvChanges = { + charChanges: [["'", CharacterStatus.Undecided, CharacterStatus.Accepted]], + wordChanges: [{ find: "Q", replace: "q", words: { ["id-a"]: "id-b" } }], +}; const mockEdit = (): Edit => ({ changes: JSON.stringify(mockCompletedMerge), @@ -296,9 +296,8 @@ describe("asyncUpdateGoal", () => { await store.dispatch(asyncAddGoal(goal)); store.dispatch(addCharInvChangesToGoal(mockCharInvChanges)); }); - const changes = store.getState().goalsState.currentGoal - .changes as CharInvChanges; - expect(changes!.charChanges).toBe(mockCharInvChanges); + const changes = store.getState().goalsState.currentGoal.changes; + expect(changes).toBe(mockCharInvChanges); // dispatch asyncUpdateGoal() await act(async () => { await store.dispatch(asyncUpdateGoal()); diff --git a/src/goals/CharacterInventory/CharInvCompleted.tsx b/src/goals/CharacterInventory/CharInvCompleted.tsx index c9b89df05d..aac80f322b 100644 --- a/src/goals/CharacterInventory/CharInvCompleted.tsx +++ b/src/goals/CharacterInventory/CharInvCompleted.tsx @@ -1,55 +1,131 @@ import { ArrowRightAlt } from "@mui/icons-material"; -import { Typography } from "@mui/material"; -import { ReactElement } from "react"; +import { Divider, Grid, Typography } from "@mui/material"; +import { type ReactElement, useCallback, useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; -import { useSelector } from "react-redux"; +import { type Word } from "api/models"; +import { areInFrontier, getWord, revertWords } from "backend"; +import UndoButton from "components/Buttons/UndoButton"; +import WordCard from "components/WordCard"; import CharacterStatusText from "goals/CharacterInventory/CharInv/CharacterList/CharacterStatusText"; import { - CharacterChange, - CharInvChanges, + type CharInvChanges, + type CharacterChange, + type FindAndReplaceChange, + defaultCharInvChanges, } from "goals/CharacterInventory/CharacterInventoryTypes"; -import { StoreState } from "types"; +import { type StoreState } from "types"; +import { useAppSelector } from "types/hooks"; +export enum CharInvCompletedId { + TypographyNoCharChanges = "no-char-changes-typography", + TypographyNoWordChanges = "no-word-changes-typography", + TypographyWordChanges = "word-changes-typography", + TypographyWordsUndo = "words-undo-typography", +} + +/** Component to display the full details of changes made during one session of the + * Create Character Inventory goal. This includes: + * - Changes to inventory status of a character (accepted vs. rejected vs. undecided); + * - Words changed with the find-and-replace tool. */ export default function CharInvCompleted(): ReactElement { - const changes = useSelector( - (state: StoreState) => - state.goalsState.currentGoal.changes as CharInvChanges + const stateChanges = useAppSelector( + (state: StoreState) => state.goalsState.currentGoal.changes ); const { t } = useTranslation(); + const changes: CharInvChanges = { ...defaultCharInvChanges, ...stateChanges }; + return ( <> {t("charInventory.title")} - {changes.charChanges?.length ? ( - changes.charChanges.map((c) => ) + {changes.charChanges.length ? ( + changes.charChanges.map((c) => ) + ) : ( + + {t("charInventory.changes.noCharChanges")} + + )} + {changes.wordChanges.length ? ( + changes.wordChanges.map((wc, i) => ( + + )) ) : ( - {t("charInventory.changes.noChanges")} + <> + + + {t("charInventory.changes.noWordChangesFindReplace")} + + )} ); } +/** Typography summarizing find-and-replace word changes for goal history + * (or undefined if no words changed). */ +function WordChangesTypography( + wordChanges: FindAndReplaceChange[] +): ReactElement | undefined { + const { t } = useTranslation(); + + const changes = wordChanges.filter((wc) => Object.keys(wc.words).length); + if (!changes.length) { + return; + } + const wordCount = changes.flatMap((wc) => Object.keys(wc.words)).length; + const description = + changes.length === 1 + ? t("charInventory.changes.wordChangesWithStrings", { + val1: changes[0].find, + val2: changes[0].replace, + }) + : t("charInventory.changes.wordChanges"); + + return ( + + {`${description} ${wordCount}`} + + ); +} + +/** Component for the goal history timeline, to display a summary of changes made during + * one session of the Create Character Inventory goal. This includes: + * - Changes to inventory status of a character (up to 3); + * - Number of words changed with the find-and-replace tool (only if more than 0). */ export function CharInvChangesGoalList(changes: CharInvChanges): ReactElement { const { t } = useTranslation(); const changeLimit = 3; + const wordChangesTypography = WordChangesTypography( + changes.wordChanges ?? [] + ); + if (!changes.charChanges?.length) { - return {t("charInventory.changes.noChanges")}; + return ( + wordChangesTypography ?? ( + <> + + {t("charInventory.changes.noCharChanges")} + + + ) + ); } if (changes.charChanges.length > changeLimit) { return ( <> {changes.charChanges.slice(0, changeLimit - 1).map((c) => ( - + ))} - {`+${changes.charChanges.length - 3} `} + {`+${changes.charChanges.length - (changeLimit - 1)} `} {t("charInventory.changes.more")} + {wordChangesTypography} ); } @@ -57,13 +133,15 @@ export function CharInvChangesGoalList(changes: CharInvChanges): ReactElement { <> {changes.charChanges.map((c) => ( - + ))} + {wordChangesTypography} ); } -function CharInvChange(props: { change: CharacterChange }): ReactElement { +/** Component to display in one line the inventory status change of a character. */ +export function CharChange(props: { change: CharacterChange }): ReactElement { return ( <> {`${props.change[0]}: `} @@ -74,3 +152,89 @@ function CharInvChange(props: { change: CharacterChange }): ReactElement { ); } + +/** Component to display words (max 5) changed by a single find-and-replace, with a + * button to undo (if at least one word is still in the project frontier). */ +function WordChanges(props: { + wordChanges: FindAndReplaceChange; +}): ReactElement { + const [inFrontier, setInFrontier] = useState([]); + const [entries] = useState(Object.entries(props.wordChanges.words)); + const { t } = useTranslation(); + const wordLimit = 5; + + const undoWordsTypography = inFrontier.length ? ( + + {t("charInventory.undo.undoWords", { + val1: inFrontier.length, + val2: entries.length, + })} + + ) : null; + + /** Fetches which of the new words are still in the project frontier; + * returns true if there are any. */ + const isUndoAllowed = useCallback(async (): Promise => { + const ids = await areInFrontier(Object.values(props.wordChanges.words)); + setInFrontier(ids); + return ids.length > 0; + }, [props.wordChanges.words]); + + /** Reverts all changes for which the new word is still in the project frontier. */ + const undo = async (): Promise => { + await revertWords(props.wordChanges.words); + }; + + return ( + <> + + + {t("charInventory.changes.wordChangesWithStrings", { + val1: props.wordChanges.find, + val2: props.wordChanges.replace, + })} + + + {entries.slice(0, wordLimit).map(([oldId, newId]) => ( + + ))} + {entries.length > wordLimit ? ( + + + {`+${entries.length - wordLimit} ${t( + "charInventory.changes.more" + )}`} + + + ) : null} + + {undoWordsTypography} + + + ); +} + +/** Component to show a word update that only involved a change in vernacular form. */ +function WordChangeGrid(props: { newId: string; oldId: string }): ReactElement { + const [oldVern, setOldVern] = useState(""); + const [newWord, setNewWord] = useState(); + + useEffect(() => { + getWord(props.oldId).then((w) => setOldVern(w.vernacular)); + getWord(props.newId).then(setNewWord); + }, [props]); + + const vernacular = `${oldVern} → ${newWord?.vernacular}`; + + return ( + + {newWord ? : null} + + ); +} diff --git a/src/goals/CharacterInventory/CharacterInventoryTypes.ts b/src/goals/CharacterInventory/CharacterInventoryTypes.ts index e727ed9a50..0328daee9f 100644 --- a/src/goals/CharacterInventory/CharacterInventoryTypes.ts +++ b/src/goals/CharacterInventory/CharacterInventoryTypes.ts @@ -1,4 +1,5 @@ import { Goal, GoalName, GoalType } from "types/goals"; +import { type Hash } from "types/hash"; export class CreateCharInv extends Goal { constructor( @@ -17,10 +18,22 @@ export enum CharacterStatus { export type CharacterChange = [string, CharacterStatus, CharacterStatus]; +export interface FindAndReplaceChange { + find: string; + replace: string; + words: Hash; +} + export interface CharInvChanges { charChanges: CharacterChange[]; + wordChanges: FindAndReplaceChange[]; } +export const defaultCharInvChanges: CharInvChanges = { + charChanges: [], + wordChanges: [], +}; + export interface CharInvData { inventory: string[][]; } diff --git a/src/goals/CharacterInventory/Redux/CharacterInventoryActions.ts b/src/goals/CharacterInventory/Redux/CharacterInventoryActions.ts index 4aeed54143..eeaa59e4f2 100644 --- a/src/goals/CharacterInventory/Redux/CharacterInventoryActions.ts +++ b/src/goals/CharacterInventory/Redux/CharacterInventoryActions.ts @@ -5,8 +5,11 @@ import { getFrontierWords, updateWord } from "backend"; import router from "browserRouter"; import { asyncUpdateCurrentProject } from "components/Project/ProjectActions"; import { + type CharInvChanges, type CharacterChange, CharacterStatus, + type FindAndReplaceChange, + defaultCharInvChanges, } from "goals/CharacterInventory/CharacterInventoryTypes"; import { addRejectedCharacterAction, @@ -29,6 +32,7 @@ import { } from "goals/Redux/GoalActions"; import { type StoreState } from "types"; import { type StoreStateDispatch } from "types/Redux/actions"; +import { type Hash } from "types/hash"; import { Path } from "types/path"; // Action Creation Functions @@ -104,8 +108,8 @@ export function uploadInventory() { return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { const charInvState = getState().characterInventoryState; const project = getState().currentProjectState.project; - const changes = getChanges(project, charInvState); - if (!changes.length) { + const charChanges = getCharChanges(project, charInvState); + if (!charChanges.length) { exit(); return; } @@ -116,7 +120,8 @@ export function uploadInventory() { validCharacters: charInvState.validCharacters, }) ); - dispatch(addCharInvChangesToGoal(changes)); + const changes = getState().goalsState.currentGoal.changes as CharInvChanges; + dispatch(addCharInvChangesToGoal({ ...changes, charChanges })); await dispatch(asyncUpdateGoal()); exit(); }; @@ -163,29 +168,44 @@ export function loadCharInvData() { }; } +/** Returns a dispatch function to: in both frontend and backend, add to the current + * goal's changes the dictionary of words updated with the find-and-replace tool. */ +function addWordChanges(wordChanges: FindAndReplaceChange) { + return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { + const changes: CharInvChanges = { + ...defaultCharInvChanges, + ...getState().goalsState.currentGoal.changes, + }; + changes.wordChanges = [...changes.wordChanges, wordChanges]; + dispatch(addCharInvChangesToGoal(changes)); + await dispatch(asyncUpdateGoal()); + }; +} + /** Returns a dispatch function to: update every word in the current project's frontier - * that has the given `findValue` in its vernacular form. Then: + * that has the given `find` in its vernacular form. Then: + * - Add those word changes to the current goal's changes; * - Update the in-state `allWords` array; * - Update the in-state character inventory. */ -export function findAndReplace(findValue: string, replaceValue: string) { +export function findAndReplace(find: string, replace: string) { return async (dispatch: StoreStateDispatch) => { const changedWords = (await getFrontierWords()).filter((w) => - w.vernacular.includes(findValue) + w.vernacular.includes(find) ); - - // Use regular expressions to replace all findValue occurrences. - const findRegExp = new RegExp( - // Escape all special characters in findValue. - findValue.replace(/[-\\^$*+?.()|[\]{}]/g, "\\$&"), - "g" - ); - for (const word of changedWords) { - word.vernacular = word.vernacular.replace(findRegExp, replaceValue); - await updateWord(word); + if (changedWords.length) { + const findRegExp = new RegExp( + find.replace(/[-\\^$*+?.()|[\]{}]/g, "\\$&"), + "g" + ); + const words: Hash = {}; + for (const word of changedWords) { + word.vernacular = word.vernacular.replace(findRegExp, replace); + words[word.id] = (await updateWord(word)).id; + } + await dispatch(addWordChanges({ find, replace, words })); + await dispatch(fetchWords()); + await dispatch(getAllCharacters()); } - - await dispatch(fetchWords()); - await dispatch(getAllCharacters()); }; } @@ -215,7 +235,7 @@ function countOccurrences(char: string, words: string[]): number { /** Compare the `.validCharacters` and `.rejectedCharacters` between the given project * and the given state to identify all characters with a change of inventory status. */ -export function getChanges( +export function getCharChanges( project: Project, charInvState: CharacterInventoryState ): CharacterChange[] { diff --git a/src/goals/CharacterInventory/Redux/tests/CharacterInventoryActions.test.tsx b/src/goals/CharacterInventory/Redux/tests/CharacterInventoryActions.test.tsx index 1a71e2c5ba..9b2380a075 100644 --- a/src/goals/CharacterInventory/Redux/tests/CharacterInventoryActions.test.tsx +++ b/src/goals/CharacterInventory/Redux/tests/CharacterInventoryActions.test.tsx @@ -1,30 +1,34 @@ -import { Action, PreloadedState } from "redux"; +import { type Action, type PreloadedState } from "redux"; -import { Project } from "api/models"; +import { type Project, type Word } from "api/models"; import { defaultState } from "components/App/DefaultState"; import { + type CharInvChanges, + type CharacterChange, CharacterStatus, - CharacterChange, + type FindAndReplaceChange, + defaultCharInvChanges, } from "goals/CharacterInventory/CharacterInventoryTypes"; import { fetchWords, + findAndReplace, getAllCharacters, - getChanges, + getCharChanges, loadCharInvData, setCharacterStatus, uploadInventory, } from "goals/CharacterInventory/Redux/CharacterInventoryActions"; import { - defaultState as defaultCharInvState, - CharacterInventoryState, - CharacterSetEntry, + type CharacterInventoryState, + type CharacterSetEntry, } from "goals/CharacterInventory/Redux/CharacterInventoryReduxTypes"; -import { RootState, setupStore } from "store"; +import { type RootState, setupStore } from "store"; import { newProject } from "types/project"; import { newWord } from "types/word"; jest.mock("backend", () => ({ getFrontierWords: (...args: any[]) => mockGetFrontierWords(...args), + updateWord: (word: Word) => mockUpdateWord(word), })); jest.mock("browserRouter"); jest.mock("components/Project/ProjectActions", () => ({ @@ -32,15 +36,26 @@ jest.mock("components/Project/ProjectActions", () => ({ mockAsyncUpdateCurrentProject(...args), })); jest.mock("goals/Redux/GoalActions", () => ({ - asyncUpdateGoal: (...args: any[]) => mockAsyncUpdateGoal(...args), - addCharInvChangesToGoal: (...args: any[]) => - mockAddCharInvChangesToGoal(...args), + asyncUpdateGoal: () => mockAsyncUpdateGoal(), + addCharInvChangesToGoal: (changes: CharInvChanges) => + mockAddCharInvChangesToGoal(changes), })); const mockAddCharInvChangesToGoal = jest.fn(); const mockAsyncUpdateCurrentProject = jest.fn(); const mockAsyncUpdateGoal = jest.fn(); const mockGetFrontierWords = jest.fn(); +const mockUpdateWord = jest.fn(); + +const bumpId = (id: string): string => `${id}++`; +const mockAction: Action = { type: null }; +const setMockFunctions = (): void => { + mockAddCharInvChangesToGoal.mockReturnValue(mockAction); + mockAsyncUpdateCurrentProject.mockReturnValue(mockAction); + mockAsyncUpdateGoal.mockReturnValue(mockAction); + // Set mockGetFrontierWords specifically for each tests. + mockUpdateWord.mockImplementation((w: Word) => ({ ...w, id: bumpId(w.id) })); +}; // Preloaded values for store when testing const persistedDefaultState: PreloadedState = { @@ -50,6 +65,7 @@ const persistedDefaultState: PreloadedState = { beforeEach(() => { jest.resetAllMocks(); + setMockFunctions(); }); describe("CharacterInventoryActions", () => { @@ -122,17 +138,11 @@ describe("CharacterInventoryActions", () => { }, }); - // Mock the dispatch functions called by uploadInventory. - const mockAction: Action = { type: null }; - mockAddCharInvChangesToGoal.mockReturnValue(mockAction); - mockAsyncUpdateCurrentProject.mockReturnValue(mockAction); - mockAsyncUpdateGoal.mockReturnValue(mockAction); - await store.dispatch(uploadInventory()); expect(mockAddCharInvChangesToGoal).toHaveBeenCalledTimes(1); - expect(mockAddCharInvChangesToGoal.mock.calls[0][0]).toHaveLength( - rejectedCharacters.length + validCharacters.length - ); + expect( + mockAddCharInvChangesToGoal.mock.calls[0][0].charChanges + ).toHaveLength(rejectedCharacters.length + validCharacters.length); expect(mockAsyncUpdateCurrentProject).toHaveBeenCalledTimes(1); const proj: Project = mockAsyncUpdateCurrentProject.mock.calls[0][0]; expect(proj.rejectedCharacters).toHaveLength(rejectedCharacters.length); @@ -212,7 +222,71 @@ describe("CharacterInventoryActions", () => { }); }); - describe("getChanges", () => { + describe("findAndReplace", () => { + it("does nothing if no words changed", async () => { + const store = setupStore(); + const word: Word = { ...newWord("bcd"), id: "mock-id" }; + mockGetFrontierWords.mockResolvedValue([word]); + + await store.dispatch(findAndReplace("A", "a")); + expect(mockGetFrontierWords).toHaveBeenCalledTimes(1); + expect(mockUpdateWord).not.toHaveBeenCalled(); + expect(mockAddCharInvChangesToGoal).not.toHaveBeenCalled(); + expect(mockAsyncUpdateGoal).not.toHaveBeenCalled(); + }); + + it("acts when words changed", async () => { + const store = setupStore(); + const word: Word = { ...newWord("Abc"), id: "mock-id" }; + mockGetFrontierWords.mockResolvedValue([word]); + + await store.dispatch(findAndReplace("A", "a")); + expect(mockGetFrontierWords).toHaveBeenCalledTimes(2); + expect(mockUpdateWord).toHaveBeenCalledTimes(1); + expect(mockAddCharInvChangesToGoal).toHaveBeenCalledTimes(1); + expect(mockAsyncUpdateGoal).toHaveBeenCalledTimes(1); + expect(store.getState().characterInventoryState.allWords).toEqual([ + "abc", + ]); + }); + + it("appends word changes", async () => { + const prevWordChanges: FindAndReplaceChange = { + find: "Q", + replace: "q", + words: { ["old"]: "new" }, + }; + const store = setupStore({ + ...persistedDefaultState, + goalsState: { + ...persistedDefaultState.goalsState, + currentGoal: { + ...persistedDefaultState.goalsState.currentGoal, + changes: { + ...defaultCharInvChanges, + wordChanges: [prevWordChanges], + }, + }, + }, + }); + const word: Word = { ...newWord("Abc"), id: "mock-id" }; + mockGetFrontierWords.mockResolvedValue([word]); + + await store.dispatch(findAndReplace("A", "a")); + expect(mockAddCharInvChangesToGoal).toHaveBeenCalledTimes(1); + const newWordChanges: FindAndReplaceChange = { + find: "A", + replace: "a", + words: { [word.id]: bumpId(word.id) }, + }; + expect(mockAddCharInvChangesToGoal).toHaveBeenCalledWith({ + ...defaultCharInvChanges, + wordChanges: [prevWordChanges, newWordChanges], + }); + }); + }); + + describe("getCharChanges", () => { it("returns correct changes", () => { const accAcc = "accepted"; const accRej = "accepted->rejected"; @@ -228,7 +302,7 @@ describe("CharacterInventoryActions", () => { rejectedCharacters: [rejAcc, rejRej, rejUnd], }; const charInvState: CharacterInventoryState = { - ...defaultCharInvState, + ...persistedDefaultState.characterInventoryState, validCharacters: [accAcc, rejAcc, undAcc], rejectedCharacters: [accRej, rejRej, undRej], }; @@ -240,9 +314,9 @@ describe("CharacterInventoryActions", () => { [undAcc, CharacterStatus.Undecided, CharacterStatus.Accepted], [undRej, CharacterStatus.Undecided, CharacterStatus.Rejected], ]; - const changes = getChanges(oldProj, charInvState); - expect(changes.length).toEqual(expectedChanges.length); - expectedChanges.forEach((ch) => expect(changes).toContainEqual(ch)); + const charChanges = getCharChanges(oldProj, charInvState); + expect(charChanges.length).toEqual(expectedChanges.length); + expectedChanges.forEach((ch) => expect(charChanges).toContainEqual(ch)); }); }); }); diff --git a/src/goals/CharacterInventory/tests/CharInvCompleted.test.tsx b/src/goals/CharacterInventory/tests/CharInvCompleted.test.tsx new file mode 100644 index 0000000000..412dff6fb7 --- /dev/null +++ b/src/goals/CharacterInventory/tests/CharInvCompleted.test.tsx @@ -0,0 +1,171 @@ +import { Provider } from "react-redux"; +import { + type ReactTestInstance, + type ReactTestRenderer, + act, + create, +} from "react-test-renderer"; +import configureMockStore from "redux-mock-store"; + +import WordCard from "components/WordCard"; +import CharInvCompleted, { + CharChange, + CharInvChangesGoalList, + CharInvCompletedId, +} from "goals/CharacterInventory/CharInvCompleted"; +import { + type CharInvChanges, + type CharacterChange, + CharacterStatus, + type FindAndReplaceChange, + defaultCharInvChanges, +} from "goals/CharacterInventory/CharacterInventoryTypes"; +import { defaultState } from "goals/Redux/GoalReduxTypes"; +import { type StoreState } from "types"; +import { newWord as mockWord } from "types/word"; + +jest.mock("backend", () => ({ + areInFrontier: (ids: string[]) => Promise.resolve(ids), + getWord: () => Promise.resolve(mockWord()), + updateWord: () => jest.fn(), +})); +// Mock "i18n", else `thrown: "Error: Error: connect ECONNREFUSED ::1:80 [...]` +jest.mock("i18n", () => ({})); + +const mockCharChanges: CharacterChange[] = [ + ["a", CharacterStatus.Accepted, CharacterStatus.Rejected], + ["b", CharacterStatus.Accepted, CharacterStatus.Undecided], + ["c", CharacterStatus.Rejected, CharacterStatus.Accepted], + ["d", CharacterStatus.Rejected, CharacterStatus.Undecided], + ["e", CharacterStatus.Undecided, CharacterStatus.Accepted], + ["f", CharacterStatus.Undecided, CharacterStatus.Rejected], +]; +const mockWordKeys = ["oldA", "oldB", "oldC"]; +const mockWordChanges: FindAndReplaceChange = { + find: "Q", + replace: "q", + words: { + [mockWordKeys[0]]: "newA", + [mockWordKeys[1]]: "newB", + [mockWordKeys[2]]: "newC", + }, +}; +const mockState = (changes?: CharInvChanges): Partial => ({ + goalsState: { + ...defaultState, + currentGoal: { ...defaultState.currentGoal, changes }, + }, +}); + +let renderer: ReactTestRenderer; +let root: ReactTestInstance; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe("CharInvCompleted", () => { + const renderCharInvCompleted = async ( + changes?: CharInvChanges + ): Promise => { + await act(async () => { + renderer = create( + + + + ); + }); + root = renderer.root; + }; + + it("renders all char inv changes", async () => { + await renderCharInvCompleted({ + ...defaultCharInvChanges, + charChanges: mockCharChanges, + }); + expect(root.findAllByType(CharChange)).toHaveLength(mockCharChanges.length); + expect(root.findAllByType(WordCard)).toHaveLength(0); + + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyNoCharChanges }) + ).toThrow(); + root.findByProps({ id: CharInvCompletedId.TypographyNoWordChanges }); + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyWordChanges }) + ).toThrow(); + }); + + it("renders all words changed", async () => { + await renderCharInvCompleted({ + ...defaultCharInvChanges, + wordChanges: [mockWordChanges], + }); + expect(root.findAllByType(CharChange)).toHaveLength(0); + expect(renderer.root.findAllByType(WordCard)).toHaveLength( + mockWordKeys.length + ); + + root.findByProps({ id: CharInvCompletedId.TypographyNoCharChanges }); + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyNoWordChanges }) + ).toThrow(); + root.findByProps({ id: CharInvCompletedId.TypographyWordChanges }); + }); +}); + +describe("CharInvChangesGoalList", () => { + const renderCharInvChangesGoalList = async ( + changes?: CharInvChanges + ): Promise => { + await act(async () => { + renderer = create( + CharInvChangesGoalList(changes ?? defaultCharInvChanges) + ); + }); + root = renderer.root; + }; + + it("renders up to 3 char changes", async () => { + const changes = (count: number): CharInvChanges => ({ + ...defaultCharInvChanges, + charChanges: mockCharChanges.slice(0, count), + }); + await renderCharInvChangesGoalList(changes(0)); + expect(root.findAllByType(CharChange)).toHaveLength(0); + await renderCharInvChangesGoalList(changes(1)); + expect(root.findAllByType(CharChange)).toHaveLength(1); + await renderCharInvChangesGoalList(changes(3)); + expect(root.findAllByType(CharChange)).toHaveLength(3); + }); + + it("doesn't render more than 3 char changes", async () => { + expect(mockCharChanges.length).toBeGreaterThan(3); + await renderCharInvChangesGoalList({ + ...defaultCharInvChanges, + charChanges: mockCharChanges, + }); + // When more than 3 changes, show 2 changes and a "+_ more" line. + expect(root.findAllByType(CharChange)).toHaveLength(2); + }); + + it("doesn't show word changes when there are none", async () => { + await renderCharInvChangesGoalList(defaultCharInvChanges); + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyNoWordChanges }) + ).toThrow(); + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyWordChanges }) + ).toThrow(); + }); + + it("shows word changes when there are some", async () => { + await renderCharInvChangesGoalList({ + ...defaultCharInvChanges, + wordChanges: [mockWordChanges], + }); + expect(() => + root.findByProps({ id: CharInvCompletedId.TypographyNoWordChanges }) + ).toThrow(); + root.findByProps({ id: CharInvCompletedId.TypographyWordChanges }); + }); +}); diff --git a/src/goals/Redux/GoalActions.ts b/src/goals/Redux/GoalActions.ts index d798161155..58bbaa58c0 100644 --- a/src/goals/Redux/GoalActions.ts +++ b/src/goals/Redux/GoalActions.ts @@ -5,7 +5,7 @@ import * as Backend from "backend"; import { getDuplicates, getGraylistEntries } from "backend"; import { getCurrentUser, getProjectId } from "backend/localStorage"; import router from "browserRouter"; -import { CharacterChange } from "goals/CharacterInventory/CharacterInventoryTypes"; +import { CharInvChanges } from "goals/CharacterInventory/CharacterInventoryTypes"; import { dispatchMergeStepData } from "goals/MergeDuplicates/Redux/MergeDupsActions"; import { addCharInvChangesToGoalAction, @@ -28,9 +28,9 @@ import { convertEditToGoal, maxNumSteps } from "utilities/goalUtilities"; // Action Creation Functions export function addCharInvChangesToGoal( - charChanges: CharacterChange[] + changes: CharInvChanges ): PayloadAction { - return addCharInvChangesToGoalAction(charChanges); + return addCharInvChangesToGoalAction(changes); } export function addEntryEditToGoal(entryEdit: EntryEdit): PayloadAction { diff --git a/src/goals/Redux/GoalReducer.ts b/src/goals/Redux/GoalReducer.ts index 64da779845..c681c1ea1e 100644 --- a/src/goals/Redux/GoalReducer.ts +++ b/src/goals/Redux/GoalReducer.ts @@ -18,7 +18,7 @@ const goalSlice = createSlice({ reducers: { addCharInvChangesToGoalAction: (state, action) => { if (state.currentGoal.goalType === GoalType.CreateCharInv) { - state.currentGoal.changes = { charChanges: action.payload }; + state.currentGoal.changes = action.payload; } }, addCompletedMergeToGoalAction: (state, action) => {