You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/25 20:05:22 UTC

[GitHub] [superset] michael-s-molina commented on a diff in pull request #19821: test(native filter): refactor and add new test

michael-s-molina commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r857956110


##########
superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts:
##########
@@ -177,38 +301,54 @@ export function clickOnAddFilterInModal() {
 
 /** ************************************************************************
  * Fills value native filter form with basic information
+ * @param {string} type type for filter: Value, Numerical range,Time column,Time grain,Time range
  * @param {string} name name for filter
  * @param {string} dataset which dataset should be used
  * @param {string} filterColumn which column should be used
  * @returns {None}
  * @summary helper for filling value native filter form
  ************************************************************************* */
-export function fillValueNativeFilterForm(
+export function fillNativeFilterForm(

Review Comment:
   Can we move functions related to native filters to another file like `nativeFilters.helper.ts`? 



##########
superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts:
##########
@@ -177,38 +301,54 @@ export function clickOnAddFilterInModal() {
 
 /** ************************************************************************
  * Fills value native filter form with basic information
+ * @param {string} type type for filter: Value, Numerical range,Time column,Time grain,Time range
  * @param {string} name name for filter
  * @param {string} dataset which dataset should be used
  * @param {string} filterColumn which column should be used
  * @returns {None}
  * @summary helper for filling value native filter form
  ************************************************************************* */
-export function fillValueNativeFilterForm(
+export function fillNativeFilterForm(

Review Comment:
   Can we move functions related to native filters to another file like `nativeFilters.helper.ts`? 



##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts:
##########
@@ -602,12 +332,314 @@ describe('Nativefilters Sanity test', () => {
       .invoke('text')
       .should('equal', '214 options', { timeout: 20000 });
     // apply first filter value and validate 2nd filter is depden on 1st filter.
-    applyNativeFilterValueWithIndex(0, 'East Asia & Pacific');
-
-    getNativeFilterPlaceholderWithIndex(0).should('have.text', '36 options', {
+    applyNativeFilterValueWithIndex(0, 'North America');
+    getNativeFilterPlaceholderWithIndex(0).should('have.text', '3 options', {
       timeout: 20000,
     });
   });
+
+  it('User can apply value filter with selected values', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    applyNativeFilterValueWithIndex(0, testItems.filterDefaultValue);
+    cy.get(nativeFilters.applyFilter).click();
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+  });
+
+  it('User can create a numerical range filter', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.numerical,
+      testItems.filterNumericalColumn,
+      testItems.datasetForNativeFilter,
+      testItems.filterNumericalColumn,
+    );
+    saveNativeFilterSettings();
+    // assertions
+    cy.get(nativeFilters.slider.slider).should('be.visible').click('center');
+    // cy.get(sqlLabView.tooltip).should('be.visible');
+    cy.intercept(`/superset/explore_json/*`).as('slices');
+    cy.get(nativeFilters.applyFilter).click();
+    cy.wait('@slices');
+    // assert that the url contains 'native_filters' in the url
+    cy.url().then(u => {
+      const ur = new URL(u);
+      expect(ur.search).to.include('native_filters');
+      // assert that the start handle has a value
+      cy.get(nativeFilters.slider.startHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert that the end handle has a value
+      cy.get(nativeFilters.slider.endHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert slider text matches what we should have
+      cy.get(nativeFilters.slider.sliderText).should('have.text', '49');
+    });
+  });
+
+  it('Verify that default value is respected after revisit', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    inputNativeFilterDefaultValue(testItems.filterDefaultValue);
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.request(
+      'api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:100)',
+    ).then(xhr => {
+      const dashboards = xhr.body.result;
+      const testDashboard = dashboards.find(
+        (d: { dashboard_title: string }) =>
+          d.dashboard_title === testItems.dashboard,
+      );
+      cy.visit(testDashboard.url);
+    });
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+  });
+
+  it('Verify that allow for deleted filter restore', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    validateFilterNameOnDashboard(testItems.topTenChart.filterColumn);
+    enterNativeFilterEditModal();
+    cy.get(nativeFilters.filtersList.removeIcon).first().click();
+    cy.get('[data-test="restore-filter-button"]').should('be.visible').click();
+    cy.get(nativeFilters.modal.container)
+      .find(nativeFilters.filtersPanel.filterName)
+      .should('have.attr', 'value', testItems.topTenChart.filterColumn);
+  });
+
+  it('User can stop filtering when filter is removed', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    inputNativeFilterDefaultValue(testItems.filterDefaultValue);
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+    validateFilterNameOnDashboard(testItems.topTenChart.filterColumn);
+    enterNativeFilterEditModal();
+    deleteNativeFilter();
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('be.visible');
+    });
+  });
+
+  it('user can delete dependent filter', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumnRegion,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumnRegion,
+    );
+    clickOnAddFilterInModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+      () => {
+        cy.contains('Values are dependent on other filters')
+          .should('be.visible')
+          .click();
+      },
+    );
+    addParentFilterWithValue(0, testItems.topTenChart.filterColumnRegion);
+    // remove year native filter to cause it disappears from parent filter input in global sales
+    cy.get(nativeFilters.modal.tabsList.removeTab)
+      .should('be.visible')
+      .first()
+      .click();
+    // make sure you are seeing global sales filter which had parent filter
+    cy.get(nativeFilters.modal.tabsList.filterItemsContainer)
+      .children()
+      .last()
+      .click();
+    //
+    cy.wait(1000);
+    cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+      () => {
+        cy.contains('Values are dependent on other filters').should(
+          'not.exist',
+        );
+      },
+    );
+  });
+
+  it('User can create filter depend on 2 other filters', () => {

Review Comment:
   We could have more functions to reduce the code even more like `fillRegionFilter`, `fillCountryCodeFilter`, `fillCountryNameFilter` and reuse them everywhere. You can still keep the original function for other combinations if needed.



##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts:
##########
@@ -602,12 +332,314 @@ describe('Nativefilters Sanity test', () => {
       .invoke('text')
       .should('equal', '214 options', { timeout: 20000 });
     // apply first filter value and validate 2nd filter is depden on 1st filter.
-    applyNativeFilterValueWithIndex(0, 'East Asia & Pacific');
-
-    getNativeFilterPlaceholderWithIndex(0).should('have.text', '36 options', {
+    applyNativeFilterValueWithIndex(0, 'North America');
+    getNativeFilterPlaceholderWithIndex(0).should('have.text', '3 options', {
       timeout: 20000,
     });
   });
+
+  it('User can apply value filter with selected values', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    applyNativeFilterValueWithIndex(0, testItems.filterDefaultValue);
+    cy.get(nativeFilters.applyFilter).click();
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+  });
+
+  it('User can create a numerical range filter', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.numerical,
+      testItems.filterNumericalColumn,
+      testItems.datasetForNativeFilter,
+      testItems.filterNumericalColumn,
+    );
+    saveNativeFilterSettings();
+    // assertions
+    cy.get(nativeFilters.slider.slider).should('be.visible').click('center');
+    // cy.get(sqlLabView.tooltip).should('be.visible');
+    cy.intercept(`/superset/explore_json/*`).as('slices');
+    cy.get(nativeFilters.applyFilter).click();
+    cy.wait('@slices');
+    // assert that the url contains 'native_filters' in the url
+    cy.url().then(u => {
+      const ur = new URL(u);
+      expect(ur.search).to.include('native_filters');
+      // assert that the start handle has a value
+      cy.get(nativeFilters.slider.startHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert that the end handle has a value
+      cy.get(nativeFilters.slider.endHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert slider text matches what we should have
+      cy.get(nativeFilters.slider.sliderText).should('have.text', '49');
+    });
+  });
+
+  it('Verify that default value is respected after revisit', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    inputNativeFilterDefaultValue(testItems.filterDefaultValue);
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.request(
+      'api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:100)',
+    ).then(xhr => {
+      const dashboards = xhr.body.result;
+      const testDashboard = dashboards.find(
+        (d: { dashboard_title: string }) =>
+          d.dashboard_title === testItems.dashboard,
+      );
+      cy.visit(testDashboard.url);
+    });
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+  });
+
+  it('Verify that allow for deleted filter restore', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();

Review Comment:
   The function `enterNativeFilterEditModal` is always dependent on `expandFilterOnLeftPanel`. We can call `expandFilterOnLeftPanel` from `enterNativeFilterEditModal` and avoid the repetition.



##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts:
##########
@@ -602,12 +332,314 @@ describe('Nativefilters Sanity test', () => {
       .invoke('text')
       .should('equal', '214 options', { timeout: 20000 });
     // apply first filter value and validate 2nd filter is depden on 1st filter.
-    applyNativeFilterValueWithIndex(0, 'East Asia & Pacific');
-
-    getNativeFilterPlaceholderWithIndex(0).should('have.text', '36 options', {
+    applyNativeFilterValueWithIndex(0, 'North America');
+    getNativeFilterPlaceholderWithIndex(0).should('have.text', '3 options', {
       timeout: 20000,
     });
   });
+
+  it('User can apply value filter with selected values', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    applyNativeFilterValueWithIndex(0, testItems.filterDefaultValue);
+    cy.get(nativeFilters.applyFilter).click();
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+  });
+
+  it('User can create a numerical range filter', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.numerical,
+      testItems.filterNumericalColumn,
+      testItems.datasetForNativeFilter,
+      testItems.filterNumericalColumn,
+    );
+    saveNativeFilterSettings();
+    // assertions
+    cy.get(nativeFilters.slider.slider).should('be.visible').click('center');
+    // cy.get(sqlLabView.tooltip).should('be.visible');
+    cy.intercept(`/superset/explore_json/*`).as('slices');
+    cy.get(nativeFilters.applyFilter).click();
+    cy.wait('@slices');
+    // assert that the url contains 'native_filters' in the url
+    cy.url().then(u => {
+      const ur = new URL(u);
+      expect(ur.search).to.include('native_filters');
+      // assert that the start handle has a value
+      cy.get(nativeFilters.slider.startHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert that the end handle has a value
+      cy.get(nativeFilters.slider.endHandle)
+        .invoke('attr', 'aria-valuenow')
+        .should('exist');
+      // assert slider text matches what we should have
+      cy.get(nativeFilters.slider.sliderText).should('have.text', '49');
+    });
+  });
+
+  it('Verify that default value is respected after revisit', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    inputNativeFilterDefaultValue(testItems.filterDefaultValue);
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.request(
+      'api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:100)',
+    ).then(xhr => {
+      const dashboards = xhr.body.result;
+      const testDashboard = dashboards.find(
+        (d: { dashboard_title: string }) =>
+          d.dashboard_title === testItems.dashboard,
+      );
+      cy.visit(testDashboard.url);
+    });
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+  });
+
+  it('Verify that allow for deleted filter restore', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    validateFilterNameOnDashboard(testItems.topTenChart.filterColumn);
+    enterNativeFilterEditModal();
+    cy.get(nativeFilters.filtersList.removeIcon).first().click();
+    cy.get('[data-test="restore-filter-button"]').should('be.visible').click();
+    cy.get(nativeFilters.modal.container)
+      .find(nativeFilters.filtersPanel.filterName)
+      .should('have.attr', 'value', testItems.topTenChart.filterColumn);
+  });
+
+  it('User can stop filtering when filter is removed', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    inputNativeFilterDefaultValue(testItems.filterDefaultValue);
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('not.exist');
+    });
+    cy.get(nativeFilters.filterItem)
+      .contains(testItems.filterDefaultValue)
+      .should('be.visible');
+    validateFilterNameOnDashboard(testItems.topTenChart.filterColumn);
+    enterNativeFilterEditModal();
+    deleteNativeFilter();
+    saveNativeFilterSettings();
+    WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
+    cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
+      cy.contains(testItems.filterDefaultValue).should('be.visible');
+      cy.contains(testItems.filterOtherCountry).should('be.visible');
+    });
+  });
+
+  it('user can delete dependent filter', () => {
+    expandFilterOnLeftPanel();
+    enterNativeFilterEditModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumnRegion,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumnRegion,
+    );
+    clickOnAddFilterInModal();
+    fillNativeFilterForm(
+      testItems.filterType.value,
+      testItems.topTenChart.filterColumn,
+      testItems.datasetForNativeFilter,
+      testItems.topTenChart.filterColumn,
+    );
+    cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+      () => {
+        cy.contains('Values are dependent on other filters')
+          .should('be.visible')
+          .click();
+      },
+    );
+    addParentFilterWithValue(0, testItems.topTenChart.filterColumnRegion);
+    // remove year native filter to cause it disappears from parent filter input in global sales
+    cy.get(nativeFilters.modal.tabsList.removeTab)
+      .should('be.visible')
+      .first()
+      .click();
+    // make sure you are seeing global sales filter which had parent filter
+    cy.get(nativeFilters.modal.tabsList.filterItemsContainer)
+      .children()
+      .last()
+      .click();
+    //
+    cy.wait(1000);
+    cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+      () => {
+        cy.contains('Values are dependent on other filters').should(
+          'not.exist',
+        );
+      },
+    );
+  });
+
+  it('User can create filter depend on 2 other filters', () => {

Review Comment:
   We could have more functions to reduce the code even more like `fillRegionFilter`, `fillCountryCodeFilter`, `fillCountryNameFilter` and reuse them everywhere. You can still keep the original function for other combinations if needed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org