Merge pull request #241 from ghiscoding/bugfix/backend-clear-filter

fix(backend): clear empty filter by header menu not stopping spinner

Author: ghiscoding

.vscode/tasks.json

@@ -34,7 +34,7 @@
     {
       "label": "Cypress Open Tool",
       "type": "shell",
-      "command": "yarn run cypress open",
+      "command": "yarn run cypress:open",
       "problemMatcher": []
     },
     {

src/app/modules/angular-slickgrid/services/__tests__/filter.service.spec.ts

@@ -363,6 +363,7 @@ describe('FilterService', () => {
   describe('clearFilter methods on backend grid', () => {
     let mockColumn1: Column;
     let mockColumn2: Column;
+    let mockColumn3: Column;
 
     beforeEach(() => {
       gridOptionMock.backendServiceApi = {
@@ -372,13 +373,16 @@ describe('FilterService', () => {
       };
       mockColumn1 = { id: 'firstName', field: 'firstName', filterable: true } as Column;
       mockColumn2 = { id: 'lastName', field: 'lastName', filterable: true } as Column;
+      mockColumn3 = { id: 'age', field: 'age', filterable: true } as Column;
       const mockArgs1 = { grid: gridStub, column: mockColumn1, node: document.getElementById(DOM_ELEMENT_ID) };
       const mockArgs2 = { grid: gridStub, column: mockColumn2, node: document.getElementById(DOM_ELEMENT_ID) };
+      const mockArgs3 = { grid: gridStub, column: mockColumn3, node: document.getElementById(DOM_ELEMENT_ID) };
 
       service.init(gridStub);
       service.bindBackendOnFilter(gridStub, dataViewStub);
       gridStub.onHeaderRowCellRendered.notify(mockArgs1, new Slick.EventData(), gridStub);
       gridStub.onHeaderRowCellRendered.notify(mockArgs2, new Slick.EventData(), gridStub);
+      gridStub.onHeaderRowCellRendered.notify(mockArgs3, new Slick.EventData(), gridStub);
       service.getFiltersMetadata()[0].callback(new Event('keyup'), { columnDef: mockColumn1, operator: 'EQ', searchTerms: ['John'], shouldTriggerQuery: true });
       service.getFiltersMetadata()[1].callback(new Event('keyup'), { columnDef: mockColumn2, operator: 'NE', searchTerms: ['Doe'], shouldTriggerQuery: true });
     });
@@ -402,6 +406,26 @@ describe('FilterService', () => {
         expect(service.getColumnFilters()).toEqual({ lastName: filterExpectation });
         expect(spyEmitter).toHaveBeenCalledWith('remote');
       });
+
+      it('should not call "onBackendFilterChange" method when the filter is previously empty', () => {
+        const filterFirstExpectation = { columnDef: mockColumn1, columnId: 'firstName', operator: 'EQ', searchTerms: ['John'] };
+        const filterLastExpectation = { columnDef: mockColumn2, columnId: 'lastName', operator: 'NE', searchTerms: ['Doe'] };
+        const newEvent = new Event('mouseup');
+        const spyClear = jest.spyOn(service.getFiltersMetadata()[2], 'clear');
+        const spyFilterChange = jest.spyOn(service, 'onBackendFilterChange');
+        const spyEmitter = jest.spyOn(service, 'emitFilterChanged');
+
+        const filterCountBefore = Object.keys(service.getColumnFilters()).length;
+        service.clearFilterByColumnId(newEvent, 'age');
+        const filterCountAfter = Object.keys(service.getColumnFilters()).length;
+
+        expect(spyClear).toHaveBeenCalled();
+        expect(spyFilterChange).not.toHaveBeenCalled();
+        expect(filterCountBefore).toBe(2);
+        expect(filterCountAfter).toBe(2);
+        expect(service.getColumnFilters()).toEqual({ firstName: filterFirstExpectation, lastName: filterLastExpectation });
+        expect(spyEmitter).toHaveBeenCalledWith('remote');
+      });
     });
 
     describe('clearFilters method', () => {

src/app/modules/angular-slickgrid/services/filter.service.ts

@@ -169,6 +169,14 @@ export class FilterService {
   }
 
   clearFilterByColumnId(event: Event, columnId: number | string) {
+    // get current column filter before clearing, this allow us to know if the filter was empty prior to calling the clear filter
+    const currentColumnFilters = Object.keys(this._columnFilters) as ColumnFilter[];
+    let currentColFilter: ColumnFilter;
+    if (Array.isArray(currentColumnFilters)) {
+      currentColFilter = currentColumnFilters.find((name) => name === columnId);
+    }
+
+    // find the filter object and call its clear method with true (the argument tells the method it was called by a clear filter)
     const colFilter: Filter = this._filtersMetadata.find((filter: Filter) => filter.columnDef.id === columnId);
     if (colFilter && colFilter.clear) {
       colFilter.clear(true);
@@ -177,10 +185,12 @@ export class FilterService {
     let emitter: EmitterType = EmitterType.local;
     const isBackendApi = this._gridOptions && this._gridOptions.backendServiceApi || false;
 
-    // when using a backend service, we need to manually trigger a filter change
+    // when using a backend service, we need to manually trigger a filter change but only if the filter was previously filled
     if (isBackendApi) {
       emitter = EmitterType.remote;
-      this.onBackendFilterChange(event as KeyboardEvent, { grid: this._grid, columnFilters: this._columnFilters });
+      if (currentColFilter) {
+        this.onBackendFilterChange(event as KeyboardEvent, { grid: this._grid, columnFilters: this._columnFilters });
+      }
     }
 
     // emit an event when filter is cleared

src/tsconfig.spec.json

@@ -6,6 +6,7 @@
     "module": "commonjs",
     "target": "es5",
     "types": [
+      "cypress",
       "jasmine",
       "node"
     ]

test/cypress/integration/example1.spec.js

@@ -1,5 +1,3 @@
-/// <reference types="Cypress" />
-
 describe('Example 1 - Basic Grids', () => {
   const titles = ['Title', 'Duration (days)', '% Complete', 'Start', 'Finish', 'Effort Driven'];
 

test/cypress/integration/example6.spec.js

@@ -1,5 +1,3 @@
-/// <reference types="Cypress" />
-
 describe('Example 6 - GraphQL Grid', () => {
   it('should display Example 6 title', () => {
     cy.visit(`${Cypress.config('baseExampleUrl')}/gridgraphql`);
@@ -69,6 +67,57 @@ describe('Example 6 - GraphQL Grid', () => {
       });
   });
 
+  it('should clear a single filter, that is not empty, by the header menu and expect query change', () => {
+    cy.get('#grid6')
+      .find('.slick-header-column:nth(1)')
+      .trigger('mouseover')
+      .children('.slick-header-menubutton')
+      .should('be.hidden')
+      .invoke('show')
+      .click();
+
+    cy.get('.slick-header-menu')
+      .should('be.visible')
+      .children('.slick-header-menuitem:nth-child(4)')
+      .children('.slick-header-menucontent')
+      .should('contain', 'Remove Filter')
+      .click();
+
+    // wait for the query to finish
+    cy.get('[data-test=status]').should('contain', 'done');
+
+    cy.get('[data-test=graphql-query-result]')
+      .should(($span) => {
+        const text = $span.text().replace(/\s/g, ''); // remove all white spaces
+        expect(text).to.eq('query{users(first:30,offset:0,orderBy:[{field:"name",direction:ASC},{field:"company",direction:DESC}],filterBy:[{field:"gender",operator:EQ,value:"male"},{field:"company",operator:IN,value:"xyz"}],locale:"en",userId:123){totalCount,nodes{id,name,gender,company,billing{address{street,zip}}}}}');
+      });
+  });
+
+  it('should try clearing same filter, which is now empty, by the header menu and expect same query without loading spinner', () => {
+    cy.get('#grid6')
+      .find('.slick-header-column:nth(1)')
+      .trigger('mouseover')
+      .children('.slick-header-menubutton')
+      .invoke('show')
+      .click();
+
+    cy.get('.slick-header-menu')
+      .should('be.visible')
+      .children('.slick-header-menuitem:nth-child(4)')
+      .children('.slick-header-menucontent')
+      .should('contain', 'Remove Filter')
+      .click();
+
+    // wait for the query to finish
+    cy.get('[data-test=status]').should('contain', 'done');
+
+    cy.get('[data-test=graphql-query-result]')
+      .should(($span) => {
+        const text = $span.text().replace(/\s/g, ''); // remove all white spaces
+        expect(text).to.eq('query{users(first:30,offset:0,orderBy:[{field:"name",direction:ASC},{field:"company",direction:DESC}],filterBy:[{field:"gender",operator:EQ,value:"male"},{field:"company",operator:IN,value:"xyz"}],locale:"en",userId:123){totalCount,nodes{id,name,gender,company,billing{address{street,zip}}}}}');
+      });
+  });
+
   it('should clear all Filters & Sorts', () => {
     cy.contains('Clear all Filter & Sorts').click();
 

test/cypress/integration/home.spec.js

@@ -1,5 +1,3 @@
-/// <reference types="Cypress" />
-
 describe('Home Page', () => {
   it('should display Home Page', () => {
     cy.visit('http://localhost:4300/home');