mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 00:46:45 +00:00
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:
parent
077bed154b
commit
b761efd899
6 changed files with 180 additions and 11 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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 = '';
|
||||
|
|
|
|||
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue