From 21eb376b4146878d0d2275024ddab678537e0ffd Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Wed, 21 Jan 2026 15:32:58 -0800 Subject: [PATCH] [PM-30906] Auto confirm nudge service fix and better nudge documentation (#18419) * [PM-30906] Refactor AutoConfirmNudgeService to be Browser specific and add additional documentation detailing when this is necessary * [PM-30906] Add README.md for custom nudge services --- .../src/popup/services/services.module.ts | 17 +- libs/angular/src/vault/index.ts | 6 +- .../services/custom-nudges-services/README.md | 204 ++++++++++++++++++ .../auto-confirm-nudge.service.ts | 15 +- .../new-account-nudge.service.ts | 12 +- .../services/default-single-nudge.service.ts | 8 +- .../vault/services/nudge-injection-tokens.ts | 19 ++ .../src/vault/services/nudges.service.ts | 16 +- 8 files changed, 280 insertions(+), 17 deletions(-) create mode 100644 libs/angular/src/vault/services/custom-nudges-services/README.md diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 06a021085ea..7b207f0fac1 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -27,8 +27,12 @@ import { WINDOW, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; -import { AUTOFILL_NUDGE_SERVICE } from "@bitwarden/angular/vault"; -import { SingleNudgeService } from "@bitwarden/angular/vault/services/default-single-nudge.service"; +import { + AUTOFILL_NUDGE_SERVICE, + AUTO_CONFIRM_NUDGE_SERVICE, + AutoConfirmNudgeService, +} from "@bitwarden/angular/vault"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { LoginComponentService, TwoFactorAuthComponentService, @@ -786,9 +790,14 @@ const safeProviders: SafeProvider[] = [ ], }), safeProvider({ - provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, + provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, useClass: BrowserAutofillNudgeService, - deps: [], + deps: [StateProvider, VaultProfileService, LogService], + }), + safeProvider({ + provide: AUTO_CONFIRM_NUDGE_SERVICE as SafeInjectionToken, + useClass: AutoConfirmNudgeService, + deps: [StateProvider, AutomaticUserConfirmationService], }), ]; diff --git a/libs/angular/src/vault/index.ts b/libs/angular/src/vault/index.ts index b9131338a45..a89be7bc2c4 100644 --- a/libs/angular/src/vault/index.ts +++ b/libs/angular/src/vault/index.ts @@ -1,4 +1,8 @@ // Note: Nudge related code is exported from `libs/angular` because it is consumed by multiple // `libs/*` packages. Exporting from the `libs/vault` package creates circular dependencies. export { NudgesService, NudgeStatus, NudgeType } from "./services/nudges.service"; -export { AUTOFILL_NUDGE_SERVICE } from "./services/nudge-injection-tokens"; +export { + AUTOFILL_NUDGE_SERVICE, + AUTO_CONFIRM_NUDGE_SERVICE, +} from "./services/nudge-injection-tokens"; +export { AutoConfirmNudgeService } from "./services/custom-nudges-services"; diff --git a/libs/angular/src/vault/services/custom-nudges-services/README.md b/libs/angular/src/vault/services/custom-nudges-services/README.md new file mode 100644 index 00000000000..f0759979de9 --- /dev/null +++ b/libs/angular/src/vault/services/custom-nudges-services/README.md @@ -0,0 +1,204 @@ +# Custom Nudge Services + +This folder contains custom implementations of `SingleNudgeService` that provide specialized logic for determining when nudges should be shown or dismissed. + +## Architecture Overview + +### Core Components + +- **`NudgesService`** (`../nudges.service.ts`) - The main service that components use to check nudge status and dismiss nudges +- **`SingleNudgeService`** - Interface that all nudge services implement +- **`DefaultSingleNudgeService`** - Base implementation that stores dismissed state in user state +- **Custom nudge services** - Specialized implementations with additional logic + +### How It Works + +1. Components call `NudgesService.showNudgeSpotlight$()` or `showNudgeBadge$()` with a `NudgeType` +2. `NudgesService` routes to the appropriate custom nudge service (or falls back to `DefaultSingleNudgeService`) +3. The custom service returns a `NudgeStatus` indicating if the badge/spotlight should be shown +4. Custom services can combine the persisted dismissed state with dynamic conditions (e.g., account age, vault contents) + +### NudgeStatus + +```typescript +type NudgeStatus = { + hasBadgeDismissed: boolean; // True if the badge indicator should be hidden + hasSpotlightDismissed: boolean; // True if the spotlight/callout should be hidden +}; +``` + +## Service Categories + +### Universal Services + +These services work on **all clients** (browser, web, desktop) and use `@Injectable({ providedIn: "root" })`. + +| Service | Purpose | +| --------------------------------- | ---------------------------------------------------------------------- | +| `NewAccountNudgeService` | Auto-dismisses after account is 30 days old | +| `NewItemNudgeService` | Checks cipher counts for "add first item" nudges | +| `HasItemsNudgeService` | Checks if vault has items | +| `EmptyVaultNudgeService` | Checks empty vault state | +| `AccountSecurityNudgeService` | Checks security settings (PIN, biometrics) | +| `VaultSettingsImportNudgeService` | Checks import status | +| `NoOpNudgeService` | Always returns dismissed (used as fallback for client specific nudges) | + +### Client-Specific Services + +These services require **platform-specific features** and must be explicitly registered in each client that supports them. + +| Service | Clients | Requires | +| ----------------------------- | ------------ | -------------------------------------- | +| `AutoConfirmNudgeService` | Browser only | `AutomaticUserConfirmationService` | +| `BrowserAutofillNudgeService` | Browser only | `BrowserApi` (lives in `apps/browser`) | + +## Adding a New Nudge Service + +### Step 1: Determine if Universal or Client-Specific + +**Universal** - If your service only depends on: + +- `StateProvider` +- Services available in all clients (e.g., `CipherService`, `OrganizationService`) + +**Client-Specific** - If your service depends on: + +- Browser APIs (`BrowserApi`, autofill services) +- Services only available in certain clients +- Platform-specific features + +### Step 2: Create the Service + +#### For Universal Services + +```typescript +// my-nudge.service.ts +import { Injectable } from "@angular/core"; +import { combineLatest, map, Observable } from "rxjs"; + +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { DefaultSingleNudgeService } from "../default-single-nudge.service"; +import { NudgeStatus, NudgeType } from "../nudges.service"; + +@Injectable({ providedIn: "root" }) +export class MyNudgeService extends DefaultSingleNudgeService { + constructor( + stateProvider: StateProvider, + private myDependency: MyDependency, // Must be available in all clients + ) { + super(stateProvider); + } + + nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { + return combineLatest([ + this.getNudgeStatus$(nudgeType, userId), // Gets persisted dismissed state + this.myDependency.someData$, + ]).pipe( + map(([persistedStatus, data]) => { + // Return dismissed if user already dismissed OR your condition is met + const autoDismiss = /* your logic */; + return { + hasBadgeDismissed: persistedStatus.hasBadgeDismissed || autoDismiss, + hasSpotlightDismissed: persistedStatus.hasSpotlightDismissed || autoDismiss, + }; + }), + ); + } +} +``` + +#### For Client-Specific Services + +```typescript +// my-client-specific-nudge.service.ts +import { Injectable } from "@angular/core"; +import { combineLatest, map, Observable } from "rxjs"; + +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { DefaultSingleNudgeService } from "../default-single-nudge.service"; +import { NudgeStatus, NudgeType } from "../nudges.service"; + +@Injectable() // NO providedIn: "root" +export class MyClientSpecificNudgeService extends DefaultSingleNudgeService { + constructor( + stateProvider: StateProvider, + private clientSpecificService: ClientSpecificService, + ) { + super(stateProvider); + } + + nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { + return combineLatest([ + this.getNudgeStatus$(nudgeType, userId), + this.clientSpecificService.someData$, + ]).pipe( + map(([persistedStatus, data]) => { + const autoDismiss = /* your logic */; + return { + hasBadgeDismissed: persistedStatus.hasBadgeDismissed || autoDismiss, + hasSpotlightDismissed: persistedStatus.hasSpotlightDismissed || autoDismiss, + }; + }), + ); + } +} +``` + +### Step 3: Add NudgeType + +Add your nudge type to `NudgeType` in `../nudges.service.ts`: + +```typescript +export const NudgeType = { + // ... existing types + MyNewNudge: "my-new-nudge", +} as const; +``` + +### Step 4: Register in NudgesService + +#### For Universal Services + +Add to `customNudgeServices` map in `../nudges.service.ts`: + +```typescript +private customNudgeServices: Partial> = { + // ... existing + [NudgeType.MyNewNudge]: inject(MyNudgeService), +}; +``` + +#### For Client-Specific Services + +1. **Add injection token** in `../nudge-injection-tokens.ts`: + +```typescript +export const MY_NUDGE_SERVICE = new InjectionToken("MyNudgeService"); +``` + +2. **Inject with optional** in `../nudges.service.ts`: + +```typescript +private myNudgeService = inject(MY_NUDGE_SERVICE, { optional: true }); + +private customNudgeServices = { + // ... existing + [NudgeType.MyNewNudge]: this.myNudgeService ?? this.noOpNudgeService, +}; +``` + +3. **Register in each supporting client** (e.g., `apps/browser/src/popup/services/services.module.ts`): + +```typescript +import { MY_NUDGE_SERVICE } from "@bitwarden/angular/vault"; + +safeProvider({ + provide: MY_NUDGE_SERVICE as SafeInjectionToken, + useClass: MyClientSpecificNudgeService, + deps: [StateProvider, ClientSpecificService], +}), +``` diff --git a/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts index 52fc87d7604..9fe843e50e0 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts @@ -1,15 +1,24 @@ -import { inject, Injectable } from "@angular/core"; +import { Injectable } from "@angular/core"; import { combineLatest, map, Observable } from "rxjs"; import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { StateProvider } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/user-core"; import { DefaultSingleNudgeService } from "../default-single-nudge.service"; import { NudgeType, NudgeStatus } from "../nudges.service"; -@Injectable({ providedIn: "root" }) +/** + * Browser specific nudge service for auto-confirm nudge. + */ +@Injectable() export class AutoConfirmNudgeService extends DefaultSingleNudgeService { - autoConfirmService = inject(AutomaticUserConfirmationService); + constructor( + stateProvider: StateProvider, + private autoConfirmService: AutomaticUserConfirmationService, + ) { + super(stateProvider); + } nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { return combineLatest([ diff --git a/libs/angular/src/vault/services/custom-nudges-services/new-account-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/new-account-nudge.service.ts index 39af9a2e4aa..8c18da8a103 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/new-account-nudge.service.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/new-account-nudge.service.ts @@ -1,10 +1,11 @@ -import { Injectable, inject } from "@angular/core"; +import { Injectable } from "@angular/core"; import { Observable, combineLatest, from, map, of } from "rxjs"; import { catchError } from "rxjs/operators"; import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { UserId } from "@bitwarden/common/types/guid"; +import { StateProvider } from "@bitwarden/state"; import { DefaultSingleNudgeService } from "../default-single-nudge.service"; import { NudgeStatus, NudgeType } from "../nudges.service"; @@ -18,8 +19,13 @@ const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; providedIn: "root", }) export class NewAccountNudgeService extends DefaultSingleNudgeService { - vaultProfileService = inject(VaultProfileService); - logService = inject(LogService); + constructor( + stateProvider: StateProvider, + private vaultProfileService: VaultProfileService, + private logService: LogService, + ) { + super(stateProvider); + } nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { const profileDate$ = from(this.vaultProfileService.getProfileCreationDate(userId)).pipe( diff --git a/libs/angular/src/vault/services/default-single-nudge.service.ts b/libs/angular/src/vault/services/default-single-nudge.service.ts index 8abc344c4a0..06c08371f41 100644 --- a/libs/angular/src/vault/services/default-single-nudge.service.ts +++ b/libs/angular/src/vault/services/default-single-nudge.service.ts @@ -1,4 +1,4 @@ -import { inject, Injectable } from "@angular/core"; +import { Injectable } from "@angular/core"; import { map, Observable } from "rxjs"; import { StateProvider } from "@bitwarden/common/platform/state"; @@ -22,7 +22,11 @@ export interface SingleNudgeService { providedIn: "root", }) export class DefaultSingleNudgeService implements SingleNudgeService { - stateProvider = inject(StateProvider); + protected stateProvider: StateProvider; + + constructor(stateProvider: StateProvider) { + this.stateProvider = stateProvider; + } protected getNudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { return this.stateProvider diff --git a/libs/angular/src/vault/services/nudge-injection-tokens.ts b/libs/angular/src/vault/services/nudge-injection-tokens.ts index 52a0838d356..43db5ec1dc6 100644 --- a/libs/angular/src/vault/services/nudge-injection-tokens.ts +++ b/libs/angular/src/vault/services/nudge-injection-tokens.ts @@ -2,6 +2,25 @@ import { InjectionToken } from "@angular/core"; import { SingleNudgeService } from "./default-single-nudge.service"; +/** + * Injection tokens for client specific nudge services. + * + * These services require platform-specific features and must be explicitly + * provided by each client that supports them. If not provided, NudgesService + * falls back to NoOpNudgeService. + * + * Client specific services should use constructor injection (not inject()) + * to maintain safeProvider type safety. + * + * Universal services use @Injectable({ providedIn: "root" }) and can use inject(). + */ + +/** Browser: Requires BrowserApi */ export const AUTOFILL_NUDGE_SERVICE = new InjectionToken( "AutofillNudgeService", ); + +/** Browser: Requires AutomaticUserConfirmationService */ +export const AUTO_CONFIRM_NUDGE_SERVICE = new InjectionToken( + "AutoConfirmNudgeService", +); diff --git a/libs/angular/src/vault/services/nudges.service.ts b/libs/angular/src/vault/services/nudges.service.ts index afd0d184d6e..a8a8feb073f 100644 --- a/libs/angular/src/vault/services/nudges.service.ts +++ b/libs/angular/src/vault/services/nudges.service.ts @@ -12,11 +12,10 @@ import { NewItemNudgeService, AccountSecurityNudgeService, VaultSettingsImportNudgeService, - AutoConfirmNudgeService, NoOpNudgeService, } from "./custom-nudges-services"; import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service"; -import { AUTOFILL_NUDGE_SERVICE } from "./nudge-injection-tokens"; +import { AUTOFILL_NUDGE_SERVICE, AUTO_CONFIRM_NUDGE_SERVICE } from "./nudge-injection-tokens"; export type NudgeStatus = { hasBadgeDismissed: boolean; @@ -63,12 +62,21 @@ export class NudgesService { // NoOp service that always returns dismissed private noOpNudgeService = inject(NoOpNudgeService); - // Optional Browser-specific service provided via injection token (not all clients have autofill) + // Client specific services (optional, via injection tokens) + // These services require platform-specific features and fallback to NoOpNudgeService if not provided + private autofillNudgeService = inject(AUTOFILL_NUDGE_SERVICE, { optional: true }); + private autoConfirmNudgeService = inject(AUTO_CONFIRM_NUDGE_SERVICE, { optional: true }); /** * Custom nudge services to use for specific nudge types * Each nudge type can have its own service to determine when to show the nudge + * + * NOTE: If a custom nudge service requires client specific services/features: + * 1. The custom nudge service must be provided via injection token and marked as optional. + * 2. The custom nudge service must be manually registered with that token in the client(s). + * + * See the README.md in the custom-nudge-services folder for more details on adding custom nudges. * @private */ private customNudgeServices: Partial> = { @@ -84,7 +92,7 @@ export class NudgesService { [NudgeType.NewIdentityItemStatus]: this.newItemNudgeService, [NudgeType.NewNoteItemStatus]: this.newItemNudgeService, [NudgeType.NewSshItemStatus]: this.newItemNudgeService, - [NudgeType.AutoConfirmNudge]: inject(AutoConfirmNudgeService), + [NudgeType.AutoConfirmNudge]: this.autoConfirmNudgeService ?? this.noOpNudgeService, }; /**