From dec7840baf012244cf47c48381632a34f693f60e Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Sun, 4 Jul 2021 20:14:54 -0400 Subject: [PATCH 01/13] feat: show a confirmation dialog when signing out and there are unsaved changes --- packages/snjs/lib/application.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/snjs/lib/application.ts b/packages/snjs/lib/application.ts index 27359ae75..657053117 100644 --- a/packages/snjs/lib/application.ts +++ b/packages/snjs/lib/application.ts @@ -1311,10 +1311,25 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR } public async signOut(): Promise { - await this.credentialService.signOut(); - await this.notifyEvent(ApplicationEvent.SignedOut); - await this.prepareForDeinit(); - this.deinit(DeinitSource.SignOut); + const performSignOut = async () => { + await this.credentialService.signOut(); + await this.notifyEvent(ApplicationEvent.SignedOut); + await this.prepareForDeinit(); + this.deinit(DeinitSource.SignOut); + }; + + const dirtyItems = this.itemManager.getDirtyItems(); + if (dirtyItems.length > 0) { + const didConfirm = await this.alertService.confirm( + `There are ${dirtyItems.length} unsynced items. Are you sure you want to sign out?` + ); + + if (didConfirm) { + await performSignOut(); + } + } else { + await performSignOut(); + } } public async validateAccountPassword(password: string): Promise { From 5992dc945766837d9ca93ab6a88d2fccc6812b54 Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Sun, 4 Jul 2021 20:15:24 -0400 Subject: [PATCH 02/13] chore: increment version --- packages/snjs/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snjs/package.json b/packages/snjs/package.json index d0e1663b4..fad911027 100644 --- a/packages/snjs/package.json +++ b/packages/snjs/package.json @@ -1,6 +1,6 @@ { "name": "@standardnotes/snjs", - "version": "2.7.11", + "version": "2.7.12", "engines": { "node": ">=14.0.0 <16.0.0" }, From bb28b2e0a8602fce4d26ebd53923de60092513bf Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Sun, 4 Jul 2021 20:23:25 -0400 Subject: [PATCH 03/13] fix: force sign out when session is revoked --- packages/snjs/lib/application.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/snjs/lib/application.ts b/packages/snjs/lib/application.ts index 657053117..1bffc0173 100644 --- a/packages/snjs/lib/application.ts +++ b/packages/snjs/lib/application.ts @@ -1310,7 +1310,7 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR ); } - public async signOut(): Promise { + public async signOut(force = false): Promise { const performSignOut = async () => { await this.credentialService.signOut(); await this.notifyEvent(ApplicationEvent.SignedOut); @@ -1318,6 +1318,11 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR this.deinit(DeinitSource.SignOut); }; + if (force) { + await performSignOut(); + return; + } + const dirtyItems = this.itemManager.getDirtyItems(); if (dirtyItems.length > 0) { const didConfirm = await this.alertService.confirm( @@ -1651,7 +1656,7 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR case SessionEvent.Revoked: { /** Keep a reference to the soon-to-be-cleared alertService */ const alertService = this.alertService; - await this.signOut(); + await this.signOut(true); void alertService.alert(SessionStrings.CurrentSessionRevoked); break; } From 86decb9d5407ec2ed81831f2e1a5eb0de230b0f8 Mon Sep 17 00:00:00 2001 From: Johnny A <5891646+johnny243@users.noreply.github.com> Date: Mon, 5 Jul 2021 09:49:03 -0400 Subject: [PATCH 04/13] Update packages/snjs/lib/application.ts Co-authored-by: Mo Bitar --- packages/snjs/lib/application.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snjs/lib/application.ts b/packages/snjs/lib/application.ts index 1bffc0173..b868fb227 100644 --- a/packages/snjs/lib/application.ts +++ b/packages/snjs/lib/application.ts @@ -1326,7 +1326,7 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR const dirtyItems = this.itemManager.getDirtyItems(); if (dirtyItems.length > 0) { const didConfirm = await this.alertService.confirm( - `There are ${dirtyItems.length} unsynced items. Are you sure you want to sign out?` + `There are ${dirtyItems.length} items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` ); if (didConfirm) { From 086f442dde804acf918fd14334383cc3c52160ee Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 10:24:21 -0400 Subject: [PATCH 05/13] test(wip): application signOut --- packages/snjs/test/lib/application.test.js | 60 +++++++++++++++++++ .../snjs/test/setup/snjs/deviceInterface.ts | 4 +- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 packages/snjs/test/lib/application.test.js diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js new file mode 100644 index 000000000..fad6ea9be --- /dev/null +++ b/packages/snjs/test/lib/application.test.js @@ -0,0 +1,60 @@ +import { + Platform, + Environment, + DeinitSource, +} from '@Lib/index'; +import { createApplication } from '../setup/snjs/appFactory'; +import { + createNoteItem, +} from '../helpers'; + +describe('Application', () => { + /** The global Standard Notes application. */ + let testSNApp; + + beforeEach(async () => { + testSNApp = await createApplication('test-application', Environment.Web, Platform.LinuxWeb); + }); + + afterEach(() => { + testSNApp.deinit(DeinitSource.SignOut); + }); + + describe('signOut()', () => { + let confirmAlert; + + beforeEach(async () => { + confirmAlert = jest.spyOn( + testSNApp.alertService, + 'confirm' + ); + }); + + it('shows confirmation dialog when there are unsaved changes', async () => { + const testNote1 = await createNoteItem(testSNApp, { + title: 'Note 1', + text: 'This is a test note!' + }); + + await testSNApp.itemManager.setItemDirty(testNote1.uuid); + + await testSNApp.signOut(); + + expect(confirmAlert).toBeCalledTimes(1); + expect(confirmAlert).toBeCalledWith( + `There are 1 items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` + ); + }); + + it('does not show confirmation dialog when there are no unsaved changes', async () => { + await createNoteItem(testSNApp, { + title: 'Note 1', + text: 'This is a test note :D' + }); + + await testSNApp.signOut(); + + expect(confirmAlert).toBeCalledTimes(0); + }); + }); +}); diff --git a/packages/snjs/test/setup/snjs/deviceInterface.ts b/packages/snjs/test/setup/snjs/deviceInterface.ts index 25f585264..5830f5d25 100644 --- a/packages/snjs/test/setup/snjs/deviceInterface.ts +++ b/packages/snjs/test/setup/snjs/deviceInterface.ts @@ -128,7 +128,9 @@ export default class DeviceInterface extends SNDeviceInterface { async getRawKeychainValue() { const keychain = this.localStorage.getItem(KEYCHAIN_STORAGE_KEY); - return JSON.parse(keychain); + if (keychain) { + return JSON.parse(keychain); + } } async clearRawKeychainValue() { From 42fee0c03a3c5720434a5d402fda1c60206fc361 Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 10:26:32 -0400 Subject: [PATCH 06/13] test(wip): application signOut --- packages/snjs/test/lib/application.test.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js index fad6ea9be..5c0fbdd42 100644 --- a/packages/snjs/test/lib/application.test.js +++ b/packages/snjs/test/lib/application.test.js @@ -37,7 +37,6 @@ describe('Application', () => { }); await testSNApp.itemManager.setItemDirty(testNote1.uuid); - await testSNApp.signOut(); expect(confirmAlert).toBeCalledTimes(1); @@ -56,5 +55,17 @@ describe('Application', () => { expect(confirmAlert).toBeCalledTimes(0); }); + + it('does not show confirmation dialog when there are unsaved changes and the "force" option is set to true', async () => { + const testNote1 = await createNoteItem(testSNApp, { + title: 'Note 1', + text: 'This is a test note!' + }); + + await testSNApp.itemManager.setItemDirty(testNote1.uuid); + await testSNApp.signOut(true); + + expect(confirmAlert).toBeCalledTimes(0); + }); }); }); From 48bb89e5f745a88968584f45217c990a91d5947d Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 10:46:57 -0400 Subject: [PATCH 07/13] fix: include lib/application --- packages/snjs/jest.config.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/snjs/jest.config.ts b/packages/snjs/jest.config.ts index f2ceaa0ba..b5f9495d8 100644 --- a/packages/snjs/jest.config.ts +++ b/packages/snjs/jest.config.ts @@ -16,6 +16,7 @@ export default { collectCoverageFrom: [ //'lib/**/{!(index),}.ts', 'lib/services/component_manager.ts', + 'lib/application.ts', ], // The directory where Jest should output its coverage files From 5d22f3d41c1163d07fba2b5946e012c278e2f1aa Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 10:47:22 -0400 Subject: [PATCH 08/13] test(wip): application signOut --- packages/snjs/test/lib/application.test.js | 45 ++++++++++++++-------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js index 5c0fbdd42..75a02aac0 100644 --- a/packages/snjs/test/lib/application.test.js +++ b/packages/snjs/test/lib/application.test.js @@ -21,21 +21,26 @@ describe('Application', () => { }); describe('signOut()', () => { + let testNote1; let confirmAlert; + let deinit; beforeEach(async () => { + testNote1 = await createNoteItem(testSNApp, { + title: 'Note 1', + text: 'This is a test note!' + }); confirmAlert = jest.spyOn( testSNApp.alertService, 'confirm' ); + deinit = jest.spyOn( + testSNApp, + 'deinit' + ); }); it('shows confirmation dialog when there are unsaved changes', async () => { - const testNote1 = await createNoteItem(testSNApp, { - title: 'Note 1', - text: 'This is a test note!' - }); - await testSNApp.itemManager.setItemDirty(testNote1.uuid); await testSNApp.signOut(); @@ -43,29 +48,39 @@ describe('Application', () => { expect(confirmAlert).toBeCalledWith( `There are 1 items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` ); + expect(deinit).toBeCalledTimes(1); + expect(deinit).toBeCalledWith(DeinitSource.SignOut); }); it('does not show confirmation dialog when there are no unsaved changes', async () => { - await createNoteItem(testSNApp, { - title: 'Note 1', - text: 'This is a test note :D' - }); - await testSNApp.signOut(); expect(confirmAlert).toBeCalledTimes(0); + expect(deinit).toBeCalledTimes(1); + expect(deinit).toBeCalledWith(DeinitSource.SignOut); }); it('does not show confirmation dialog when there are unsaved changes and the "force" option is set to true', async () => { - const testNote1 = await createNoteItem(testSNApp, { - title: 'Note 1', - text: 'This is a test note!' - }); - await testSNApp.itemManager.setItemDirty(testNote1.uuid); await testSNApp.signOut(true); expect(confirmAlert).toBeCalledTimes(0); + expect(deinit).toBeCalledTimes(1); + expect(deinit).toBeCalledWith(DeinitSource.SignOut); + }); + + it('cancels sign out if confirmation dialog is rejected', async () => { + // Rejecting all confirmation prompts via the presentPermissionsDialog + confirmAlert.mockImplementation((message) => false); + + await testSNApp.itemManager.setItemDirty(testNote1.uuid); + await testSNApp.signOut(); + + expect(confirmAlert).toBeCalledTimes(1); + expect(confirmAlert).toBeCalledWith( + `There are 1 items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` + ); + expect(deinit).toBeCalledTimes(0); }); }); }); From 1ef8fc463bdfb849397f0b34c1a7ca3eabef6873 Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 11:05:34 -0400 Subject: [PATCH 09/13] fix: remove comment --- packages/snjs/test/lib/application.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js index 75a02aac0..8e0e62d9a 100644 --- a/packages/snjs/test/lib/application.test.js +++ b/packages/snjs/test/lib/application.test.js @@ -70,7 +70,6 @@ describe('Application', () => { }); it('cancels sign out if confirmation dialog is rejected', async () => { - // Rejecting all confirmation prompts via the presentPermissionsDialog confirmAlert.mockImplementation((message) => false); await testSNApp.itemManager.setItemDirty(testNote1.uuid); From d2bb16fe759be7ce7e543a8e81226f14167b93fe Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 11:06:44 -0400 Subject: [PATCH 10/13] fix: remove extra line --- packages/snjs/lib/application.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/snjs/lib/application.ts b/packages/snjs/lib/application.ts index b868fb227..d338f0e89 100644 --- a/packages/snjs/lib/application.ts +++ b/packages/snjs/lib/application.ts @@ -1328,7 +1328,6 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR const didConfirm = await this.alertService.confirm( `There are ${dirtyItems.length} items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` ); - if (didConfirm) { await performSignOut(); } From ab275a46676a02cbc4ccea861240d9bd6ae18bfa Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 11:12:21 -0400 Subject: [PATCH 11/13] fix: remove afterEach --- packages/snjs/test/lib/application.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js index 8e0e62d9a..50c6c0cfd 100644 --- a/packages/snjs/test/lib/application.test.js +++ b/packages/snjs/test/lib/application.test.js @@ -16,10 +16,6 @@ describe('Application', () => { testSNApp = await createApplication('test-application', Environment.Web, Platform.LinuxWeb); }); - afterEach(() => { - testSNApp.deinit(DeinitSource.SignOut); - }); - describe('signOut()', () => { let testNote1; let confirmAlert; From 95b5f6767d51825732c2a14244b6e30c5cff55c1 Mon Sep 17 00:00:00 2001 From: Johnny A <5891646+johnny243@users.noreply.github.com> Date: Mon, 5 Jul 2021 11:55:38 -0400 Subject: [PATCH 12/13] Update packages/snjs/lib/application.ts Co-authored-by: Mo Bitar --- packages/snjs/lib/application.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snjs/lib/application.ts b/packages/snjs/lib/application.ts index d338f0e89..59b497748 100644 --- a/packages/snjs/lib/application.ts +++ b/packages/snjs/lib/application.ts @@ -1326,7 +1326,7 @@ public getSessions(): Promise<(HttpResponse & { data: RemoteSession[] }) | HttpR const dirtyItems = this.itemManager.getDirtyItems(); if (dirtyItems.length > 0) { const didConfirm = await this.alertService.confirm( - `There are ${dirtyItems.length} items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` + `There are ${dirtyItems.length} items with unsynced changes. If you sign out, these changes will be lost forever. Are you sure you want to sign out?` ); if (didConfirm) { await performSignOut(); From 433be8944d87a4cdefd17b9b87a136da73e86658 Mon Sep 17 00:00:00 2001 From: Johnny Almonte Date: Mon, 5 Jul 2021 12:01:26 -0400 Subject: [PATCH 13/13] fix: expected confirm message --- packages/snjs/test/lib/application.test.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/snjs/test/lib/application.test.js b/packages/snjs/test/lib/application.test.js index 50c6c0cfd..69561df48 100644 --- a/packages/snjs/test/lib/application.test.js +++ b/packages/snjs/test/lib/application.test.js @@ -21,6 +21,11 @@ describe('Application', () => { let confirmAlert; let deinit; + const signOutConfirmMessage = (numberOfItems) => { + return `There are ${numberOfItems} items with unsynced changes. ` + + 'If you sign out, these changes will be lost forever. Are you sure you want to sign out?' + }; + beforeEach(async () => { testNote1 = await createNoteItem(testSNApp, { title: 'Note 1', @@ -40,10 +45,10 @@ describe('Application', () => { await testSNApp.itemManager.setItemDirty(testNote1.uuid); await testSNApp.signOut(); + const expectedConfirmMessage = signOutConfirmMessage(1); + expect(confirmAlert).toBeCalledTimes(1); - expect(confirmAlert).toBeCalledWith( - `There are 1 items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` - ); + expect(confirmAlert).toBeCalledWith(expectedConfirmMessage); expect(deinit).toBeCalledTimes(1); expect(deinit).toBeCalledWith(DeinitSource.SignOut); }); @@ -71,10 +76,10 @@ describe('Application', () => { await testSNApp.itemManager.setItemDirty(testNote1.uuid); await testSNApp.signOut(); + const expectedConfirmMessage = signOutConfirmMessage(1); + expect(confirmAlert).toBeCalledTimes(1); - expect(confirmAlert).toBeCalledWith( - `There are 1 items with unsynced changes. If you sign out, these changes will be forever lost. Are you sure you want to sign out?` - ); + expect(confirmAlert).toBeCalledWith(expectedConfirmMessage); expect(deinit).toBeCalledTimes(0); }); });