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 2021/01/25 08:15:47 UTC
[GitHub] [superset] villebro commented on a change in pull request #12716: feat(native-filters): apply scoping of native filters to dashboard
villebro commented on a change in pull request #12716:
URL: https://github.com/apache/superset/pull/12716#discussion_r563520165
##########
File path: superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx
##########
@@ -28,10 +28,17 @@ import newComponentFactory from 'src/dashboard/util/newComponentFactory';
// mock data
import chartQueries from 'spec/fixtures/mockChartQueries';
import datasources from 'spec/fixtures/mockDatasource';
+import {
+ extraFormData,
+ NATIVE_FILTER_ID,
+ layoutForSingleNativeFilter,
+ singleNativeFiltersState,
+} from 'spec/fixtures/mockNativeFilters';
import dashboardInfo from 'spec/fixtures/mockDashboardInfo';
import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout';
import dashboardState from 'spec/fixtures/mockDashboardState';
import { sliceEntitiesForChart as sliceEntities } from 'spec/fixtures/mockSliceEntities';
+import { getActiveNativeFilters } from '../../../../src/dashboard/util/activeDashboardNativeFilters';
Review comment:
nit:
```suggestion
import { getActiveNativeFilters } from 'src/dashboard/util/activeDashboardNativeFilters';
```
##########
File path: superset-frontend/spec/fixtures/mockNativeFilters.ts
##########
@@ -88,3 +88,117 @@ export const nativeFilters: NativeFiltersState = {
},
},
};
+
+export const extraFormData = {
+ append_form_data: {
+ filters: [
+ {
+ col: 'ethnic_minority',
+ op: 'IN',
+ val: 'No, not an ethnic minority',
+ },
+ ],
+ },
+};
+
+export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA';
+
+export const singleNativeFiltersState = {
+ filters: {
+ [NATIVE_FILTER_ID]: {
+ id: [NATIVE_FILTER_ID],
+ name: 'eth',
+ type: 'text',
+ targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
+ defaultValue: null,
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] },
+ inverseSelection: false,
+ isInstant: true,
+ allowsMultipleValues: false,
+ isRequired: false,
+ },
+ },
+ filtersState: {
+ [NATIVE_FILTER_ID]: {
+ id: NATIVE_FILTER_ID,
+ extraFormData,
+ },
+ },
+};
+
+export const layoutForSingleNativeFilter = {
+ 'CHART-ZHVS7YasaQ': {
+ children: [],
+ id: 'CHART-ZHVS7YasaQ',
+ meta: {
+ chartId: 230,
+ height: 50,
+ sliceName: 'Pie',
+ uuid: '05ef6145-3950-4f59-891f-160852613eca',
+ width: 12,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-NweUz7oC0'],
+ type: 'CHART',
+ },
+ 'CHART-gsGu8NIKQT': {
+ children: [],
+ id: 'CHART-gsGu8NIKQT',
+ meta: {
+ chartId: 227,
+ height: 50,
+ sliceName: 'Select filter',
+ uuid: 'ddb78f6c-7876-47fc-ae98-70183b05ba90',
+ width: 4,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-QkiTjeZGs'],
+ type: 'CHART',
+ },
+ 'CHART-hgYjD8axJX': {
+ children: [],
+ id: 'CHART-hgYjD8axJX',
+ meta: {
+ chartId: 229,
+ height: 47,
+ sliceName: 'Bars 2',
Review comment:
If there isn't a "Bars" or "Bars 1", this could perhaps be "Bar Chart", "My Bar Chart" or similar.
##########
File path: superset-frontend/spec/fixtures/mockNativeFilters.ts
##########
@@ -88,3 +88,117 @@ export const nativeFilters: NativeFiltersState = {
},
},
};
+
+export const extraFormData = {
+ append_form_data: {
+ filters: [
+ {
+ col: 'ethnic_minority',
+ op: 'IN',
+ val: 'No, not an ethnic minority',
+ },
+ ],
+ },
+};
+
+export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA';
+
+export const singleNativeFiltersState = {
+ filters: {
+ [NATIVE_FILTER_ID]: {
+ id: [NATIVE_FILTER_ID],
+ name: 'eth',
+ type: 'text',
+ targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
+ defaultValue: null,
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] },
+ inverseSelection: false,
+ isInstant: true,
+ allowsMultipleValues: false,
+ isRequired: false,
+ },
+ },
+ filtersState: {
+ [NATIVE_FILTER_ID]: {
+ id: NATIVE_FILTER_ID,
+ extraFormData,
+ },
+ },
+};
+
+export const layoutForSingleNativeFilter = {
+ 'CHART-ZHVS7YasaQ': {
+ children: [],
+ id: 'CHART-ZHVS7YasaQ',
+ meta: {
+ chartId: 230,
+ height: 50,
+ sliceName: 'Pie',
+ uuid: '05ef6145-3950-4f59-891f-160852613eca',
+ width: 12,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-NweUz7oC0'],
+ type: 'CHART',
+ },
+ 'CHART-gsGu8NIKQT': {
+ children: [],
+ id: 'CHART-gsGu8NIKQT',
+ meta: {
+ chartId: 227,
+ height: 50,
+ sliceName: 'Select filter',
Review comment:
It would be nice to have very generic names for these to reduce cognitive load. Especially mixing in (forthcoming) filter charts with regular charts in this context can be confusing. Could this be "Line" or even "Another Chart" or similar to make it clear that it's just a regular vanilla chart in a dashboard without any significant properties?
##########
File path: superset-frontend/spec/fixtures/mockNativeFilters.ts
##########
@@ -88,3 +88,117 @@ export const nativeFilters: NativeFiltersState = {
},
},
};
+
+export const extraFormData = {
+ append_form_data: {
+ filters: [
+ {
+ col: 'ethnic_minority',
+ op: 'IN',
+ val: 'No, not an ethnic minority',
+ },
+ ],
+ },
+};
+
+export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA';
+
+export const singleNativeFiltersState = {
+ filters: {
+ [NATIVE_FILTER_ID]: {
+ id: [NATIVE_FILTER_ID],
+ name: 'eth',
+ type: 'text',
+ targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
+ defaultValue: null,
+ cascadeParentIds: [],
+ scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] },
+ inverseSelection: false,
+ isInstant: true,
+ allowsMultipleValues: false,
+ isRequired: false,
+ },
+ },
+ filtersState: {
+ [NATIVE_FILTER_ID]: {
+ id: NATIVE_FILTER_ID,
+ extraFormData,
+ },
+ },
+};
+
+export const layoutForSingleNativeFilter = {
+ 'CHART-ZHVS7YasaQ': {
+ children: [],
+ id: 'CHART-ZHVS7YasaQ',
+ meta: {
+ chartId: 230,
+ height: 50,
+ sliceName: 'Pie',
+ uuid: '05ef6145-3950-4f59-891f-160852613eca',
+ width: 12,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-NweUz7oC0'],
+ type: 'CHART',
+ },
+ 'CHART-gsGu8NIKQT': {
+ children: [],
+ id: 'CHART-gsGu8NIKQT',
+ meta: {
+ chartId: 227,
+ height: 50,
+ sliceName: 'Select filter',
+ uuid: 'ddb78f6c-7876-47fc-ae98-70183b05ba90',
+ width: 4,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-QkiTjeZGs'],
+ type: 'CHART',
+ },
+ 'CHART-hgYjD8axJX': {
+ children: [],
+ id: 'CHART-hgYjD8axJX',
+ meta: {
+ chartId: 229,
+ height: 47,
+ sliceName: 'Bars 2',
+ uuid: 'e1501e54-d632-4fdc-ae16-07cafee31093',
+ width: 12,
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-mcdVZi0rL'],
+ type: 'CHART',
+ },
+ DASHBOARD_VERSION_KEY: 'v2',
+ GRID_ID: {
+ children: ['ROW-mcdVZi0rL', 'ROW-NweUz7oC0', 'ROW-QkiTjeZGs'],
+ id: 'GRID_ID',
+ parents: ['ROOT_ID'],
+ type: 'GRID',
+ },
+ HEADER_ID: {
+ id: 'HEADER_ID',
+ type: 'HEADER',
+ meta: { text: '[ untitled dashboard ]' },
Review comment:
Same here - "My Native Filter Dashboard"?
----------------------------------------------------------------
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.
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