From 37248340bc1624380232d85c5a10bae4cb2ffdef Mon Sep 17 00:00:00 2001
From: John Harrington <84741727+harr1424@users.noreply.github.com>
Date: Tue, 19 Aug 2025 08:39:46 -0700
Subject: [PATCH] Implemented autofill match logic for duplicate detection, UI
 controls & output, and test coverage

---
 ...e-duplicate-warnings-dialog.component.html | 28 ++++++
 .../de-duplicate-warnings-dialog.component.ts | 37 ++++++++
 .../de-duplicate/de-duplicate.component.html  |  2 +-
 .../de-duplicate/de-duplicate.component.ts    | 51 ++++++-----
 .../duplicate-review-dialog.component.html    |  7 +-
 .../vault/services/de-duplicate.service.ts    | 86 +++++++++----------
 apps/web/src/locales/en/messages.json         | 16 ++--
 7 files changed, 149 insertions(+), 78 deletions(-)
 create mode 100644 apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.html
 create mode 100644 apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.ts

diff --git a/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.html b/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.html
new file mode 100644
index 00000000000..db9546993f7
--- /dev/null
+++ b/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.html
@@ -0,0 +1,28 @@
+<bit-simple-dialog>
+  <i bitDialogIcon class="bwi bwi-info-circle tw-text-3xl tw-text-info" aria-hidden="true"></i>
+
+  <span bitDialogTitle>{{ data.title }}</span>
+
+  <div bitDialogContent class="tw-text-left">
+    <ng-container *ngFor="let s of data.sections; let last = last">
+      <div class="tw-font-medium tw-mb-1">{{ s.title }}</div>
+      <ul class="tw-list-disc tw-pl-6 tw-mb-2">
+        <li *ngFor="let item of s.items">{{ item }}</li>
+      </ul>
+      <div *ngIf="s.help" class="tw-text-sm tw-text-secondary-700 tw-mb-3">{{ s.help }}</div>
+      <hr *ngIf="!last" class="tw-my-3" />
+    </ng-container>
+  </div>
+
+  <ng-container bitDialogFooter>
+    <button
+      id="de-duplicate-warnings-dialog_button_ok"
+      type="button"
+      bitButton
+      buttonType="primary"
+      (click)="dialogRef.close(true)"
+    >
+      {{ okText }}
+    </button>
+  </ng-container>
+</bit-simple-dialog>
diff --git a/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.ts b/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.ts
new file mode 100644
index 00000000000..90a218633e9
--- /dev/null
+++ b/apps/web/src/app/tools/de-duplicate/de-duplicate-warnings-dialog.component.ts
@@ -0,0 +1,37 @@
+import { CommonModule } from "@angular/common";
+import { Component, Inject } from "@angular/core";
+
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import {
+  ButtonModule,
+  DialogModule,
+  DialogRef,
+  DIALOG_DATA,
+  DialogService,
+} from "@bitwarden/components";
+
+@Component({
+  selector: "app-de-duplicate-warnings-dialog",
+  standalone: true,
+  imports: [CommonModule, DialogModule, ButtonModule],
+  templateUrl: "./de-duplicate-warnings-dialog.component.html",
+})
+export class DeDuplicateWarningsDialogComponent {
+  okText: string;
+
+  constructor(
+    public dialogRef: DialogRef<boolean>,
+    @Inject(DIALOG_DATA)
+    public data: { title: string; sections: { title: string; items: string[]; help?: string }[] },
+    private i18nService: I18nService,
+  ) {
+    this.okText = this.i18nService.t("ok");
+  }
+
+  static open(
+    dialogService: DialogService,
+    data: { title: string; sections: { title: string; items: string[]; help?: string }[] },
+  ) {
+    return dialogService.open<boolean, typeof data>(DeDuplicateWarningsDialogComponent, { data });
+  }
+}
diff --git a/apps/web/src/app/tools/de-duplicate/de-duplicate.component.html b/apps/web/src/app/tools/de-duplicate/de-duplicate.component.html
index c3b38a65882..df9fcd24962 100644
--- a/apps/web/src/app/tools/de-duplicate/de-duplicate.component.html
+++ b/apps/web/src/app/tools/de-duplicate/de-duplicate.component.html
@@ -54,7 +54,7 @@
 
       <div class="tw-flex tw-gap-2">
         <button
-          id="de-duplicate__button__find-duplicates"
+          id="de-duplicate_button_find-duplicates"
           type="button"
           buttonType="primary"
           bitButton
diff --git a/apps/web/src/app/tools/de-duplicate/de-duplicate.component.ts b/apps/web/src/app/tools/de-duplicate/de-duplicate.component.ts
index 6e9d183edf7..d91cdf23758 100644
--- a/apps/web/src/app/tools/de-duplicate/de-duplicate.component.ts
+++ b/apps/web/src/app/tools/de-duplicate/de-duplicate.component.ts
@@ -1,5 +1,5 @@
 import { CommonModule } from "@angular/common";
-import { ChangeDetectorRef, Component, Inject } from "@angular/core";
+import { ChangeDetectorRef, Component, Inject, LOCALE_ID } from "@angular/core";
 import { FormsModule } from "@angular/forms";
 import { firstValueFrom } from "rxjs";
 
@@ -24,6 +24,8 @@ import {
   type DuplicateOperationWarnings,
 } from "../../vault/services/de-duplicate.service";
 
+import { DeDuplicateWarningsDialogComponent } from "./de-duplicate-warnings-dialog.component";
+
 @Component({
   selector: "app-de-duplicate",
   standalone: true,
@@ -62,6 +64,7 @@ export class DeDuplicateComponent {
 
   constructor(
     @Inject(DeDuplicateService) private deDuplicateService: DeDuplicateService,
+    @Inject(LOCALE_ID) private locale: string,
     private i18nService: I18nService,
     private accountService: AccountService,
     private cdr: ChangeDetectorRef,
@@ -124,14 +127,19 @@ export class DeDuplicateComponent {
           warningParts.push(this.i18nService.t(key, w.permissionDeniedCount));
         }
         const title = this.i18nService.t("duplicateWarningsTitle");
+        const message = (Intl as any).ListFormat
+          ? new Intl.ListFormat(this.locale, { style: "long", type: "conjunction" }).format(
+              warningParts,
+            )
+          : warningParts.join(", ");
         this.warnings = w;
         this.warningsCallout = {
           type: "warning",
           title,
-          message: warningParts.join(" \u2022 "),
+          message,
           detailsButtonText:
             (w.unparseableUris?.length || 0) > 0 ||
-            (w.exactFallbackSamples?.length || 0) > 0 ||
+            (w.exactFallbackUris?.length || 0) > 0 ||
             (w.permissionDeniedNames?.length || 0) > 0
               ? this.i18nService.t("duplicateWarningsDetailsButton")
               : undefined,
@@ -152,29 +160,32 @@ export class DeDuplicateComponent {
 
   async openWarningsDetails() {
     const w = this.warnings;
-    const bodyLines: string[] = [];
+    const sections: { title: string; items: string[]; help?: string }[] = [];
     if (w?.unparseableUris?.length) {
-      bodyLines.push(this.i18nService.t("duplicateWarningsDialogUnparseableTitle"));
-      bodyLines.push(...w.unparseableUris.slice(0, 10).map((d) => `• ${d}`));
-      bodyLines.push("");
+      sections.push({
+        title: this.i18nService.t("duplicateWarningsDialogUnparseableTitle"),
+        items: w.unparseableUris.slice(0, 10),
+      });
     }
-    if (w?.exactFallbackSamples?.length) {
-      bodyLines.push(this.i18nService.t("duplicateWarningsDialogExactSamplesTitle"));
-      bodyLines.push(...w.exactFallbackSamples.slice(0, 10).map((d) => `• ${d}`));
-      bodyLines.push("");
+    if (w?.exactFallbackUris?.length) {
+      sections.push({
+        title: this.i18nService.t("duplicateWarningsDialogExactSamplesTitle"),
+        items: w.exactFallbackUris.slice(0, 10),
+        help: this.i18nService.t("duplicateWarningsDialogExactHelp"),
+      });
     }
     if (w?.permissionDeniedNames?.length) {
-      bodyLines.push(this.i18nService.t("duplicateWarningsDialogPermissionDeniedTitle"));
-      bodyLines.push(...w.permissionDeniedNames.slice(0, 10).map((n) => `• ${n}`));
-      bodyLines.push("");
+      sections.push({
+        title: this.i18nService.t("duplicateWarningsDialogPermissionDeniedTitle"),
+        items: w.permissionDeniedNames.slice(0, 10),
+        help: this.i18nService.t("duplicateWarningsDialogPermHelp"),
+      });
     }
-    bodyLines.push(this.i18nService.t("duplicateWarningsDialogExactHelp"));
-    bodyLines.push(this.i18nService.t("duplicateWarningsDialogPermHelp"));
-    await this.dialogService.openSimpleDialog({
+
+    const ref = DeDuplicateWarningsDialogComponent.open(this.dialogService, {
       title: this.i18nService.t("duplicateWarningsDialogTitle"),
-      content: bodyLines.join("\n"),
-      type: "info",
-      acceptButtonText: { key: "ok" },
+      sections,
     });
+    await firstValueFrom(ref.closed);
   }
 }
diff --git a/apps/web/src/app/tools/de-duplicate/duplicate-review-dialog.component.html b/apps/web/src/app/tools/de-duplicate/duplicate-review-dialog.component.html
index ae1d814e1cd..430b7473584 100644
--- a/apps/web/src/app/tools/de-duplicate/duplicate-review-dialog.component.html
+++ b/apps/web/src/app/tools/de-duplicate/duplicate-review-dialog.component.html
@@ -55,9 +55,8 @@
               >
                 <div class="tw-flex tw-items-start tw-gap-2">
                   <input
-                    id="duplicate-review-dialog__checkbox__select-{{ row.id }}"
+                    id="duplicate-review-dialog_input_select-{{ row.id }}"
                     type="checkbox"
-                    [disabled]="row.id === set.ciphers[0]?.id"
                     [ngModel]="selection[row.id]"
                     (ngModelChange)="onSelectChange(row.id, $event)"
                     [ngModelOptions]="{ standalone: true }"
@@ -101,7 +100,7 @@
   </div>
   <ng-container bitDialogFooter>
     <button
-      id="duplicate-review-dialog__button__cancel"
+      id="duplicate-review-dialog_button_cancel"
       bitButton
       buttonType="secondary"
       type="button"
@@ -110,7 +109,7 @@
       {{ "cancel" | i18n }}
     </button>
     <button
-      id="duplicate-review-dialog__button__delete-selected"
+      id="duplicate-review-dialog_button_delete-selected"
       bitButton
       buttonType="danger"
       type="button"
diff --git a/apps/web/src/app/vault/services/de-duplicate.service.ts b/apps/web/src/app/vault/services/de-duplicate.service.ts
index af8d85fe7a1..10a3a39a55d 100644
--- a/apps/web/src/app/vault/services/de-duplicate.service.ts
+++ b/apps/web/src/app/vault/services/de-duplicate.service.ts
@@ -25,7 +25,7 @@ export interface DuplicateOperationWarnings {
   unparseableUriCount: number;
   permissionDeniedCount: number;
   unparseableUris?: string[];
-  exactFallbackSamples?: string[];
+  exactFallbackUris?: string[];
   permissionDeniedNames?: string[];
 }
 
@@ -34,6 +34,15 @@ interface DuplicateSet {
   ciphers: CipherView[];
 }
 
+interface WarningAccumulator {
+  exactFallbackCount: number;
+  unparseableUriCount: number;
+  permissionDeniedCount: number;
+  unparseableUris: string[];
+  exactFallbackUris: string[];
+  permissionDeniedNames: string[];
+}
+
 export type UriMatchStrategy = "Base" | "Hostname" | "Host" | "Exact";
 
 interface ParsedUri {
@@ -71,6 +80,30 @@ export class DeDuplicateService {
    */
   private static readonly DEFAULT_URI_STRATEGY: UriMatchStrategy = "Base";
 
+  private createWarningAccumulator(): WarningAccumulator {
+    return {
+      exactFallbackCount: 0,
+      unparseableUriCount: 0,
+      permissionDeniedCount: 0,
+      unparseableUris: [],
+      exactFallbackUris: [],
+      permissionDeniedNames: [],
+    };
+  }
+
+  private toWarningsResult(acc: WarningAccumulator): DuplicateOperationWarnings {
+    return {
+      exactFallbackCount: acc.exactFallbackCount,
+      unparseableUriCount: acc.unparseableUriCount,
+      permissionDeniedCount: acc.permissionDeniedCount,
+      unparseableUris: acc.unparseableUris.length ? acc.unparseableUris : undefined,
+      exactFallbackUris: acc.exactFallbackUris.length ? acc.exactFallbackUris : undefined,
+      permissionDeniedNames: acc.permissionDeniedNames.length
+        ? acc.permissionDeniedNames
+        : undefined,
+    };
+  }
+
   /**
    * Main entry point to find and handle duplicate ciphers for a given user.
    * @param userId The ID of the current user.
@@ -123,13 +156,13 @@ export class DeDuplicateService {
   ): DuplicateSet[] {
     const uriBuckets = new Map<string, CipherView[]>();
     const nameBuckets = new Map<string, CipherView[]>();
-    const nameOnlyBuckets = new Map<string, CipherView[]>(); // used in edge cases when no useername is present for a login
+    const nameOnlyBuckets = new Map<string, CipherView[]>(); // used in edge cases when no username is present for a login
 
-    // DuplicateSet will be created to hold duplicate login ciphers once two matching ci\hers appear in a bucket
+    // DuplicateSet will be created to hold duplicate login ciphers once two matching ciphers appear in a bucket
     const duplicateSets: DuplicateSet[] = [];
 
     // Used to prevent redundant groupings for a given display key ['username+uri', 'username+name']
-    // Note that matchings based solely on name (no username for login) will share the 'username+name' display key
+    // Note that matches based solely on name (no username for login) will share the 'username+name' display key
     const setByDisplayKey = new Map<string, DuplicateSet>();
 
     /**
@@ -152,7 +185,7 @@ export class DeDuplicateService {
       const username = cipher.login?.username?.trim() || "";
 
       // Match URIs when username is present
-      // Almost all dudplicates can be identified by matching username and URI - other cases handled in next block
+      // Almost all duplicates can be identified by matching username and URI - other cases handled in next block
       if (username) {
         const uris = this.extractUriStrings(cipher);
         if (uris.length > 0) {
@@ -280,7 +313,6 @@ export class DeDuplicateService {
       if (!parsed) {
         continue;
       }
-      // Count unparseable URIs and skip them
       if (parsed.unparseable) {
         if (warningAccumulator) {
           warningAccumulator.unparseableUriCount++;
@@ -313,8 +345,8 @@ export class DeDuplicateService {
         case "Exact": {
           if (parsed.approximate && warningAccumulator) {
             warningAccumulator.exactFallbackCount++;
-            if (warningAccumulator.exactFallbackSamples.length < 10) {
-              warningAccumulator.exactFallbackSamples.push(parsed.original);
+            if (warningAccumulator.exactFallbackUris.length < 10) {
+              warningAccumulator.exactFallbackUris.push(parsed.original);
             }
           }
           const exact = this.getExactUrlKey(parsed);
@@ -383,7 +415,7 @@ export class DeDuplicateService {
       };
     } catch {
       // Fallback manual authority extraction sufficient for Hostname/Base/Host strategies.
-      // We mark the result as `approximate`; getUriKeysForStrategy counts a fallback warning
+      // Mark the result as `approximate`; getUriKeysForStrategy counts a fallback warning
       // only when the selected strategy is "Exact". Other strategies continue normally.
       const authorityMatch = toParse.match(/^[a-z][a-z0-9+.-]*:\/\/([^/?#]+)/i);
       if (!authorityMatch) {
@@ -615,40 +647,4 @@ export class DeDuplicateService {
     // 6. Return summary to display in a callout by the caller.
     return { trashed: toSoftDelete.length, permanentlyDeleted: toPermanentlyDelete.length };
   }
-
-  // ------------------------
-  // Warnings accumulator helpers
-  // ------------------------
-  private createWarningAccumulator(): WarningAccumulator {
-    return {
-      exactFallbackCount: 0,
-      unparseableUriCount: 0,
-      permissionDeniedCount: 0,
-      unparseableUris: [],
-      exactFallbackSamples: [],
-      permissionDeniedNames: [],
-    };
-  }
-
-  private toWarningsResult(acc: WarningAccumulator): DuplicateOperationWarnings {
-    return {
-      exactFallbackCount: acc.exactFallbackCount,
-      unparseableUriCount: acc.unparseableUriCount,
-      permissionDeniedCount: acc.permissionDeniedCount,
-      unparseableUris: acc.unparseableUris.length ? acc.unparseableUris : undefined,
-      exactFallbackSamples: acc.exactFallbackSamples.length ? acc.exactFallbackSamples : undefined,
-      permissionDeniedNames: acc.permissionDeniedNames.length
-        ? acc.permissionDeniedNames
-        : undefined,
-    };
-  }
-}
-
-interface WarningAccumulator {
-  exactFallbackCount: number;
-  unparseableUriCount: number;
-  permissionDeniedCount: number;
-  unparseableUris: string[];
-  exactFallbackSamples: string[];
-  permissionDeniedNames: string[];
 }
diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json
index e2f30419105..2ef37c6c5a6 100644
--- a/apps/web/src/locales/en/messages.json
+++ b/apps/web/src/locales/en/messages.json
@@ -516,7 +516,7 @@
     "description": "Domain name. Example: website.com"
   },
   "hostName" : {
-    "message": "Host name",
+    "message": "Hostname (include subdomain)",
     "description": "Host name includes subdomain but excludes port. Example: subdomain.website.com"
   },
   "host": {
@@ -11101,9 +11101,9 @@
     "description": "Warning indicating that due to parsing limitations, exact URI matching could not be guaranteed for some items."
   },
   "duplicateExactFallbackPlural": {
-    "message": "Exact matching for for $COUNT$ URIs.",
-    "placeholders": {
-      "count": { "content": "$1", "example": "3" }
+    "message": "Exact matching failed for $COUNT$ URIs.",
+      "placeholders": {
+        "count": { "content": "$1", "example": "3" }
     },
     "description": "Plural form of duplicateExactFallback."
   },
@@ -11140,7 +11140,7 @@
     "description": "Title of the dialog showing details for de-duplication warnings."
   },
   "duplicateWarningsDialogExactHelp": {
-    "message": "Fix invalid URIs or try a less strict match strategy (Host/Hostname/Base).",
+    "message": "Fix invalid URIs or try a less strict match strategy (Base/Hostname/Host).",
     "description": "Helper sentence shown in the warnings details dialog to guide users about URI-related issues."
   },
   "duplicateWarningsDialogPermHelp": {
@@ -11152,15 +11152,15 @@
     "description": "Label for a button that opens a dialog with more information about de-duplication warnings."
   },
   "duplicateWarningsDialogUnparseableTitle": {
-    "message": "Unparseable URIs",
+    "message": "Unparseable URIs (limit ten)",
     "description": "Section title above the list of unparseable URIs."
   },
   "duplicateWarningsDialogExactSamplesTitle": {
-    "message": "Exact matching fallbacks (sample)",
+    "message": "Exact matching fallbacks (limit ten)",
     "description": "Section title above the sample of URIs where exact matching could not be guaranteed."
   },
   "duplicateWarningsDialogPermissionDeniedTitle": {
-    "message": "Items not deleted (permission denied)",
+    "message": "Items not deleted (permission denied - limit ten)",
     "description": "Section title above the list of cipher names that could not be deleted due to permissions."
   }
 }