From 8e9eacd74eedeff2666313d26d3b02534210f718 Mon Sep 17 00:00:00 2001 From: wnvko Date: Tue, 11 Dec 2018 15:22:04 +0200 Subject: [PATCH 1/5] fix(igxTransaction): call onStateUpdate in endPending, #2921 --- .../src/lib/services/transaction/igx-transaction.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.ts b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.ts index 82851be8b4e..1c9d3b939d7 100644 --- a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.ts +++ b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.ts @@ -116,6 +116,7 @@ export class IgxTransactionService exten this._isPending = false; if (commit) { const actions: { transaction: T, recordRef: any }[] = []; + // don't use addTransaction due to custom undo handling for (const transaction of this._pendingTransactions) { const pendingState = this._pendingStates.get(transaction.id); this._transactions.push(transaction); @@ -125,6 +126,8 @@ export class IgxTransactionService exten this._undoStack.push(actions); this._redoStack = []; + + this.onStateUpdate.emit(); } super.endPending(commit); } From 6de7ce25507cf0b5d4ffebbc93206112d5892a98 Mon Sep 17 00:00:00 2001 From: wnvko Date: Tue, 11 Dec 2018 15:37:47 +0200 Subject: [PATCH 2/5] test(igxTransaction): add tests for onStateUpdate event, #2921 --- .../transaction/igx-transaction.spec.ts | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts index e207da45939..f7cf8205be7 100644 --- a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts +++ b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts @@ -3,7 +3,7 @@ import { Transaction, TransactionType, HierarchicalTransaction } from './transac import { SampleTestData } from '../../test-utils/sample-test-data.spec'; import { IgxHierarchicalTransactionService } from './igx-hierarchical-transaction'; -describe('IgxTransaction', () => { +fdescribe('IgxTransaction', () => { describe('IgxTransaction UNIT tests', () => { it('Should initialize transactions log properly', () => { const trans = new IgxTransactionService(); @@ -161,8 +161,9 @@ describe('IgxTransaction', () => { expect(trans.getTransactionLog('100').pop()).toEqual(undefined); }); - it('Should add ADD type transaction - all feasible paths', () => { + it('Should add ADD type transaction - all feasible paths, and correctly fires onStateUpdate', () => { const trans = new IgxTransactionService(); + spyOn(trans.onStateUpdate, 'emit').and.callThrough(); expect(trans).toBeDefined(); // ADD @@ -176,18 +177,24 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(1); + trans.clear(); expect(trans.getState('1')).toBeUndefined(); expect(trans.getAggregatedValue('1', true)).toBeNull(); expect(trans.getTransactionLog()).toEqual([]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(2); // ADD -> Undo trans.add(addTransaction); trans.undo(); expect(trans.getTransactionLog()).toEqual([]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(4); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(5); // ADD -> Undo -> Redo trans.add(addTransaction); @@ -199,7 +206,10 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(8); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(9); // ADD -> DELETE trans.add(addTransaction); @@ -207,7 +217,10 @@ describe('IgxTransaction', () => { trans.add(deleteTransaction); expect(trans.getTransactionLog()).toEqual([addTransaction, deleteTransaction]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(11); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(12); // ADD -> DELETE -> Undo trans.add(addTransaction); @@ -219,7 +232,10 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(15); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(16); // ADD -> DELETE -> Undo -> Redo trans.add(addTransaction); @@ -228,7 +244,10 @@ describe('IgxTransaction', () => { trans.redo(); expect(trans.getTransactionLog()).toEqual([addTransaction, deleteTransaction]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(20); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(21); // ADD -> DELETE -> Undo -> Undo trans.add(addTransaction); @@ -237,7 +256,10 @@ describe('IgxTransaction', () => { trans.undo(); expect(trans.getTransactionLog()).toEqual([]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(25); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(26); // ADD -> UPDATE trans.add(addTransaction); @@ -249,7 +271,10 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(28); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(29); // ADD -> UPDATE -> Undo trans.add(addTransaction); @@ -261,7 +286,10 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(32); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(33); // ADD -> UPDATE -> Undo -> Redo trans.add(addTransaction); @@ -274,7 +302,10 @@ describe('IgxTransaction', () => { recordRef: undefined, type: addTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(37); + trans.clear(); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(38); }); it('Should add DELETE type transaction - all feasible paths', () => { @@ -540,8 +571,10 @@ describe('IgxTransaction', () => { expect(originalData[49]).toEqual('Added Row'); }); - it('Should add pending transaction and push it to transaction log', () => { + it('Should add pending transaction and push it to transaction log, and correctly fires onStateUpdate', () => { const trans = new IgxTransactionService(); + spyOn(trans.onStateUpdate, 'emit').and.callThrough(); + expect(trans).toBeDefined(); const recordRef = { key: 'Key1', value1: 1, value2: 2, value3: 3 }; let newValue: any = { key: 'Key1', value1: 10 }; @@ -585,10 +618,13 @@ describe('IgxTransaction', () => { recordRef: recordRef, type: updateTransaction.type }); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(1); }); - it('Should not add pending transaction and push it to transaction log', () => { + it('Should not add pending transaction and push it to transaction log, and correctly fires onStateUpdate', () => { const trans = new IgxTransactionService(); + spyOn(trans.onStateUpdate, 'emit').and.callThrough(); + expect(trans).toBeDefined(); const recordRef = { key: 'Key1', value1: 1, value2: 2, value3: 3 }; let newValue: any = { key: 'Key1', value1: 10 }; @@ -611,6 +647,7 @@ describe('IgxTransaction', () => { expect(trans.getTransactionLog()).toEqual([]); expect(trans.getAggregatedChanges(true)).toEqual([]); + expect(trans.onStateUpdate.emit).toHaveBeenCalledTimes(0); }); }); From 5ed94d2dfc61038800ff4771c247d70fcc296764 Mon Sep 17 00:00:00 2001 From: wnvko Date: Tue, 11 Dec 2018 16:15:38 +0200 Subject: [PATCH 3/5] test(igxTransaction): error on calling getAggregatedChanges(false), #2921 --- .../lib/services/transaction/igx-transaction.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts index f7cf8205be7..1d3cbcea927 100644 --- a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts +++ b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts @@ -719,6 +719,16 @@ fdescribe('IgxTransaction', () => { expect(transaction.getState(2)).toBeDefined(); expect(transaction.getState(2).type).toBe(TransactionType.DELETE); }); + + it('Should correctly call getAggregatedChanges with commit', () => { + const transaction = new IgxHierarchicalTransactionService(); + expect(transaction).toBeDefined(); + + const deleteTransaction: HierarchicalTransaction = { id: 0, type: TransactionType.DELETE, newValue: null, path: [] }; + transaction.add(deleteTransaction, 'Deleted row'); + + expect(transaction.getAggregatedChanges(false)).toBe(null); + }); }); }); From dc9241d6dd443f381ee88a98bc5d36a3527b64c3 Mon Sep 17 00:00:00 2001 From: wnvko Date: Tue, 11 Dec 2018 16:21:22 +0200 Subject: [PATCH 4/5] fix(igxTransaction): should correctly call getAggregatedChanges(false), #2921 --- .../services/transaction/igx-hierarchical-transaction.ts | 8 +++++--- .../src/lib/services/transaction/igx-transaction.spec.ts | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/projects/igniteui-angular/src/lib/services/transaction/igx-hierarchical-transaction.ts b/projects/igniteui-angular/src/lib/services/transaction/igx-hierarchical-transaction.ts index 2f722f5d4e3..786dd331a4d 100644 --- a/projects/igniteui-angular/src/lib/services/transaction/igx-hierarchical-transaction.ts +++ b/projects/igniteui-angular/src/lib/services/transaction/igx-hierarchical-transaction.ts @@ -61,9 +61,11 @@ export class IgxHierarchicalTransactionService { }); }); - describe('IgxHierarchicalTransaction UNIT Test', () => { + fdescribe('IgxHierarchicalTransaction UNIT Test', () => { it('Should set path for each state when transaction is added in Hierarchical data source', () => { const transaction = new IgxHierarchicalTransactionService(); expect(transaction).toBeDefined(); @@ -720,14 +720,14 @@ fdescribe('IgxTransaction', () => { expect(transaction.getState(2).type).toBe(TransactionType.DELETE); }); - it('Should correctly call getAggregatedChanges with commit', () => { + it('Should correctly call getAggregatedChanges without commit when recordRef is null', () => { const transaction = new IgxHierarchicalTransactionService(); expect(transaction).toBeDefined(); const deleteTransaction: HierarchicalTransaction = { id: 0, type: TransactionType.DELETE, newValue: null, path: [] }; transaction.add(deleteTransaction, 'Deleted row'); - expect(transaction.getAggregatedChanges(false)).toBe(null); + expect(transaction.getAggregatedChanges(false)).toEqual([deleteTransaction]); }); }); }); From 090840db30b55f2d4bc569cecee6fafd24a052a6 Mon Sep 17 00:00:00 2001 From: wnvko Date: Tue, 11 Dec 2018 16:31:59 +0200 Subject: [PATCH 5/5] chore(igxTransaction): remove fdescribe from tests, #2921 --- .../src/lib/services/transaction/igx-transaction.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts index 22c325bb148..cf839ec2070 100644 --- a/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts +++ b/projects/igniteui-angular/src/lib/services/transaction/igx-transaction.spec.ts @@ -3,7 +3,7 @@ import { Transaction, TransactionType, HierarchicalTransaction } from './transac import { SampleTestData } from '../../test-utils/sample-test-data.spec'; import { IgxHierarchicalTransactionService } from './igx-hierarchical-transaction'; -fdescribe('IgxTransaction', () => { +describe('IgxTransaction', () => { describe('IgxTransaction UNIT tests', () => { it('Should initialize transactions log properly', () => { const trans = new IgxTransactionService(); @@ -651,7 +651,7 @@ fdescribe('IgxTransaction', () => { }); }); - fdescribe('IgxHierarchicalTransaction UNIT Test', () => { + describe('IgxHierarchicalTransaction UNIT Test', () => { it('Should set path for each state when transaction is added in Hierarchical data source', () => { const transaction = new IgxHierarchicalTransactionService(); expect(transaction).toBeDefined();