fix(sync): warn before destructive SYNC_IMPORT actions

Previously the 'Server Already Has Data' dialog described a destructive
SYNC_IMPORT as a 'merge' with a primary-colored 'Upload Local Data'
button — leading users to clobber syncing devices' data. The decrypt-
error 'Overwrite Remote' button had similarly understated copy and no
final confirmation gate.

- Rewrite D_SERVER_MIGRATION_CONFIRM body to call out 'overwrite' /
  'replace' / 'other devices'; affirmative button is now 'Replace
  Server Data' with color=warn.
- Rewrite D_DECRYPT_ERROR P3 + button label to make cross-device
  blast radius explicit.
- Gate updatePWAndForceUpload behind a confirmDialog with a stronger
  warning string.
- Add component spec for the migration dialog as a regression guard.
This commit is contained in:
Johannes Millan 2026-04-28 15:46:08 +02:00
parent 077bed154b
commit b761efd899
6 changed files with 180 additions and 11 deletions

View file

@ -68,7 +68,20 @@ describe('DialogHandleDecryptErrorComponent', () => {
});
describe('updatePWAndForceUpload()', () => {
it('should update password, clear field, and close with isForceUpload', async () => {
let originalConfirm: typeof window.confirm;
let confirmReturn: boolean;
beforeEach(() => {
originalConfirm = window.confirm;
confirmReturn = true;
window.confirm = (() => confirmReturn) as typeof window.confirm;
});
afterEach(() => {
window.confirm = originalConfirm;
});
it('should update password, clear field, and close with isForceUpload when confirmed', async () => {
component.passwordVal = 'new-password';
mockSyncConfigService.updateEncryptionPassword.and.resolveTo();
@ -81,6 +94,17 @@ describe('DialogHandleDecryptErrorComponent', () => {
expect(mockDialogRef.close).toHaveBeenCalledWith({ isForceUpload: true });
});
it('should abort without changes when user cancels confirmation', async () => {
confirmReturn = false;
component.passwordVal = 'new-password';
await component.updatePWAndForceUpload();
expect(mockSyncConfigService.updateEncryptionPassword).not.toHaveBeenCalled();
expect(mockDialogRef.close).not.toHaveBeenCalled();
expect(component.passwordVal).toBe('new-password');
});
it('should show error snack and not close on failure', async () => {
component.passwordVal = 'new-password';
mockSyncConfigService.updateEncryptionPassword.and.rejectWith(

View file

@ -5,6 +5,7 @@ import {
MatDialogRef,
MatDialogTitle,
} from '@angular/material/dialog';
import { TranslateService } from '@ngx-translate/core';
import { T } from '../../../t.const';
import { MatFormField, MatLabel } from '@angular/material/form-field';
import { MatInput } from '@angular/material/input';
@ -15,6 +16,7 @@ import { TranslatePipe } from '@ngx-translate/core';
import { SyncConfigService } from '../sync-config.service';
import { SnackService } from '../../../core/snack/snack.service';
import { SyncLog } from '../../../core/log';
import { confirmDialog } from '../../../util/native-dialogs';
@Component({
selector: 'dialog-handle-decrypt-error',
@ -37,6 +39,7 @@ import { SyncLog } from '../../../core/log';
export class DialogHandleDecryptErrorComponent {
private _syncConfigService = inject(SyncConfigService);
private _snackService = inject(SnackService);
private _translateService = inject(TranslateService);
private _matDialogRef =
inject<MatDialogRef<DialogHandleDecryptErrorComponent>>(MatDialogRef);
@ -45,6 +48,9 @@ export class DialogHandleDecryptErrorComponent {
passwordVal: string = '';
async updatePWAndForceUpload(): Promise<void> {
if (!confirmDialog(this._translateService.instant(T.F.SYNC.C.DECRYPT_OVERWRITE))) {
return;
}
try {
await this._syncConfigService.updateEncryptionPassword(this.passwordVal);
this.passwordVal = '';

View file

@ -4,7 +4,7 @@
</h1>
<mat-dialog-content>
<p>{{ T.F.SYNC.D_SERVER_MIGRATION_CONFIRM.MESSAGE | translate }}</p>
<p [innerHTML]="T.F.SYNC.D_SERVER_MIGRATION_CONFIRM.MESSAGE | translate"></p>
</mat-dialog-content>
<mat-dialog-actions align="end">
@ -16,10 +16,10 @@
</button>
<button
(click)="close(true)"
color="primary"
color="warn"
mat-stroked-button
>
<mat-icon aria-hidden="true">cloud_upload</mat-icon>
<mat-icon aria-hidden="true">warning</mat-icon>
{{ T.F.SYNC.D_SERVER_MIGRATION_CONFIRM.CONFIRM | translate }}
</button>
</mat-dialog-actions>

View file

@ -0,0 +1,137 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { MatDialogRef } from '@angular/material/dialog';
import {
TranslateModule,
TranslateLoader,
TranslateService,
TranslationObject,
} from '@ngx-translate/core';
import { Observable, of } from 'rxjs';
import { DialogServerMigrationConfirmComponent } from './dialog-server-migration-confirm.component';
import enTranslations from '../../../../assets/i18n/en.json';
/**
* Regression guard for the destructive-replace dialog wording.
*
* The original copy advertised this destructive SYNC_IMPORT action as a
* "merge" with a primary-colored "Upload Local Data" button which led at
* least one user (johannes, 2026-04-27) to clobber a syncing device's data
* thinking they were merging it with another device's. The dialog must:
* - Never use "merge" in its body.
* - Mention "overwrite" or "replace" so the destructive intent is plain.
* - Mention "other device" so the cross-device blast radius is visible.
* - Render the affirmative button with color="warn" (not "primary").
* - Label the affirmative button "Replace…" (not "Upload Local Data").
*/
class JsonTranslateLoader implements TranslateLoader {
getTranslation(): Observable<TranslationObject> {
return of(enTranslations as TranslationObject);
}
}
describe('DialogServerMigrationConfirmComponent', () => {
let fixture: ComponentFixture<DialogServerMigrationConfirmComponent>;
let mockDialogRef: jasmine.SpyObj<MatDialogRef<DialogServerMigrationConfirmComponent>>;
beforeEach(async () => {
mockDialogRef = jasmine.createSpyObj('MatDialogRef', ['close'], {
disableClose: false,
});
await TestBed.configureTestingModule({
imports: [
DialogServerMigrationConfirmComponent,
NoopAnimationsModule,
TranslateModule.forRoot({
loader: { provide: TranslateLoader, useClass: JsonTranslateLoader },
}),
],
providers: [{ provide: MatDialogRef, useValue: mockDialogRef }],
}).compileComponents();
const translate = TestBed.inject(TranslateService);
translate.use('en');
fixture = TestBed.createComponent(DialogServerMigrationConfirmComponent);
fixture.detectChanges();
await fixture.whenStable();
fixture.detectChanges();
});
describe('body copy', () => {
let bodyText: string;
beforeEach(() => {
bodyText =
fixture.nativeElement
.querySelector('mat-dialog-content')
?.textContent?.toLowerCase() ?? '';
});
it('does not call the action a "merge" (the original misleading phrasing)', () => {
// Original copy: "Would you like to upload your local data to merge with it?"
// The fixed copy is allowed to *deny* a merge (e.g. "This is not a merge"),
// so just check that no positive merge-marketing phrase remains.
expect(bodyText).not.toMatch(/merge with|to merge|will merge/);
});
it('uses destructive language ("overwrite" or "replace")', () => {
expect(bodyText).toMatch(/overwrite|replace/);
});
it('warns about cross-device impact ("other device")', () => {
expect(bodyText).toMatch(/other device/);
});
});
describe('action buttons', () => {
let buttons: HTMLButtonElement[];
beforeEach(() => {
buttons = Array.from(
fixture.nativeElement.querySelectorAll('mat-dialog-actions button'),
);
});
it('renders exactly two action buttons', () => {
expect(buttons.length).toBe(2);
});
it('affirmative button is labelled "Replace…", not "Upload Local Data"', () => {
const affirmative = buttons[1];
const label = affirmative.textContent?.toLowerCase() ?? '';
expect(label).toContain('replace');
expect(label).not.toContain('upload local data');
});
it('affirmative button uses warn color (not primary)', () => {
const affirmative = buttons[1];
expect(affirmative.getAttribute('color')).toBe('warn');
});
it('cancel button has no color (per cancel-button-color rule)', () => {
const cancel = buttons[0];
expect(cancel.getAttribute('color')).toBeNull();
});
});
describe('close behavior', () => {
it('cancel button closes with false', () => {
const cancel = fixture.nativeElement.querySelectorAll(
'mat-dialog-actions button',
)[0] as HTMLButtonElement;
cancel.click();
expect(mockDialogRef.close).toHaveBeenCalledWith(false);
});
it('affirmative button closes with true', () => {
const affirmative = fixture.nativeElement.querySelectorAll(
'mat-dialog-actions button',
)[1] as HTMLButtonElement;
affirmative.click();
expect(mockDialogRef.close).toHaveBeenCalledWith(true);
});
});
});

View file

@ -1061,6 +1061,7 @@ const T = {
BTN_SYNC_NOW: 'F.SYNC.BTN_SYNC_NOW',
C: {
FORCE_UPLOAD: 'F.SYNC.C.FORCE_UPLOAD',
DECRYPT_OVERWRITE: 'F.SYNC.C.DECRYPT_OVERWRITE',
},
D_AUTH_CODE: {
FOLLOW_LINK: 'F.SYNC.D_AUTH_CODE.FOLLOW_LINK',

View file

@ -1043,7 +1043,8 @@
"BTN_RESTORE_FROM_HISTORY": "Restore from History",
"BTN_SYNC_NOW": "Sync Now",
"C": {
"FORCE_UPLOAD": "Forcing an upload of your data could lead to data loss. Are you sure you want to continue?"
"FORCE_UPLOAD": "This will REPLACE all data on the server and on every other device with this device's local copy. Any unsynced changes on those devices will be permanently lost. Continue?",
"DECRYPT_OVERWRITE": "This will REPLACE the encrypted data on the server with this device's local copy and re-encrypt it with the new password. All other devices will need to re-enter the password and download the replaced data. Any unsynced changes on those devices will be permanently lost. Continue?"
},
"D_AUTH_CODE": {
"FOLLOW_LINK": "Please open the following link and copy the auth code provided there into the input field below.",
@ -1092,11 +1093,11 @@
"UNKNOWN_ERROR": "Unknown Sync Error: {{err}}"
},
"D_DECRYPT_ERROR": {
"BTN_OVER_WRITE_REMOTE": "Use This Password & Overwrite Remote",
"BTN_OVER_WRITE_REMOTE": "Overwrite Server & Other Devices",
"CHANGE_PW_AND_DECRYPT": "Retry Decrypt",
"P1": "Your remote data is encrypted and decryption failed.",
"P2_FORGOT_PW": "If you <strong>forgot your password</strong>, enter the correct password and click \"Retry Decrypt\".",
"P3_PW_CHANGED": "If you <strong>changed your password on another device</strong>, enter your new password and click \"Overwrite Remote\". This will upload your local data to the server.",
"P3_PW_CHANGED": "If you <strong>changed your password on another device</strong>, enter your new password and click <strong>Overwrite Server & Other Devices</strong>. ⚠️ This is destructive: it replaces ALL data on the server and on your other devices with this device's local copy. Any unsynced changes on those devices will be lost.",
"PASSWORD": "Password",
"TITLE": "Decryption Failed"
},
@ -1159,10 +1160,10 @@
"WARNING": "This will replace ALL your current data with the selected backup. This action cannot be undone!"
},
"D_SERVER_MIGRATION_CONFIRM": {
"CONFIRM": "Upload Local Data",
"MESSAGE": "This sync server already contains data. Would you like to upload your local data to merge with it? If you skip this, some of your local changes may not sync.",
"SKIP": "Skip",
"TITLE": "Server Already Has Data"
"CONFIRM": "Replace Server Data",
"MESSAGE": "This sync server already contains data — likely from your other devices. Choosing <strong>Replace Server Data</strong> will <strong>overwrite all data on the server and on every other device</strong> with this device's local copy. This is <strong>not a merge</strong>. Choose <strong>Cancel</strong> to download the server's data instead (recommended unless you're sure this device has the newest version).",
"SKIP": "Cancel",
"TITLE": "Server Already Contains Data"
},
"D_SYNC_IMPORT_CONFLICT": {
"FILTERED_CHANGES": "Affected changes",