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/22 16:41:12 UTC
[GitHub] [superset] jinghua-qa opened a new pull request, #19821: test(native filter): refactor and add new test
jinghua-qa opened a new pull request, #19821:
URL: https://github.com/apache/superset/pull/19821
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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
[GitHub] [superset] michael-s-molina commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r859778511
##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilter.helper.ts:
##########
@@ -0,0 +1,412 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { dashboardView, nativeFilters } from 'cypress/support/directories';
+import { testItems } from './dashboard.helper';
+
+export const nativeFilterTooltips = {
+ searchAllFilterOptions:
+ 'By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type (may add stress to your database).',
+ defaultToFirstItem: 'When using this option, default value can’t be set',
+ inverseSelection: 'Exclude selected values',
+ required: 'User must select a value before applying the filter',
+ multipleSelect: 'Allow selecting multiple values',
+ defaultValue:
+ 'Default value must be set when "Filter value is required" is checked',
+};
Review Comment:
I think using constants is a good idea but that would require changing the original components to also use these constants right?
--
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
[GitHub] [superset] zhaoyongjie commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r859914594
##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilter.helper.ts:
##########
@@ -0,0 +1,412 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { dashboardView, nativeFilters } from 'cypress/support/directories';
+import { testItems } from './dashboard.helper';
+
+export const nativeFilterTooltips = {
+ searchAllFilterOptions:
+ 'By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type (may add stress to your database).',
+ defaultToFirstItem: 'When using this option, default value can’t be set',
+ inverseSelection: 'Exclude selected values',
+ required: 'User must select a value before applying the filter',
+ multipleSelect: 'Allow selecting multiple values',
+ defaultValue:
+ 'Default value must be set when "Filter value is required" is checked',
+};
Review Comment:
The original components and test case might share common constants 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
[GitHub] [superset] jinghua-qa commented on pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #19821:
URL: https://github.com/apache/superset/pull/19821#issuecomment-1112384328
>
Done, PR description added
--
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
[GitHub] [superset] michael-s-molina commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [superset] jinghua-qa commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r858245578
##########
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:
done
--
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
[GitHub] [superset] jinghua-qa commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r858245519
##########
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:
done
##########
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:
done
--
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
[GitHub] [superset] jinghua-qa merged pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa merged PR #19821:
URL: https://github.com/apache/superset/pull/19821
--
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
[GitHub] [superset] jinghua-qa commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r860477231
##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilter.helper.ts:
##########
@@ -0,0 +1,412 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { dashboardView, nativeFilters } from 'cypress/support/directories';
+import { testItems } from './dashboard.helper';
+
+export const nativeFilterTooltips = {
+ searchAllFilterOptions:
+ 'By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type (may add stress to your database).',
+ defaultToFirstItem: 'When using this option, default value can’t be set',
+ inverseSelection: 'Exclude selected values',
+ required: 'User must select a value before applying the filter',
+ multipleSelect: 'Allow selecting multiple values',
+ defaultValue:
+ 'Default value must be set when "Filter value is required" is checked',
+};
Review Comment:
can i do this in a separated PR when it is ready?
--
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
[GitHub] [superset] jinghua-qa commented on pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #19821:
URL: https://github.com/apache/superset/pull/19821#issuecomment-1112384839
> Can you also add a PR description with the summary of your changes?
Done, added PR description
--
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
[GitHub] [superset] codecov[bot] commented on pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19821:
URL: https://github.com/apache/superset/pull/19821#issuecomment-1107403433
# [Codecov](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19821](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8b4ed13) into [master](https://codecov.io/gh/apache/superset/commit/fa680369ea66c9713d63b609df19bebe5cd99d08?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fa68036) will **increase** coverage by `0.00%`.
> The diff coverage is `22.22%`.
> :exclamation: Current head 8b4ed13 differs from pull request most recent head 5fe72e0. Consider uploading reports for the commit 5fe72e0 to get more accurate results
```diff
@@ Coverage Diff @@
## master #19821 +/- ##
=======================================
Coverage 66.54% 66.55%
=======================================
Files 1692 1692
Lines 64807 64802 -5
Branches 6661 6657 -4
=======================================
Hits 43129 43129
+ Misses 19978 19973 -5
Partials 1700 1700
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `51.25% <12.50%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.39% <ø> (ø)` | |
| [...abaseModal/DatabaseConnectionForm/TableCatalog.tsx](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0vVGFibGVDYXRhbG9nLnRzeA==) | `10.00% <ø> (ø)` | |
| [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `46.15% <0.00%> (-0.26%)` | :arrow_down: |
| [superset/dashboards/filters.py](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJzLnB5) | `93.67% <ø> (ø)` | |
| [superset/db\_engine\_specs/gsheets.py](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2dzaGVldHMucHk=) | `75.51% <ø> (ø)` | |
| [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `32.51% <14.28%> (+0.34%)` | :arrow_up: |
| [superset/utils/csv.py](https://codecov.io/gh/apache/superset/pull/19821/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY3N2LnB5) | `97.67% <100.00%> (+0.05%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fa68036...5fe72e0](https://codecov.io/gh/apache/superset/pull/19821?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [superset] zhaoyongjie commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r859488101
##########
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilter.helper.ts:
##########
@@ -0,0 +1,412 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { dashboardView, nativeFilters } from 'cypress/support/directories';
+import { testItems } from './dashboard.helper';
+
+export const nativeFilterTooltips = {
+ searchAllFilterOptions:
+ 'By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type (may add stress to your database).',
+ defaultToFirstItem: 'When using this option, default value can’t be set',
+ inverseSelection: 'Exclude selected values',
+ required: 'User must select a value before applying the filter',
+ multipleSelect: 'Allow selecting multiple values',
+ defaultValue:
+ 'Default value must be set when "Filter value is required" is checked',
+};
Review Comment:
I am considering the constants whether uses the _original text_ of components rather than hard code in the test case. How about you see this? @michael-s-molina
--
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
[GitHub] [superset] michael-s-molina commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r858633788
##########
superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts:
##########
@@ -159,97 +175,34 @@ export function cleanUp() {
}
/** ************************************************************************
- * Clicks on new filter button
+ * Copy dashboard for testing purpose
* @returns {None}
- * @summary helper for adding new filter
+ * @summary helper for copy dashboard for testing purpose
************************************************************************* */
-export function clickOnAddFilterInModal() {
- return cy
- .get(nativeFilters.addFilterButton.button)
- .first()
- .click()
- .then(() => {
- cy.get(nativeFilters.addFilterButton.dropdownItem)
- .contains('Filter')
- .click({ force: true });
- });
-}
-
-/** ************************************************************************
- * Fills value native filter form with basic information
- * @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(
- name: string,
- dataset: string,
- filterColumn: string,
-) {
- cy.get(nativeFilters.modal.container)
- .find(nativeFilters.filtersPanel.filterName)
- .last()
- .click({ scrollBehavior: false })
- .type(name, { scrollBehavior: false });
- cy.get(nativeFilters.modal.container)
- .find(nativeFilters.filtersPanel.datasetName)
- .last()
- .click({ scrollBehavior: false })
- .type(`${dataset}{enter}`, { scrollBehavior: false });
- cy.get(nativeFilters.silentLoading).should('not.exist');
- cy.get(nativeFilters.filtersPanel.filterInfoInput)
- .last()
+export function copyTestDashboard(dashboard: string) {
+ cy.intercept('POST', '**/copy_dash/**').as('copy');
+ cy.intercept('**/api/v1/dashboard/**').as('dashboard');
Review Comment:
`cy.intercept('**/api/v1/dashboard/**').as('dashboard');` should come after `cy.intercept('**/api/v1/dashboard/?q=**').as('dashboardsList');` because it's more generic.
--
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
[GitHub] [superset] michael-s-molina commented on pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #19821:
URL: https://github.com/apache/superset/pull/19821#issuecomment-1109744098
Can you also add a PR description with the summary of your changes?
--
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
[GitHub] [superset] jinghua-qa commented on a diff in pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on code in PR #19821:
URL: https://github.com/apache/superset/pull/19821#discussion_r861067440
##########
superset-frontend/cypress-base/cypress/integration/dashboard/dashboard.helper.ts:
##########
@@ -159,97 +175,34 @@ export function cleanUp() {
}
/** ************************************************************************
- * Clicks on new filter button
+ * Copy dashboard for testing purpose
* @returns {None}
- * @summary helper for adding new filter
+ * @summary helper for copy dashboard for testing purpose
************************************************************************* */
-export function clickOnAddFilterInModal() {
- return cy
- .get(nativeFilters.addFilterButton.button)
- .first()
- .click()
- .then(() => {
- cy.get(nativeFilters.addFilterButton.dropdownItem)
- .contains('Filter')
- .click({ force: true });
- });
-}
-
-/** ************************************************************************
- * Fills value native filter form with basic information
- * @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(
- name: string,
- dataset: string,
- filterColumn: string,
-) {
- cy.get(nativeFilters.modal.container)
- .find(nativeFilters.filtersPanel.filterName)
- .last()
- .click({ scrollBehavior: false })
- .type(name, { scrollBehavior: false });
- cy.get(nativeFilters.modal.container)
- .find(nativeFilters.filtersPanel.datasetName)
- .last()
- .click({ scrollBehavior: false })
- .type(`${dataset}{enter}`, { scrollBehavior: false });
- cy.get(nativeFilters.silentLoading).should('not.exist');
- cy.get(nativeFilters.filtersPanel.filterInfoInput)
- .last()
+export function copyTestDashboard(dashboard: string) {
+ cy.intercept('POST', '**/copy_dash/**').as('copy');
+ cy.intercept('**/api/v1/dashboard/**').as('dashboard');
Review Comment:
done
--
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
[GitHub] [superset] jinghua-qa commented on pull request #19821: test(native filter): refactor and add new test
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #19821:
URL: https://github.com/apache/superset/pull/19821#issuecomment-1112386048
> @jinghua-qa I executed the tests locally and it's really slow. The main reason is that we are recreating the dashboard and waiting for all charts to be loaded for each test as you can see in the `beforeEach` below:
>
> ```
> beforeEach(() => {
> cy.login();
> cleanUp();
> copyTestDashboard("World Bank's Data");
> WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
> closeDashboardToastMessage();
> });
> ```
>
> The majority of tests don't need a fresh initial state to execute. We can start the initial state with an existing filter and handle these types of tests without recreating the whole dashboard:
>
> * 'User can expand / retract native filter sidebar on a dashboard
> * Verify setting options and tooltips for value filter
> * User can check 'Filter has default value
> ...
>
> For tests like:
>
> * User can create a time range filter
> * User can create a time grain filter
> * User can create a time column filter
> ...
>
> We can just add new filters to execute the tests.
>
> Only a few tests will required a complete fresh initial state. Making these suggested changes will greatly improve execution time.
Separated the tests into 2 different setup suite
--
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