From b8cc180c2d01c34529edfaf390e131ba976113cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 19:47:20 +0000 Subject: [PATCH 1/9] Initial plan From 165c2c34643845bc0c25a1e9f98c5169658da5c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 19:58:10 +0000 Subject: [PATCH 2/9] Backend: Add findIdenticalDuplicates endpoint and update frontend integration Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com> --- Backend/Controllers/MergeController.cs | 29 ++++ Backend/Interfaces/IMergeService.cs | 3 + Backend/Services/MergeService.cs | 19 +-- src/api/api/merge-api.ts | 197 +++++++++++++++++++++++++ src/backend/index.ts | 14 ++ src/goals/Redux/GoalActions.ts | 47 +++--- 6 files changed, 274 insertions(+), 35 deletions(-) diff --git a/Backend/Controllers/MergeController.cs b/Backend/Controllers/MergeController.cs index a6ffbdf0d5..8b1fdff517 100644 --- a/Backend/Controllers/MergeController.cs +++ b/Backend/Controllers/MergeController.cs @@ -121,6 +121,35 @@ public async Task GraylistAdd(string projectId, [FromBody, BindRe return Ok(graylistEntry.WordIds); } + /// Find and return lists of potential duplicates with identical vernacular. + /// Id of project in which to search the frontier for potential duplicates. + /// Max number of words allowed within a list of potential duplicates. + /// Max number of lists of potential duplicates. + /// Whether to require each set to have at least one unprotected word. + /// List of Lists of s, each sublist a set of potential duplicates. + [HttpGet("findidenticaldups/{maxInList:int}/{maxLists:int}/{ignoreProtected:bool}", Name = "FindIdenticalPotentialDuplicates")] + [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(List>))] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + public async Task FindIdenticalPotentialDuplicates( + string projectId, int maxInList, int maxLists, bool ignoreProtected) + { + using var activity = OtelService.StartActivityWithTag(otelTagName, "finding identical potential duplicates"); + + if (!await _permissionService.HasProjectPermission( + HttpContext, Permission.MergeAndReviewEntries, projectId)) + { + return Forbid(); + } + + await _mergeService.UpdateMergeBlacklist(projectId); + + var userId = _permissionService.GetUserId(HttpContext); + var dups = await _mergeService.GetPotentialDuplicates( + projectId, maxInList, maxLists, identicalVernacular: true, userId, ignoreProtected); + + return Ok(dups); + } + /// Start finding lists of potential duplicates for merging. /// Id of project in which to search the frontier for potential duplicates. /// Max number of words allowed within a list of potential duplicates. diff --git a/Backend/Interfaces/IMergeService.cs b/Backend/Interfaces/IMergeService.cs index 7bcda787dc..352ba3d504 100644 --- a/Backend/Interfaces/IMergeService.cs +++ b/Backend/Interfaces/IMergeService.cs @@ -17,6 +17,9 @@ public interface IMergeService Task UpdateMergeGraylist(string projectId); Task GetAndStorePotentialDuplicates( string projectId, int maxInList, int maxLists, string userId, bool ignoreProtected = false); + Task>> GetPotentialDuplicates( + string projectId, int maxInList, int maxLists, bool identicalVernacular, + string? userId = null, bool ignoreProtected = false); List>? RetrieveDups(string userId); Task HasGraylistEntries(string projectId, string? userId = null); Task>> GetGraylistEntries(string projectId, int maxLists, string? userId = null); diff --git a/Backend/Services/MergeService.cs b/Backend/Services/MergeService.cs index 039965eaca..47dc5001c9 100644 --- a/Backend/Services/MergeService.cs +++ b/Backend/Services/MergeService.cs @@ -413,7 +413,8 @@ public async Task GetAndStorePotentialDuplicates( { return false; } - var dups = await GetPotentialDuplicates(projectId, maxInList, maxLists, userId, ignoreProtected); + var dups = await GetPotentialDuplicates( + projectId, maxInList, maxLists, identicalVernacular: false, userId, ignoreProtected); // Store the potential duplicates for user to retrieve later. return StoreDups(userId, counter, dups) == counter; } @@ -421,8 +422,9 @@ public async Task GetAndStorePotentialDuplicates( /// /// Get Lists of potential duplicate s in specified 's frontier. /// - private async Task>> GetPotentialDuplicates( - string projectId, int maxInList, int maxLists, string? userId = null, bool ignoreProtected = false) + public async Task>> GetPotentialDuplicates( + string projectId, int maxInList, int maxLists, bool identicalVernacular, + string? userId = null, bool ignoreProtected = false) { var dupFinder = new DuplicateFinder(maxInList, maxLists, 2); @@ -431,17 +433,12 @@ async Task isUnavailableSet(List wordIds) => (await IsInMergeBlacklist(projectId, wordIds, userId)) || (await IsInMergeGraylist(projectId, wordIds, userId)); - // First pass, only look for words with identical vernacular. - var wordLists = await dupFinder.GetIdenticalVernWords(collection, isUnavailableSet, ignoreProtected); - - // If no such sets found, look for similar words. - if (wordLists.Count == 0) + if (identicalVernacular) { - collection = await _wordRepo.GetFrontier(projectId); - wordLists = await dupFinder.GetSimilarWords(collection, isUnavailableSet, ignoreProtected); + return await dupFinder.GetIdenticalVernWords(collection, isUnavailableSet, ignoreProtected); } - return wordLists; + return await dupFinder.GetSimilarWords(collection, isUnavailableSet, ignoreProtected); } public sealed class InvalidMergeWordSetException : Exception diff --git a/src/api/api/merge-api.ts b/src/api/api/merge-api.ts index 2b098697c1..d844eecefc 100644 --- a/src/api/api/merge-api.ts +++ b/src/api/api/merge-api.ts @@ -107,6 +107,84 @@ export const MergeApiAxiosParamCreator = function ( options: localVarRequestOptions, }; }, + /** + * + * @param {string} projectId + * @param {number} maxInList + * @param {number} maxLists + * @param {boolean} ignoreProtected + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + findIdenticalPotentialDuplicates: async ( + projectId: string, + maxInList: number, + maxLists: number, + ignoreProtected: boolean, + options: any = {} + ): Promise => { + // verify required parameter 'projectId' is not null or undefined + assertParamExists( + "findIdenticalPotentialDuplicates", + "projectId", + projectId + ); + // verify required parameter 'maxInList' is not null or undefined + assertParamExists( + "findIdenticalPotentialDuplicates", + "maxInList", + maxInList + ); + // verify required parameter 'maxLists' is not null or undefined + assertParamExists( + "findIdenticalPotentialDuplicates", + "maxLists", + maxLists + ); + // verify required parameter 'ignoreProtected' is not null or undefined + assertParamExists( + "findIdenticalPotentialDuplicates", + "ignoreProtected", + ignoreProtected + ); + const localVarPath = + `/v1/projects/{projectId}/merge/findidenticaldups/{maxInList}/{maxLists}/{ignoreProtected}` + .replace(`{${"projectId"}}`, encodeURIComponent(String(projectId))) + .replace(`{${"maxInList"}}`, encodeURIComponent(String(maxInList))) + .replace(`{${"maxLists"}}`, encodeURIComponent(String(maxLists))) + .replace( + `{${"ignoreProtected"}}`, + encodeURIComponent(String(ignoreProtected)) + ); + // 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: "GET", + ...baseOptions, + ...options, + }; + const localVarHeaderParameter = {} as any; + const localVarQueryParameter = {} as any; + + setSearchParams(localVarUrlObj, localVarQueryParameter, options.query); + let headersFromBaseOptions = + baseOptions && baseOptions.headers ? baseOptions.headers : {}; + localVarRequestOptions.headers = { + ...localVarHeaderParameter, + ...headersFromBaseOptions, + ...options.headers, + }; + + return { + url: toPathString(localVarUrlObj), + options: localVarRequestOptions, + }; + }, /** * * @param {string} projectId @@ -526,6 +604,42 @@ export const MergeApiFp = function (configuration?: Configuration) { configuration ); }, + /** + * + * @param {string} projectId + * @param {number} maxInList + * @param {number} maxLists + * @param {boolean} ignoreProtected + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + async findIdenticalPotentialDuplicates( + projectId: string, + maxInList: number, + maxLists: number, + ignoreProtected: boolean, + options?: any + ): Promise< + ( + axios?: AxiosInstance, + basePath?: string + ) => AxiosPromise>> + > { + const localVarAxiosArgs = + await localVarAxiosParamCreator.findIdenticalPotentialDuplicates( + projectId, + maxInList, + maxLists, + ignoreProtected, + options + ); + return createRequestFunction( + localVarAxiosArgs, + globalAxios, + BASE_PATH, + configuration + ); + }, /** * * @param {string} projectId @@ -754,6 +868,32 @@ export const MergeApiFactory = function ( .blacklistAdd(projectId, requestBody, options) .then((request) => request(axios, basePath)); }, + /** + * + * @param {string} projectId + * @param {number} maxInList + * @param {number} maxLists + * @param {boolean} ignoreProtected + * @param {*} [options] Override http request option. + * @throws {RequiredError} + */ + findIdenticalPotentialDuplicates( + projectId: string, + maxInList: number, + maxLists: number, + ignoreProtected: boolean, + options?: any + ): AxiosPromise>> { + return localVarFp + .findIdenticalPotentialDuplicates( + projectId, + maxInList, + maxLists, + ignoreProtected, + options + ) + .then((request) => request(axios, basePath)); + }, /** * * @param {string} projectId @@ -900,6 +1040,41 @@ export interface MergeApiBlacklistAddRequest { readonly requestBody: Array; } +/** + * Request parameters for findIdenticalPotentialDuplicates operation in MergeApi. + * @export + * @interface MergeApiFindIdenticalPotentialDuplicatesRequest + */ +export interface MergeApiFindIdenticalPotentialDuplicatesRequest { + /** + * + * @type {string} + * @memberof MergeApiFindIdenticalPotentialDuplicates + */ + readonly projectId: string; + + /** + * + * @type {number} + * @memberof MergeApiFindIdenticalPotentialDuplicates + */ + readonly maxInList: number; + + /** + * + * @type {number} + * @memberof MergeApiFindIdenticalPotentialDuplicates + */ + readonly maxLists: number; + + /** + * + * @type {boolean} + * @memberof MergeApiFindIdenticalPotentialDuplicates + */ + readonly ignoreProtected: boolean; +} + /** * Request parameters for findPotentialDuplicates operation in MergeApi. * @export @@ -1088,6 +1263,28 @@ export class MergeApi extends BaseAPI { .then((request) => request(this.axios, this.basePath)); } + /** + * + * @param {MergeApiFindIdenticalPotentialDuplicatesRequest} requestParameters Request parameters. + * @param {*} [options] Override http request option. + * @throws {RequiredError} + * @memberof MergeApi + */ + public findIdenticalPotentialDuplicates( + requestParameters: MergeApiFindIdenticalPotentialDuplicatesRequest, + options?: any + ) { + return MergeApiFp(this.configuration) + .findIdenticalPotentialDuplicates( + requestParameters.projectId, + requestParameters.maxInList, + requestParameters.maxLists, + requestParameters.ignoreProtected, + options + ) + .then((request) => request(this.axios, this.basePath)); + } + /** * * @param {MergeApiFindPotentialDuplicatesRequest} requestParameters Request parameters. diff --git a/src/backend/index.ts b/src/backend/index.ts index 20a226062b..47ae48e377 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -362,6 +362,20 @@ export async function graylistAdd(wordIds: string[]): Promise { ); } +/** Find and return lists of potential duplicates with identical vernacular. */ +export async function findIdenticalDuplicates( + maxInList: number, + maxLists: number, + ignoreProtected = false +): Promise { + const projectId = LocalStorage.getProjectId(); + const resp = await mergeApi.findIdenticalPotentialDuplicates( + { ignoreProtected, maxInList, maxLists, projectId }, + defaultOptions() + ); + return resp.data; +} + /** Start finding list of potential duplicates for merging. */ export async function findDuplicates( maxInList: number, diff --git a/src/goals/Redux/GoalActions.ts b/src/goals/Redux/GoalActions.ts index 152fce6c8e..d6bcf377fc 100644 --- a/src/goals/Redux/GoalActions.ts +++ b/src/goals/Redux/GoalActions.ts @@ -83,21 +83,8 @@ export function asyncAddGoal(goal: Goal) { await Backend.addGoalToUserEdit(userEditId, goal); dispatch(setCurrentGoal(goal)); - // Start loading goal data. - if (goal.goalType === GoalType.MergeDups) { - // Initialize data loading in the backend. - dispatch(setDataLoadStatus(DataLoadStatus.Loading)); - const currentProj = getState().currentProjectState.project; - await Backend.findDuplicates( - 5, // More than 5 entries doesn't fit well. - maxNumSteps(goal.goalType), - currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On - ); - // Don't load goal data, since it'll be triggered by a signal from the backend when data is ready. - } else { - // Load the goal data, but don't await, to allow a loading screen. - dispatch(asyncLoadNewGoalData()); - } + // Load the goal data, but don't await, to allow a loading screen. + dispatch(asyncLoadNewGoalData()); } // Serve goal. @@ -160,11 +147,16 @@ export function asyncLoadExistingUserEdits( export function asyncLoadNewGoalData() { return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { const currentGoal = getState().goalsState.currentGoal; - const goalData = await loadGoalData(currentGoal.goalType).catch(() => { - dispatch(setDataLoadStatus(DataLoadStatus.Failure)); - alert("Failed to load data."); - router.navigate(Path.Goals); - }); + const currentProj = getState().currentProjectState.project; + const ignoreProtected = + currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On; + const goalData = await loadGoalData(currentGoal.goalType, ignoreProtected).catch( + () => { + dispatch(setDataLoadStatus(DataLoadStatus.Failure)); + alert("Failed to load data."); + router.navigate(Path.Goals); + } + ); if (!goalData) { return; } @@ -242,12 +234,19 @@ function goalCleanup(goal: Goal): void { } /** Returns goal data for some goal types. */ -async function loadGoalData(goalType: GoalType): Promise { +async function loadGoalData( + goalType: GoalType, + ignoreProtected = false +): Promise { switch (goalType) { case GoalType.MergeDups: - // Catch failure and pass to caller to allow for error dispatch. - const dups = await Backend.retrieveDuplicates().catch(() => {}); - return dups ? checkMergeData(dups) : Promise.reject(); + // Find identical duplicates first (fast). + const dups = await Backend.findIdenticalDuplicates( + 5, // More than 5 entries doesn't fit well. + maxNumSteps(goalType), + ignoreProtected + ); + return checkMergeData(dups); case GoalType.ReviewDeferredDups: return checkMergeData( await Backend.getGraylistEntries(maxNumSteps(goalType)) From 564db9b6141bce4a762eaff8dd145f158f6c0cd7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 20:03:06 +0000 Subject: [PATCH 3/9] Frontend: Add dialog for when identical duplicates are all processed Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com> --- public/locales/en/translation.json | 9 ++ .../IdenticalDuplicatesDialog.tsx | 105 ++++++++++++++++++ .../MergeDuplicates/MergeDupsStep/index.tsx | 39 ++++++- src/goals/Redux/GoalActions.ts | 26 ++++- src/goals/Redux/GoalReduxTypes.ts | 1 + 5 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index 29c3d3b82f..f39d58a3ad 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -516,6 +516,15 @@ "yes": "Yes", "no": "No" }, + "identicalCompleted": { + "title": "All Identical Duplicates Processed", + "congratulations": "Congratulations! You have processed all sets of words with identical vernacular forms.", + "deferredCount": "You have {{count}} deferred duplicate set(s) that can be reviewed later.", + "findingSimilar": "The Combine will now search for potential duplicates with similar (non-identical) vernacular forms.", + "warning": "This project has more than 1,000 entries. Finding similar duplicates may take several minutes.", + "reviewDeferred": "Review Deferred", + "continue": "Continue" + }, "undo": { "undo": "Undo Merge", "undoDialog": "Undo this merge?", diff --git a/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx new file mode 100644 index 0000000000..ea15d0973f --- /dev/null +++ b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx @@ -0,0 +1,105 @@ +import { + Button, + Dialog, + DialogActions, + DialogContent, + DialogTitle, + Typography, +} from "@mui/material"; +import { ReactElement, useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; + +import { getFrontierWords, hasGraylistEntries } from "backend"; + +export interface IdenticalDuplicatesDialogProps { + onCancel: () => void; + onContinue: () => void; + onReviewDeferred: () => void; +} + +export default function IdenticalDuplicatesDialog( + props: IdenticalDuplicatesDialogProps +): ReactElement { + const [open, setOpen] = useState(true); + const [deferredCount, setDeferredCount] = useState(0); + const [frontierCount, setFrontierCount] = useState(0); + const { t } = useTranslation(); + + const { onCancel, onContinue, onReviewDeferred } = props; + + useEffect(() => { + hasGraylistEntries().then((hasDeferred) => { + if (hasDeferred) { + // Count deferred entries - this is a rough estimate + // In a real implementation, we'd need a backend endpoint to get the exact count + setDeferredCount(1); // Placeholder + } + }); + getFrontierWords().then((words) => { + setFrontierCount(words.length); + }); + }, []); + + const handleCancel = (): void => { + setOpen(false); + onCancel(); + }; + + const handleContinue = (): void => { + setOpen(false); + onContinue(); + }; + + const handleReviewDeferred = (): void => { + setOpen(false); + onReviewDeferred(); + }; + + return ( + + {t("mergeDups.identicalCompleted.title")} + + + {t("mergeDups.identicalCompleted.congratulations")} + + {deferredCount > 0 && ( + + {t("mergeDups.identicalCompleted.deferredCount", { + count: deferredCount, + })} + + )} + + {t("mergeDups.identicalCompleted.findingSimilar")} + + {frontierCount > 1000 && ( + + {t("mergeDups.identicalCompleted.warning")} + + )} + + + + {deferredCount > 0 && ( + + )} + + + + ); +} diff --git a/src/goals/MergeDuplicates/MergeDupsStep/index.tsx b/src/goals/MergeDuplicates/MergeDupsStep/index.tsx index da42fbea19..42b331391c 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/index.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/index.tsx @@ -2,19 +2,56 @@ import { Box, Typography } from "@mui/material"; import { ReactElement } from "react"; import { useTranslation } from "react-i18next"; +import IdenticalDuplicatesDialog from "goals/MergeDuplicates/IdenticalDuplicatesDialog"; import MergeDragDrop from "goals/MergeDuplicates/MergeDupsStep/MergeDragDrop"; import SaveDeferButtons from "goals/MergeDuplicates/MergeDupsStep/SaveDeferButtons"; -import { useAppSelector } from "rootRedux/hooks"; +import { + asyncLoadSimilarDuplicates, + setDataLoadStatus, +} from "goals/Redux/GoalActions"; +import { DataLoadStatus } from "goals/Redux/GoalReduxTypes"; +import { useAppDispatch, useAppSelector } from "rootRedux/hooks"; import { type StoreState } from "rootRedux/types"; +import router from "router/browserRouter"; +import { Path } from "types/path"; export default function MergeDupsStep(): ReactElement { + const dispatch = useAppDispatch(); const wordCount = useAppSelector( (state: StoreState) => Object.keys(state.mergeDuplicateGoal.data.words).length ); + const dataLoadStatus = useAppSelector( + (state: StoreState) => state.goalsState.dataLoadStatus + ); const { t } = useTranslation(); + const handleCancel = (): void => { + dispatch(setDataLoadStatus(DataLoadStatus.Default)); + router.navigate(Path.DataEntry); + }; + + const handleContinue = (): void => { + dispatch(asyncLoadSimilarDuplicates()); + }; + + const handleReviewDeferred = (): void => { + dispatch(setDataLoadStatus(DataLoadStatus.Default)); + router.navigate(Path.Goals); + // TODO: Navigate to review deferred duplicates goal + }; + + if (dataLoadStatus === DataLoadStatus.IdenticalCompleted) { + return ( + + ); + } + return wordCount ? ( <> diff --git a/src/goals/Redux/GoalActions.ts b/src/goals/Redux/GoalActions.ts index d6bcf377fc..4004392b6f 100644 --- a/src/goals/Redux/GoalActions.ts +++ b/src/goals/Redux/GoalActions.ts @@ -73,7 +73,7 @@ export function updateStepFromData(): Action { // Dispatch Functions export function asyncAddGoal(goal: Goal) { - return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { + return async (dispatch: StoreStateDispatch) => { const userEditId = getUserEditId(); if (userEditId) { dispatch(setCurrentGoal(goal)); @@ -167,8 +167,30 @@ export function asyncLoadNewGoalData() { dispatch(dispatchStepData(updatedGoal)); await Backend.addGoalToUserEdit(getUserEditId()!, updatedGoal); await saveCurrentStep(updatedGoal); + dispatch(setGoalStatus(GoalStatus.InProgress)); + } else if (currentGoal.goalType === GoalType.MergeDups) { + // No identical duplicates found, show dialog before finding similar ones + dispatch(setDataLoadStatus(DataLoadStatus.IdenticalCompleted)); + } else { + dispatch(setGoalStatus(GoalStatus.InProgress)); } - dispatch(setGoalStatus(GoalStatus.InProgress)); + }; +} + +export function asyncLoadSimilarDuplicates() { + return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { + const currentProj = getState().currentProjectState.project; + const ignoreProtected = + currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On; + + // Initialize data loading in the backend. + dispatch(setDataLoadStatus(DataLoadStatus.Loading)); + await Backend.findDuplicates( + 5, // More than 5 entries doesn't fit well. + maxNumSteps(GoalType.MergeDups), + ignoreProtected + ); + // Don't load goal data, since it'll be triggered by a signal from the backend when data is ready. }; } diff --git a/src/goals/Redux/GoalReduxTypes.ts b/src/goals/Redux/GoalReduxTypes.ts index d5b5ca73de..bfbcde5d76 100644 --- a/src/goals/Redux/GoalReduxTypes.ts +++ b/src/goals/Redux/GoalReduxTypes.ts @@ -5,6 +5,7 @@ export enum DataLoadStatus { Failure = "FAILURE", Loading = "LOADING", Success = "SUCCESS", + IdenticalCompleted = "IDENTICAL_COMPLETED", } // The representation of goals in the redux store From eda09e98264c8c2f86a1e7b19cbd6c0c9926f92a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 20:04:39 +0000 Subject: [PATCH 4/9] Add tests for FindIdenticalPotentialDuplicates endpoint Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com> --- .../Controllers/MergeControllerTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Backend.Tests/Controllers/MergeControllerTests.cs b/Backend.Tests/Controllers/MergeControllerTests.cs index c32ef329b3..fb4658c3da 100644 --- a/Backend.Tests/Controllers/MergeControllerTests.cs +++ b/Backend.Tests/Controllers/MergeControllerTests.cs @@ -148,5 +148,21 @@ public void TestGetGraylistEntriesNoPermission() var result = _mergeController.GetGraylistEntries("projId", 3, "userId").Result; Assert.That(result, Is.InstanceOf()); } + + [Test] + public void TestFindIdenticalPotentialDuplicatesNoPermission() + { + _mergeController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext(); + var result = _mergeController.FindIdenticalPotentialDuplicates("projId", 2, 1, false).Result; + Assert.That(result, Is.InstanceOf()); + } + + [Test] + public void TestFindIdenticalPotentialDuplicates() + { + // This test verifies the endpoint returns OK and data + var result = _mergeController.FindIdenticalPotentialDuplicates(ProjId, 5, 10, false).Result; + Assert.That(result, Is.InstanceOf()); + } } } From 311e3a6104b74706c6247a7581607320bae22032 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 20:08:05 +0000 Subject: [PATCH 5/9] Fix code review issues: improve deferred handling and imports Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com> --- public/locales/en/translation.json | 2 +- .../IdenticalDuplicatesDialog.tsx | 18 +++++------------- .../MergeDuplicates/MergeDupsStep/index.tsx | 5 +++-- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index f39d58a3ad..f8d0416214 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -519,7 +519,7 @@ "identicalCompleted": { "title": "All Identical Duplicates Processed", "congratulations": "Congratulations! You have processed all sets of words with identical vernacular forms.", - "deferredCount": "You have {{count}} deferred duplicate set(s) that can be reviewed later.", + "hasDeferred": "You have deferred duplicate sets that can be reviewed later.", "findingSimilar": "The Combine will now search for potential duplicates with similar (non-identical) vernacular forms.", "warning": "This project has more than 1,000 entries. Finding similar duplicates may take several minutes.", "reviewDeferred": "Review Deferred", diff --git a/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx index ea15d0973f..65828d3e8d 100644 --- a/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx +++ b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx @@ -21,20 +21,14 @@ export default function IdenticalDuplicatesDialog( props: IdenticalDuplicatesDialogProps ): ReactElement { const [open, setOpen] = useState(true); - const [deferredCount, setDeferredCount] = useState(0); + const [hasDeferred, setHasDeferred] = useState(false); const [frontierCount, setFrontierCount] = useState(0); const { t } = useTranslation(); const { onCancel, onContinue, onReviewDeferred } = props; useEffect(() => { - hasGraylistEntries().then((hasDeferred) => { - if (hasDeferred) { - // Count deferred entries - this is a rough estimate - // In a real implementation, we'd need a backend endpoint to get the exact count - setDeferredCount(1); // Placeholder - } - }); + hasGraylistEntries().then(setHasDeferred); getFrontierWords().then((words) => { setFrontierCount(words.length); }); @@ -62,11 +56,9 @@ export default function IdenticalDuplicatesDialog( {t("mergeDups.identicalCompleted.congratulations")} - {deferredCount > 0 && ( + {hasDeferred && ( - {t("mergeDups.identicalCompleted.deferredCount", { - count: deferredCount, - })} + {t("mergeDups.identicalCompleted.hasDeferred")} )} @@ -82,7 +74,7 @@ export default function IdenticalDuplicatesDialog( - {deferredCount > 0 && ( + {hasDeferred && ( {hasDeferred && ( - )} - diff --git a/src/goals/MergeDuplicates/MergeDupsStep/index.tsx b/src/goals/MergeDuplicates/MergeDupsStep/index.tsx index 44d619b27a..da42fbea19 100644 --- a/src/goals/MergeDuplicates/MergeDupsStep/index.tsx +++ b/src/goals/MergeDuplicates/MergeDupsStep/index.tsx @@ -2,57 +2,19 @@ import { Box, Typography } from "@mui/material"; import { ReactElement } from "react"; import { useTranslation } from "react-i18next"; -import IdenticalDuplicatesDialog from "goals/MergeDuplicates/IdenticalDuplicatesDialog"; import MergeDragDrop from "goals/MergeDuplicates/MergeDupsStep/MergeDragDrop"; import SaveDeferButtons from "goals/MergeDuplicates/MergeDupsStep/SaveDeferButtons"; -import { ReviewDeferredDups } from "goals/MergeDuplicates/MergeDupsTypes"; -import { - asyncAddGoal, - asyncLoadSimilarDuplicates, - setDataLoadStatus, -} from "goals/Redux/GoalActions"; -import { DataLoadStatus } from "goals/Redux/GoalReduxTypes"; -import { useAppDispatch, useAppSelector } from "rootRedux/hooks"; +import { useAppSelector } from "rootRedux/hooks"; import { type StoreState } from "rootRedux/types"; -import router from "router/browserRouter"; -import { Path } from "types/path"; export default function MergeDupsStep(): ReactElement { - const dispatch = useAppDispatch(); const wordCount = useAppSelector( (state: StoreState) => Object.keys(state.mergeDuplicateGoal.data.words).length ); - const dataLoadStatus = useAppSelector( - (state: StoreState) => state.goalsState.dataLoadStatus - ); const { t } = useTranslation(); - const handleCancel = (): void => { - dispatch(setDataLoadStatus(DataLoadStatus.Default)); - router.navigate(Path.DataEntry); - }; - - const handleContinue = (): void => { - dispatch(asyncLoadSimilarDuplicates()); - }; - - const handleReviewDeferred = (): void => { - dispatch(setDataLoadStatus(DataLoadStatus.Default)); - dispatch(asyncAddGoal(new ReviewDeferredDups())); - }; - - if (dataLoadStatus === DataLoadStatus.IdenticalCompleted) { - return ( - - ); - } - return wordCount ? ( <> diff --git a/src/goals/Redux/GoalActions.ts b/src/goals/Redux/GoalActions.ts index 2a0192f048..f54fd4a96a 100644 --- a/src/goals/Redux/GoalActions.ts +++ b/src/goals/Redux/GoalActions.ts @@ -1,6 +1,6 @@ import { Action, PayloadAction } from "@reduxjs/toolkit"; -import { MergeUndoIds, OffOnSetting, Word } from "api/models"; +import { MergeUndoIds, OffOnSetting, Project, Word } from "api/models"; import * as Backend from "backend"; import { getCurrentUser, getProjectId } from "backend/localStorage"; import { CharInvChanges } from "goals/CharacterInventory/CharacterInventoryTypes"; @@ -146,17 +146,17 @@ export function asyncLoadExistingUserEdits( export function asyncLoadNewGoalData() { return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { - const currentGoal = getState().goalsState.currentGoal; - const currentProj = getState().currentProjectState.project; - const ignoreProtected = - currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On; - const goalData = await loadGoalData(currentGoal.goalType, ignoreProtected).catch( - () => { - dispatch(setDataLoadStatus(DataLoadStatus.Failure)); - alert("Failed to load data."); - router.navigate(Path.Goals); - } - ); + const { goalsState, currentProjectState } = getState(); + const { currentGoal, dataLoadStatus } = goalsState; + const goalData = await loadGoalData( + currentGoal.goalType, + dataLoadStatus, + currentProjectState.project + ).catch(() => { + dispatch(setDataLoadStatus(DataLoadStatus.Failure)); + alert("Failed to load data."); + router.navigate(Path.Goals); + }); if (!goalData) { return; } @@ -167,30 +167,22 @@ export function asyncLoadNewGoalData() { dispatch(dispatchStepData(updatedGoal)); await Backend.addGoalToUserEdit(getUserEditId()!, updatedGoal); await saveCurrentStep(updatedGoal); - dispatch(setGoalStatus(GoalStatus.InProgress)); - } else if (currentGoal.goalType === GoalType.MergeDups) { - // All identical duplicates have been processed, show dialog before finding similar ones - dispatch(setDataLoadStatus(DataLoadStatus.IdenticalCompleted)); - } else { - dispatch(setGoalStatus(GoalStatus.InProgress)); + } else if ( + currentGoal.goalType === GoalType.MergeDups && + dataLoadStatus !== DataLoadStatus.Success + ) { + // All identical-vernacular duplicates have been processed. + // Initialize similar-vernacular duplicate finding in the backend. + dispatch(setDataLoadStatus(DataLoadStatus.Loading)); + const currentProj = getState().currentProjectState.project; + await Backend.findDuplicates( + 5, // More than 5 entries doesn't fit well. + maxNumSteps(currentGoal.goalType), + currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On + ); + return; } - }; -} - -export function asyncLoadSimilarDuplicates() { - return async (dispatch: StoreStateDispatch, getState: () => StoreState) => { - const currentProj = getState().currentProjectState.project; - const ignoreProtected = - currentProj.protectedDataMergeAvoidEnabled === OffOnSetting.On; - - // Initialize data loading in the backend. - dispatch(setDataLoadStatus(DataLoadStatus.Loading)); - await Backend.findDuplicates( - 5, // More than 5 entries doesn't fit well. - maxNumSteps(GoalType.MergeDups), - ignoreProtected - ); - // Don't load goal data, since it'll be triggered by a signal from the backend when data is ready. + dispatch(setGoalStatus(GoalStatus.InProgress)); }; } @@ -258,17 +250,21 @@ function goalCleanup(goal: Goal): void { /** Returns goal data for some goal types. */ async function loadGoalData( goalType: GoalType, - ignoreProtected = false + dataLoadStatus: DataLoadStatus, + project: Project ): Promise { switch (goalType) { case GoalType.MergeDups: - // Find identical duplicates first (fast). - const dups = await Backend.findIdenticalDuplicates( - 5, // More than 5 entries doesn't fit well. - maxNumSteps(goalType), - ignoreProtected - ); - return checkMergeData(dups); + // Catch failure and pass to caller to allow for error dispatch. + const dups = + dataLoadStatus === DataLoadStatus.Success + ? await Backend.retrieveDuplicates().catch(() => {}) + : await Backend.findIdenticalDuplicates( + 5, // More than 5 entries doesn't fit well. + maxNumSteps(goalType), + project.protectedDataMergeAvoidEnabled === OffOnSetting.On + ).catch(() => {}); + return dups ? checkMergeData(dups) : Promise.reject(); case GoalType.ReviewDeferredDups: return checkMergeData( await Backend.getGraylistEntries(maxNumSteps(goalType)) diff --git a/src/goals/Redux/GoalReduxTypes.ts b/src/goals/Redux/GoalReduxTypes.ts index bfbcde5d76..d5b5ca73de 100644 --- a/src/goals/Redux/GoalReduxTypes.ts +++ b/src/goals/Redux/GoalReduxTypes.ts @@ -5,7 +5,6 @@ export enum DataLoadStatus { Failure = "FAILURE", Loading = "LOADING", Success = "SUCCESS", - IdenticalCompleted = "IDENTICAL_COMPLETED", } // The representation of goals in the redux store From dd384dd4be7a3e1d6d02419005a77ef45a3263ad Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Dec 2025 17:21:06 -0500 Subject: [PATCH 8/9] Keep dialog open until user closes it --- src/goals/DefaultGoal/BaseGoalScreen.tsx | 24 +++++++--- .../IdenticalDuplicatesDialog.tsx | 47 ++++++++++++------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/goals/DefaultGoal/BaseGoalScreen.tsx b/src/goals/DefaultGoal/BaseGoalScreen.tsx index e4683a7acf..66196b24f7 100644 --- a/src/goals/DefaultGoal/BaseGoalScreen.tsx +++ b/src/goals/DefaultGoal/BaseGoalScreen.tsx @@ -1,5 +1,5 @@ import loadable from "@loadable/component"; -import { type ReactElement, useEffect } from "react"; +import { type ReactElement, useEffect, useState } from "react"; import { useNavigate } from "react-router"; import PageNotFound from "components/PageNotFound/component"; @@ -46,6 +46,8 @@ export default function LoadingGoalScreen(): ReactElement { ); const navigate = useNavigate(); + const [openDupsDialog, setOpenDupsDialog] = useState(false); + useEffect(() => { // Prevent getting stuck on loading screen when user clicks the back button. if (goalType === GoalType.Default) { @@ -53,14 +55,24 @@ export default function LoadingGoalScreen(): ReactElement { } }, [goalType, navigate]); + useEffect(() => { + if (goalType === GoalType.MergeDups && status !== GoalStatus.Completed) { + setOpenDupsDialog( + (prev) => prev || dataLoadStatus === DataLoadStatus.Loading + ); + } else { + setOpenDupsDialog(false); + } + }, [dataLoadStatus, goalType, status]); + return ( <> {status === GoalStatus.Loading ? : } - {goalType === GoalType.MergeDups && - status !== GoalStatus.Completed && - dataLoadStatus === DataLoadStatus.Loading && ( - - )} + {openDupsDialog && ( + + )} ); } diff --git a/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx index c5e038060b..0b28b7ca00 100644 --- a/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx +++ b/src/goals/MergeDuplicates/IdenticalDuplicatesDialog.tsx @@ -4,6 +4,7 @@ import { DialogActions, DialogContent, DialogTitle, + Stack, Typography, } from "@mui/material"; import { ReactElement, useEffect, useState } from "react"; @@ -20,7 +21,9 @@ import { Path } from "types/path"; // Threshold for warning about long processing time const LARGE_PROJECT_THRESHOLD = 1000; -export default function IdenticalDuplicatesDialog(): ReactElement { +export default function IdenticalDuplicatesDialog(props: { + loading?: boolean; +}): ReactElement { const dispatch = useAppDispatch(); const [open, setOpen] = useState(true); @@ -52,27 +55,35 @@ export default function IdenticalDuplicatesDialog(): ReactElement { {t("mergeDups.identicalCompleted.title")} - - {t("mergeDups.identicalCompleted.congratulations")} - - {hasDeferred && ( + - {t("mergeDups.identicalCompleted.hasDeferred")} - - )} - - {t("mergeDups.identicalCompleted.findingSimilar")} - - {frontierCount > LARGE_PROJECT_THRESHOLD && ( - - {t("mergeDups.identicalCompleted.warning")} + {t("mergeDups.identicalCompleted.congratulations")} - )} + + {hasDeferred && ( + + {t("mergeDups.identicalCompleted.hasDeferred")} + + )} + +
+ + {t("mergeDups.identicalCompleted.findingSimilar")} + + {frontierCount > LARGE_PROJECT_THRESHOLD && props.loading && ( + + {t("mergeDups.identicalCompleted.warning")} + + )} +
+ - + {props.loading && ( + + )} {hasDeferred && (