From b6b68259db452e2a6c9f6cec6722c396e6cb00e7 Mon Sep 17 00:00:00 2001 From: ViktorSlavov Date: Fri, 14 Dec 2018 15:28:45 +0200 Subject: [PATCH 1/5] test(transaction): deleting w/ transactions + paging in grid, #3425 --- .../src/lib/grids/grid/grid.component.spec.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts b/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts index 621c29b03ed..abe73959e86 100644 --- a/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts +++ b/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts @@ -2865,6 +2865,89 @@ describe('IgxGrid Component Tests', () => { expect(targetRowElement.classList).toContain('igx-grid__tr--edited', 'row does not contain edited class w/ edits'); expect(targetCellElement.classList).toContain('igx-grid__td--edited', 'cell does not contain edited class w/ edits'); })); + + it('Should change pages when the only item on the last page is a pending added row that gets deleted', fakeAsync(() => { + const fixture = TestBed.createComponent(IgxGridRowEditingTransactionComponent); + fixture.detectChanges(); + + const grid = fixture.componentInstance.grid; + expect(grid.data.length).toEqual(10); + grid.paging = true; + grid.perPage = 5; + fixture.detectChanges(); + tick(); + expect(grid.totalPages).toEqual(2); + grid.addRow({ + ProductID: 123, + ProductName: 'DummyItem', + InStock: true, + UnitsInStock: 1, + OrderDate: new Date() + }); + fixture.detectChanges(); + tick(); + expect(grid.totalPages).toEqual(3); + grid.page = 2; + tick(); + fixture.detectChanges(); + expect(grid.page).toEqual(2); + grid.deleteRowById(123); + tick(); + fixture.detectChanges(); + // This is behaving incorrectly - if there is only 1 transaction and it is an ADD transaction on the last page + // Deleting the ADD transaction on the last page will trigger grid.page-- TWICE + expect(grid.page).toEqual(0); // Should be 1 + expect(grid.totalPages).toEqual(2); + })); + + it('Should change pages when commiting deletes on the last page', fakeAsync(() => { + const fixture = TestBed.createComponent(IgxGridRowEditingTransactionComponent); + fixture.detectChanges(); + + const grid = fixture.componentInstance.grid; + expect(grid.data.length).toEqual(10); + grid.paging = true; + grid.perPage = 5; + fixture.detectChanges(); + tick(); + expect(grid.totalPages).toEqual(2); + grid.page = 1; + tick(); + fixture.detectChanges(); + expect(grid.page).toEqual(1); + for (let i = 0; i < grid.data.length / 2; i++) { + grid.deleteRowById(grid.data.reverse()[i].ProductID); + } + fixture.detectChanges(); + tick(); + expect(grid.page).toEqual(1); + grid.transactions.commit(grid.data); + fixture.detectChanges(); + tick(); + expect(grid.page).toEqual(0); + expect(grid.totalPages).toEqual(1); + })); + + it('Should NOT change pages when deleting a row on the last page', fakeAsync(() => { + const fixture = TestBed.createComponent(IgxGridRowEditingTransactionComponent); + fixture.detectChanges(); + const grid = fixture.componentInstance.grid; + grid.paging = true; + grid.perPage = 5; + fixture.detectChanges(); + tick(); + expect(grid.totalPages).toEqual(2); + expect(grid.data.length).toEqual(10); + grid.page = 1; + tick(); + fixture.detectChanges(); + expect(grid.page).toEqual(1); + grid.deleteRowById(grid.data[grid.data.length - 1].ProductID); + fixture.detectChanges(); + tick(); + expect(grid.page).toEqual(1); + expect(grid.totalPages).toEqual(2); + })); }); describe('Row Editing - Grouping', () => { From 3d09204a34129ef3d351e5c1ec514c84e4416983 Mon Sep 17 00:00:00 2001 From: ViktorSlavov Date: Fri, 14 Dec 2018 15:46:39 +0200 Subject: [PATCH 2/5] fix(transaction): check for transactions when commiting delete, #3425 --- .../src/lib/grids/grid-base.component.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts index 74fa509ebb0..1b9ccaa6a87 100644 --- a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts +++ b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts @@ -2288,6 +2288,12 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements this.summaryService.clearSummaryCache(); this._pipeTrigger++; this.markForCheck(); + if (this.transactions.getAggregatedChanges(false).length === 0) { + // Needs better check, calling 'transactions.clear()' will also trigger this + if (this.data.length % this.perPage === 0 && this.isLastPage && this.page !== 0) { + this.page--; + } + } }); } @@ -3072,13 +3078,22 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements this.checkHeaderCheckboxStatus(); } + const addedRowsDif = this.dataWithAddedInTransactionRows.length - this.data.length; this.deleteRowFromData(rowId, index); this._pipeTrigger++; this.cdr.markForCheck(); this.refreshSearch(); - if (data.length % this.perPage === 0 && this.isLastPage && this.page !== 0) { - this.page--; + if (this.isLastPage && this.page !== 0) { + let pageSwitch = 0; + if (!this.transactions.enabled) { + pageSwitch = this.data.length % this.perPage === 0 ? 1 : 0; + } else { + if (addedRowsDif) { + pageSwitch = this.dataWithAddedInTransactionRows.length % this.perPage === 0 ? 1 : 0; + } + } + this.page -= pageSwitch; } } From 1f1634b6be841889c7cdabee532b5b76a3ae48eb Mon Sep 17 00:00:00 2001 From: ViktorSlavov Date: Fri, 14 Dec 2018 16:39:22 +0200 Subject: [PATCH 3/5] fix(transaction): hier trans getAggregatedChanges uses state copy instead, #3425 --- .../lib/services/transaction/igx-hierarchical-transaction.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 786dd331a4d..a79ffaa3d57 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 @@ -2,6 +2,7 @@ import { HierarchicalTransaction, HierarchicalState, TransactionType } from './t import { Injectable } from '@angular/core'; import { IgxTransactionService } from './igx-transaction'; import { DataUtil } from '../../data-operations/data-util'; +import { cloneValue } from '../../core/utils'; /** @experimental @hidden */ @Injectable() @@ -11,7 +12,7 @@ export class IgxHierarchicalTransactionService { - const value = mergeChanges ? this.mergeValues(state.recordRef, state.value) : state.value; + const value = mergeChanges ? this.mergeValues(state.recordRef, state.value) : cloneValue(state.value); this.clearArraysFromObject(value); result.push({ id: key, path: state.path, newValue: value, type: state.type } as T); }); From 01659ccaf35c4f6e0369bf8e714e12388fb1b51f Mon Sep 17 00:00:00 2001 From: ViktorSlavov Date: Mon, 17 Dec 2018 14:20:26 +0200 Subject: [PATCH 4/5] fix(grid): proper check for paging w/ transactions in place, #3425 --- .../src/lib/grids/grid-base.component.ts | 18 ++++++------------ .../src/lib/grids/grid/grid.component.spec.ts | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts index 1b9ccaa6a87..a8006f8d078 100644 --- a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts +++ b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts @@ -3049,7 +3049,7 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements /** @hidden */ public deleteRowById(rowId: any) { let index: number; - const data = this.gridAPI.get_all_data(this.id); + const data = this.gridAPI.get_all_data(this.id, this.transactions.enabled); if (this.primaryKey) { index = data.map((record) => record[this.primaryKey]).indexOf(rowId); } else { @@ -3078,22 +3078,16 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements this.checkHeaderCheckboxStatus(); } - const addedRowsDif = this.dataWithAddedInTransactionRows.length - this.data.length; this.deleteRowFromData(rowId, index); this._pipeTrigger++; this.cdr.markForCheck(); - + const dataAfterDelete = !this.transactions.enabled ? data : this.dataWithAddedInTransactionRows; this.refreshSearch(); - if (this.isLastPage && this.page !== 0) { - let pageSwitch = 0; - if (!this.transactions.enabled) { - pageSwitch = this.data.length % this.perPage === 0 ? 1 : 0; - } else { - if (addedRowsDif) { - pageSwitch = this.dataWithAddedInTransactionRows.length % this.perPage === 0 ? 1 : 0; - } + if (dataAfterDelete.length % this.perPage === 0 && this.isLastPage && this.page !== 0) { + const currentPage = Math.ceil(dataAfterDelete.length / this.perPage) - 1; + if (this.page !== currentPage) { + this.page = currentPage; } - this.page -= pageSwitch; } } diff --git a/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts b/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts index abe73959e86..c5e79acea5c 100644 --- a/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts +++ b/projects/igniteui-angular/src/lib/grids/grid/grid.component.spec.ts @@ -2896,7 +2896,7 @@ describe('IgxGrid Component Tests', () => { fixture.detectChanges(); // This is behaving incorrectly - if there is only 1 transaction and it is an ADD transaction on the last page // Deleting the ADD transaction on the last page will trigger grid.page-- TWICE - expect(grid.page).toEqual(0); // Should be 1 + expect(grid.page).toEqual(1); // Should be 1 expect(grid.totalPages).toEqual(2); })); From 6fb83806ec0c3b4fb2422781886317c1b215b0b7 Mon Sep 17 00:00:00 2001 From: ViktorSlavov Date: Tue, 18 Dec 2018 17:13:25 +0200 Subject: [PATCH 5/5] refactor(grid): add comment to row delete+paging, change check for page, #3425 --- .../src/lib/grids/grid-base.component.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts index a8006f8d078..4ec424bc82a 100644 --- a/projects/igniteui-angular/src/lib/grids/grid-base.component.ts +++ b/projects/igniteui-angular/src/lib/grids/grid-base.component.ts @@ -3049,7 +3049,7 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements /** @hidden */ public deleteRowById(rowId: any) { let index: number; - const data = this.gridAPI.get_all_data(this.id, this.transactions.enabled); + const data = this.gridAPI.get_all_data(this.id); if (this.primaryKey) { index = data.map((record) => record[this.primaryKey]).indexOf(rowId); } else { @@ -3081,13 +3081,12 @@ export abstract class IgxGridBaseComponent extends DisplayDensityBase implements this.deleteRowFromData(rowId, index); this._pipeTrigger++; this.cdr.markForCheck(); - const dataAfterDelete = !this.transactions.enabled ? data : this.dataWithAddedInTransactionRows; + // Data needs to be recalculated if transactions are in place + // If no transactions, `data` will be a reference to the grid getter, otherwise it will be stale + const dataAfterDelete = this.transactions.enabled ? this.dataWithAddedInTransactionRows : data; this.refreshSearch(); - if (dataAfterDelete.length % this.perPage === 0 && this.isLastPage && this.page !== 0) { - const currentPage = Math.ceil(dataAfterDelete.length / this.perPage) - 1; - if (this.page !== currentPage) { - this.page = currentPage; - } + if (dataAfterDelete.length % this.perPage === 0 && dataAfterDelete.length / this.perPage - 1 < this.page && this.page !== 0) { + this.page--; } }