Implemented autofill match logic for duplicate detection, UI controls & output, and test coverage

2026-02-05 19:23:19 +00:00 · 2025-08-19 08:39:46 -07:00
parent a4e88cf2c7
commit 37248340bc
7 changed files with 149 additions and 78 deletions
--- 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
@@ -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>
--- 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
@@ -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 });
+  }
+}
--- 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
--- 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);
  }
 }
--- 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"
--- 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[];
 }
--- 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."
  }
 }