diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 19446c352a..3d8f606419 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -37,28 +37,6 @@ export interface CardSortSelectOption { value: any; } -type FilterOperator = - | 'sw' - | 'ew' - | 'ct' - | 'eq' - | 'nsw' - | 'new' - | 'nct' - | 'neq' - | 'gt' - | 'lt' - | 'rel_m_m' - | 'rel_o_m' - | 'title_or_slug' - | 'name_or_description' - | 'all_text' - | 'chart_all_text' - | 'dataset_is_null_or_empty' - | 'between' - | 'dashboard_is_favorite' - | 'chart_is_favorite'; - export interface Filter { Header: ReactNode; id: string; @@ -104,7 +82,7 @@ export interface InternalFilter extends FilterValue { Header?: string; } -export enum FilterOperators { +export enum FilterOperator { startsWith = 'sw', endsWith = 'ew', contains = 'ct', diff --git a/superset-frontend/src/views/CRUD/alert/AlertList.tsx b/superset-frontend/src/views/CRUD/alert/AlertList.tsx index f5fd9a35c0..29be77b5d7 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertList.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertList.tsx @@ -26,7 +26,7 @@ import Button from 'src/components/Button'; import FacePile from 'src/components/FacePile'; import { Tooltip } from 'src/components/Tooltip'; import ListView, { - FilterOperators, + FilterOperator, Filters, ListViewProps, } from 'src/components/ListView'; @@ -84,7 +84,7 @@ function AlertList({ () => [ { id: 'type', - operator: FilterOperators.equals, + operator: FilterOperator.equals, value: isReportEnabled ? 'Report' : 'Alert', }, ], @@ -373,7 +373,7 @@ function AlertList({ Header: t('Created by'), id: 'created_by', input: 'select', - operator: FilterOperators.relationOneMany, + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'report', @@ -389,7 +389,7 @@ function AlertList({ Header: t('Status'), id: 'last_state', input: 'select', - operator: FilterOperators.equals, + operator: FilterOperator.equals, unfilteredLabel: 'Any', selects: [ { label: t(`${AlertState.success}`), value: AlertState.success }, @@ -403,7 +403,7 @@ function AlertList({ Header: t('Search'), id: 'name', input: 'search', - operator: FilterOperators.contains, + operator: FilterOperator.contains, }, ], [], diff --git a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx index b52fd252d8..55f7da5a4c 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx +++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx @@ -27,7 +27,11 @@ import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu'; import ActionsBar, { ActionProps } from 'src/components/ListView/ActionsBar'; -import ListView, { ListViewProps, Filters } from 'src/components/ListView'; +import ListView, { + ListViewProps, + Filters, + FilterOperator, +} from 'src/components/ListView'; import Button from 'src/components/Button'; import DeleteModal from 'src/components/DeleteModal'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; @@ -286,7 +290,7 @@ function AnnotationLayersList({ Header: t('Created by'), id: 'created_by', input: 'select', - operator: 'rel_o_m', + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'annotation_layer', @@ -305,7 +309,7 @@ function AnnotationLayersList({ Header: t('Search'), id: 'name', input: 'search', - operator: 'ct', + operator: FilterOperator.contains, }, ], [], diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 7667b913e4..64e4cc5e33 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -44,7 +44,7 @@ import ListView, { ListViewProps, Filters, SelectOption, - FilterOperators, + FilterOperator, } from 'src/components/ListView'; import { getFromLocalStorage } from 'src/utils/localStorageHelpers'; import withToasts from 'src/messageToasts/enhancers/withToasts'; @@ -385,7 +385,7 @@ function ChartList(props: ChartListProps) { Header: t('Owner'), id: 'owners', input: 'select', - operator: FilterOperators.relationManyMany, + operator: FilterOperator.relationManyMany, unfilteredLabel: t('All'), fetchSelects: createFetchRelated( 'chart', @@ -406,7 +406,7 @@ function ChartList(props: ChartListProps) { Header: t('Created by'), id: 'created_by', input: 'select', - operator: FilterOperators.relationOneMany, + operator: FilterOperator.relationOneMany, unfilteredLabel: t('All'), fetchSelects: createFetchRelated( 'chart', @@ -427,7 +427,7 @@ function ChartList(props: ChartListProps) { Header: t('Viz type'), id: 'viz_type', input: 'select', - operator: FilterOperators.equals, + operator: FilterOperator.equals, unfilteredLabel: t('All'), selects: registry .keys() @@ -451,7 +451,7 @@ function ChartList(props: ChartListProps) { Header: t('Dataset'), id: 'datasource_id', input: 'select', - operator: FilterOperators.equals, + operator: FilterOperator.equals, unfilteredLabel: t('All'), fetchSelects: createFetchDatasets( createErrorHandler(errMsg => @@ -470,7 +470,7 @@ function ChartList(props: ChartListProps) { id: 'id', urlDisplay: 'favorite', input: 'select', - operator: FilterOperators.chartIsFav, + operator: FilterOperator.chartIsFav, unfilteredLabel: t('Any'), selects: [ { label: t('Yes'), value: true }, @@ -481,7 +481,7 @@ function ChartList(props: ChartListProps) { Header: t('Search'), id: 'slice_name', input: 'search', - operator: FilterOperators.chartAllText, + operator: FilterOperator.chartAllText, }, ]; diff --git a/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx b/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx index 4d6fd664d3..02ce896987 100644 --- a/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx +++ b/superset-frontend/src/views/CRUD/csstemplates/CssTemplatesList.tsx @@ -30,7 +30,11 @@ import DeleteModal from 'src/components/DeleteModal'; import { Tooltip } from 'src/components/Tooltip'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import ActionsBar, { ActionProps } from 'src/components/ListView/ActionsBar'; -import ListView, { ListViewProps, Filters } from 'src/components/ListView'; +import ListView, { + ListViewProps, + Filters, + FilterOperator, +} from 'src/components/ListView'; import CssTemplateModal from './CssTemplateModal'; import { TemplateObject } from './types'; @@ -272,7 +276,7 @@ function CssTemplatesList({ Header: t('Created by'), id: 'created_by', input: 'select', - operator: 'rel_o_m', + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'css_template', @@ -291,7 +295,7 @@ function CssTemplatesList({ Header: t('Search'), id: 'template_name', input: 'search', - operator: 'ct', + operator: FilterOperator.contains, }, ], [], diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 28efa59ac2..840bfd6a47 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -32,7 +32,7 @@ import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu'; import ListView, { ListViewProps, Filters, - FilterOperators, + FilterOperator, } from 'src/components/ListView'; import { getFromLocalStorage } from 'src/utils/localStorageHelpers'; import Owner from 'src/types/Owner'; @@ -46,6 +46,7 @@ import ImportModelsModal from 'src/components/ImportModal/index'; import Dashboard from 'src/dashboard/containers/Dashboard'; import DashboardCard from './DashboardCard'; +import { DashboardStatus } from './types'; const PAGE_SIZE = 25; const PASSWORDS_NEEDED_MESSAGE = t( @@ -230,9 +231,10 @@ function DashboardList(props: DashboardListProps) { { Cell: ({ row: { - original: { published }, + original: { status }, }, - }: any) => (published ? t('Published') : t('Draft')), + }: any) => + status === DashboardStatus.PUBLISHED ? t('Published') : t('Draft'), Header: t('Status'), accessor: 'published', size: 'xl', @@ -362,7 +364,7 @@ function DashboardList(props: DashboardListProps) { Header: t('Owner'), id: 'owners', input: 'select', - operator: FilterOperators.relationManyMany, + operator: FilterOperator.relationManyMany, unfilteredLabel: t('All'), fetchSelects: createFetchRelated( 'dashboard', @@ -383,7 +385,7 @@ function DashboardList(props: DashboardListProps) { Header: t('Created by'), id: 'created_by', input: 'select', - operator: FilterOperators.relationOneMany, + operator: FilterOperator.relationOneMany, unfilteredLabel: t('All'), fetchSelects: createFetchRelated( 'dashboard', @@ -404,11 +406,11 @@ function DashboardList(props: DashboardListProps) { Header: t('Status'), id: 'published', input: 'select', - operator: FilterOperators.equals, + operator: FilterOperator.equals, unfilteredLabel: t('Any'), selects: [ { label: t('Published'), value: true }, - { label: t('Unpublished'), value: false }, + { label: t('Draft'), value: false }, ], }, { @@ -416,7 +418,7 @@ function DashboardList(props: DashboardListProps) { id: 'id', urlDisplay: 'favorite', input: 'select', - operator: FilterOperators.dashboardIsFav, + operator: FilterOperator.dashboardIsFav, unfilteredLabel: t('Any'), selects: [ { label: t('Yes'), value: true }, @@ -427,7 +429,7 @@ function DashboardList(props: DashboardListProps) { Header: t('Search'), id: 'dashboard_title', input: 'search', - operator: FilterOperators.titleOrSlug, + operator: FilterOperator.titleOrSlug, }, ]; diff --git a/superset-frontend/src/views/CRUD/dashboard/types.ts b/superset-frontend/src/views/CRUD/dashboard/types.ts index 5f442effc1..93be661803 100644 --- a/superset-frontend/src/views/CRUD/dashboard/types.ts +++ b/superset-frontend/src/views/CRUD/dashboard/types.ts @@ -24,3 +24,8 @@ export type DashboardObject = { position?: string; metadata?: string; }; + +export enum DashboardStatus { + PUBLISHED = 'published', + DRAFT = 'draft', +} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index 81c99105c8..2b7be5e70a 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -27,7 +27,7 @@ import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu'; import DeleteModal from 'src/components/DeleteModal'; import { Tooltip } from 'src/components/Tooltip'; import Icons from 'src/components/Icons'; -import ListView, { Filters } from 'src/components/ListView'; +import ListView, { FilterOperator, Filters } from 'src/components/ListView'; import { commonMenuData } from 'src/views/CRUD/data/common'; import ImportModelsModal from 'src/components/ImportModal/index'; import DatabaseModal from './DatabaseModal'; @@ -374,7 +374,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { Header: t('Expose in SQL Lab'), id: 'expose_in_sqllab', input: 'select', - operator: 'eq', + operator: FilterOperator.equals, unfilteredLabel: 'All', selects: [ { label: 'Yes', value: true }, @@ -393,7 +393,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { ), id: 'allow_run_async', input: 'select', - operator: 'eq', + operator: FilterOperator.equals, unfilteredLabel: 'All', selects: [ { label: 'Yes', value: true }, @@ -404,7 +404,7 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { Header: t('Search'), id: 'database_name', input: 'search', - operator: 'ct', + operator: FilterOperator.contains, }, ], [], diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index e37c6fd27f..06b91b5651 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -33,7 +33,11 @@ import { useListViewResource } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import DatasourceModal from 'src/datasource/DatasourceModal'; import DeleteModal from 'src/components/DeleteModal'; -import ListView, { ListViewProps, Filters } from 'src/components/ListView'; +import ListView, { + ListViewProps, + Filters, + FilterOperator, +} from 'src/components/ListView'; import SubMenu, { SubMenuProps, ButtonProps, @@ -308,7 +312,7 @@ const DatasetList: FunctionComponent = ({ { Cell: ({ row: { - original: { owners = [], table_name: tableName }, + original: { owners = [] }, }, }: any) => , Header: t('Owners'), @@ -397,7 +401,7 @@ const DatasetList: FunctionComponent = ({ Header: t('Owner'), id: 'owners', input: 'select', - operator: 'rel_m_m', + operator: FilterOperator.relationManyMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'dataset', @@ -416,7 +420,7 @@ const DatasetList: FunctionComponent = ({ Header: t('Database'), id: 'database', input: 'select', - operator: 'rel_o_m', + operator: FilterOperator.relationManyMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'dataset', @@ -431,7 +435,7 @@ const DatasetList: FunctionComponent = ({ Header: t('Schema'), id: 'schema', input: 'select', - operator: 'eq', + operator: FilterOperator.equals, unfilteredLabel: 'All', fetchSelects: createFetchDistinct( 'dataset', @@ -446,7 +450,7 @@ const DatasetList: FunctionComponent = ({ Header: t('Type'), id: 'sql', input: 'select', - operator: 'dataset_is_null_or_empty', + operator: FilterOperator.datasetIsNullOrEmpty, unfilteredLabel: 'All', selects: [ { label: 'Virtual', value: false }, @@ -457,7 +461,7 @@ const DatasetList: FunctionComponent = ({ Header: t('Search'), id: 'table_name', input: 'search', - operator: 'ct', + operator: FilterOperator.contains, }, ], [], diff --git a/superset-frontend/src/views/CRUD/data/query/QueryList.tsx b/superset-frontend/src/views/CRUD/data/query/QueryList.tsx index 49d7feaf80..88d8dec34e 100644 --- a/superset-frontend/src/views/CRUD/data/query/QueryList.tsx +++ b/superset-frontend/src/views/CRUD/data/query/QueryList.tsx @@ -32,7 +32,7 @@ import Popover from 'src/components/Popover'; import { commonMenuData } from 'src/views/CRUD/data/common'; import ListView, { Filters, - FilterOperators, + FilterOperator, ListViewProps, } from 'src/components/ListView'; import Icon, { IconName } from 'src/components/Icon'; @@ -340,7 +340,7 @@ function QueryList({ addDangerToast, addSuccessToast }: QueryListProps) { Header: t('Database'), id: 'database', input: 'select', - operator: FilterOperators.relationOneMany, + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'query', @@ -357,7 +357,7 @@ function QueryList({ addDangerToast, addSuccessToast }: QueryListProps) { Header: t('State'), id: 'status', input: 'select', - operator: FilterOperators.equals, + operator: FilterOperator.equals, unfilteredLabel: 'All', fetchSelects: createFetchDistinct( 'query', @@ -374,7 +374,7 @@ function QueryList({ addDangerToast, addSuccessToast }: QueryListProps) { Header: t('User'), id: 'user', input: 'select', - operator: FilterOperators.relationOneMany, + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'query', @@ -391,13 +391,13 @@ function QueryList({ addDangerToast, addSuccessToast }: QueryListProps) { Header: t('Time range'), id: 'start_time', input: 'datetime_range', - operator: FilterOperators.between, + operator: FilterOperator.between, }, { Header: t('Search by query text'), id: 'sql', input: 'search', - operator: FilterOperators.contains, + operator: FilterOperator.contains, }, ], [addDangerToast], diff --git a/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx b/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx index 3e23fcac7c..04a418b39d 100644 --- a/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx +++ b/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx @@ -35,7 +35,11 @@ import SubMenu, { SubMenuProps, ButtonProps, } from 'src/components/Menu/SubMenu'; -import ListView, { ListViewProps, Filters } from 'src/components/ListView'; +import ListView, { + ListViewProps, + Filters, + FilterOperator, +} from 'src/components/ListView'; import DeleteModal from 'src/components/DeleteModal'; import ActionsBar, { ActionProps } from 'src/components/ListView/ActionsBar'; import { Tooltip } from 'src/components/Tooltip'; @@ -411,7 +415,7 @@ function SavedQueryList({ Header: t('Database'), id: 'database', input: 'select', - operator: 'rel_o_m', + operator: FilterOperator.relationOneMany, unfilteredLabel: 'All', fetchSelects: createFetchRelated( 'saved_query', @@ -431,7 +435,7 @@ function SavedQueryList({ Header: t('Schema'), id: 'schema', input: 'select', - operator: 'eq', + operator: FilterOperator.equals, unfilteredLabel: 'All', fetchSelects: createFetchDistinct( 'saved_query', @@ -448,7 +452,7 @@ function SavedQueryList({ Header: t('Search'), id: 'label', input: 'search', - operator: 'all_text', + operator: FilterOperator.allText, }, ], [addDangerToast], diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 458e59c47d..db807e8daa 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -52,8 +52,8 @@ from superset.dashboards.commands.importers.dispatcher import ImportDashboardsCo from superset.dashboards.commands.update import UpdateDashboardCommand from superset.dashboards.dao import DashboardDAO from superset.dashboards.filters import ( + DashboardAccessFilter, DashboardFavoriteFilter, - DashboardFilter, DashboardTitleOrSlugFilter, FilterRelatedRoles, ) @@ -105,6 +105,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): list_columns = [ "id", "published", + "status", "slug", "url", "css", @@ -153,13 +154,13 @@ class DashboardRestApi(BaseSupersetModelRestApi): search_columns = ( "created_by", + "changed_by", "dashboard_title", "id", "owners", - "roles", "published", + "roles", "slug", - "changed_by", ) search_filters = { "dashboard_title": [DashboardTitleOrSlugFilter], @@ -173,7 +174,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): dashboard_get_response_schema = DashboardGetResponseSchema() dashboard_dataset_schema = DashboardDatasetSchema() - base_filters = [["slice", DashboardFilter, lambda: []]] + base_filters = [["id", DashboardAccessFilter, lambda: []]] order_rel_fields = { "slices": ("slice_name", "asc"), diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 800ee66c90..707da7bc8a 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -18,16 +18,15 @@ import json import logging from typing import Any, Dict, List, Optional -from flask_appbuilder.models.sqla.interface import SQLAInterface from sqlalchemy.exc import SQLAlchemyError -from sqlalchemy.orm import contains_eager +from superset import security_manager from superset.dao.base import BaseDAO from superset.dashboards.commands.exceptions import DashboardNotFoundError -from superset.dashboards.filters import DashboardFilter +from superset.dashboards.filters import DashboardAccessFilter from superset.extensions import db from superset.models.core import FavStar, FavStarClassName -from superset.models.dashboard import Dashboard, id_or_slug_filter +from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.utils import core from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes @@ -37,42 +36,19 @@ logger = logging.getLogger(__name__) class DashboardDAO(BaseDAO): model_cls = Dashboard - base_filter = DashboardFilter + base_filter = DashboardAccessFilter @staticmethod def get_by_id_or_slug(id_or_slug: str) -> Dashboard: - query = ( - db.session.query(Dashboard) - .filter(id_or_slug_filter(id_or_slug)) - .outerjoin(Slice, Dashboard.slices) - .outerjoin(Slice.table) - .outerjoin(Dashboard.owners) - .outerjoin(Dashboard.roles) - ) - # Apply dashboard base filters - query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply( - query, None - ) - dashboard = query.one_or_none() + dashboard = Dashboard.get(id_or_slug) if not dashboard: raise DashboardNotFoundError() + security_manager.raise_for_dashboard_access(dashboard) return dashboard @staticmethod def get_datasets_for_dashboard(id_or_slug: str) -> List[Any]: - query = ( - db.session.query(Dashboard) - .filter(id_or_slug_filter(id_or_slug)) - .outerjoin(Slice, Dashboard.slices) - .outerjoin(Slice.table) - ) - # Apply dashboard base filters - query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply( - query, None - ) - dashboard = query.one_or_none() - if not dashboard: - raise DashboardNotFoundError() + dashboard = DashboardDAO.get_by_id_or_slug(id_or_slug) datasource_slices = core.indexed(dashboard.slices, "datasource") data = [ datasource.data_for_slices(slices) @@ -83,22 +59,7 @@ class DashboardDAO(BaseDAO): @staticmethod def get_charts_for_dashboard(id_or_slug: str) -> List[Slice]: - query = ( - db.session.query(Dashboard) - .outerjoin(Slice, Dashboard.slices) - .outerjoin(Slice.table) - .filter(id_or_slug_filter(id_or_slug)) - .options(contains_eager(Dashboard.slices)) - ) - # Apply dashboard base filters - query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply( - query, None - ) - - dashboard = query.one_or_none() - if not dashboard: - raise DashboardNotFoundError() - return dashboard.slices + return DashboardDAO.get_by_id_or_slug(id_or_slug).slices @staticmethod def validate_slug_uniqueness(slug: str) -> bool: diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index d7216ce919..12cdc7fe40 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -57,16 +57,16 @@ class DashboardFavoriteFilter(BaseFavoriteFilter): model = Dashboard -class DashboardFilter(BaseFilter): +class DashboardAccessFilter(BaseFilter): """ List dashboards with the following criteria: 1. Those which the user owns 2. Those which the user has favorited 3. Those which have been published (if they have access to at least one slice) - If the user is an admin show them all dashboards. + If the user is an admin then show all dashboards. This means they do not get curation but can still sort by "published" - if they wish to see those dashboards which are published first + if they wish to see those dashboards which are published first. """ def apply(self, query: Query, value: Any) -> Query: diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 0f21c524a5..774cf621de 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -184,6 +184,12 @@ class Dashboard( # pylint: disable=too-many-instance-attributes meta = MetaData(bind=self.get_sqla_engine()) meta.reflect() + @property + def status(self) -> utils.DashboardStatus: + if self.published: + return utils.DashboardStatus.PUBLISHED + return utils.DashboardStatus.DRAFT + @renders("dashboard_title") def dashboard_link(self) -> Markup: title = escape(self.dashboard_title or "") diff --git a/superset/reports/api.py b/superset/reports/api.py index 5fc2808b60..0b495ef864 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -25,7 +25,7 @@ from marshmallow import ValidationError from superset.charts.filters import ChartFilter from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod -from superset.dashboards.filters import DashboardFilter +from superset.dashboards.filters import DashboardAccessFilter from superset.databases.filters import DatabaseFilter from superset.models.reports import ReportSchedule from superset.reports.commands.bulk_delete import BulkDeleteReportScheduleCommand @@ -170,7 +170,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): allowed_rel_fields = {"owners", "chart", "dashboard", "database", "created_by"} filter_rel_fields = { "chart": [["id", ChartFilter, lambda: []]], - "dashboard": [["id", DashboardFilter, lambda: []]], + "dashboard": [["id", DashboardAccessFilter, lambda: []]], "database": [["id", DatabaseFilter, lambda: []]], } text_field_rel_fields = { diff --git a/superset/security/manager.py b/superset/security/manager.py index 7a62e1a436..73672bb964 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1139,7 +1139,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods @staticmethod def can_access_based_on_dashboard(datasource: "BaseDatasource") -> bool: from superset import db - from superset.dashboards.filters import DashboardFilter + from superset.dashboards.filters import DashboardAccessFilter from superset.models.slice import Slice from superset.models.dashboard import Dashboard @@ -1150,7 +1150,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods .filter(datasource_class.id == datasource.id) ) - query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply( + query = DashboardAccessFilter("id", SQLAInterface(Dashboard, db.session)).apply( query, None ) diff --git a/superset/utils/core.py b/superset/utils/core.py index a9d1bb6044..4e9e470739 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -263,6 +263,13 @@ class QueryStatus(str, Enum): # pylint: disable=too-few-public-methods TIMED_OUT: str = "timed_out" +class DashboardStatus(str, Enum): + """Dashboard status used for frontend filters""" + + PUBLISHED = "published" + DRAFT = "draft" + + class ReservedUrlParameters(str, Enum): """ Reserved URL parameters that are used internally by Superset. These will not be diff --git a/superset/views/chart/mixin.py b/superset/views/chart/mixin.py index 467d53e836..186e86904a 100644 --- a/superset/views/chart/mixin.py +++ b/superset/views/chart/mixin.py @@ -17,7 +17,7 @@ from flask import Markup from flask_babel import lazy_gettext as _ -from superset.dashboards.filters import DashboardFilter +from superset.dashboards.filters import DashboardAccessFilter from superset.views.chart.filters import SliceFilter @@ -88,6 +88,6 @@ class SliceMixin: # pylint: disable=too-few-public-methods "viz_type": _("Visualization Type"), } - add_form_query_rel_fields = {"dashboards": [["name", DashboardFilter, None]]} + add_form_query_rel_fields = {"dashboards": [["name", DashboardAccessFilter, None]]} edit_form_query_rel_fields = add_form_query_rel_fields diff --git a/superset/views/dashboard/mixin.py b/superset/views/dashboard/mixin.py index 9a5510728b..a01354615a 100644 --- a/superset/views/dashboard/mixin.py +++ b/superset/views/dashboard/mixin.py @@ -16,7 +16,7 @@ # under the License. from flask_babel import lazy_gettext as _ -from ...dashboards.filters import DashboardFilter +from ...dashboards.filters import DashboardAccessFilter from ..base import check_ownership @@ -73,7 +73,7 @@ class DashboardMixin: # pylint: disable=too-few-public-methods "visible in the list of all dashboards" ), } - base_filters = [["slice", DashboardFilter, lambda: []]] + base_filters = [["slice", DashboardAccessFilter, lambda: []]] label_columns = { "dashboard_link": _("Dashboard"), "dashboard_title": _("Title"), diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index 97d9e7b544..f88d3a4aa3 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -437,11 +437,11 @@ class TestDashboard(SupersetTestCase): self.test_save_dash("alpha") @pytest.mark.usefixtures("load_energy_table_with_slice", "load_dashboard") - def test_users_can_view_published_dashboard(self): + def test_users_can_list_published_dashboard(self): self.login("alpha") resp = self.get_resp("/api/v1/dashboard/") - self.assertNotIn(f"/superset/dashboard/{pytest.hidden_dash_slug}/", resp) - self.assertIn(f"/superset/dashboard/{pytest.published_dash_slug}/", resp) + assert f"/superset/dashboard/{pytest.hidden_dash_slug}/" not in resp + assert f"/superset/dashboard/{pytest.published_dash_slug}/" in resp def test_users_can_view_own_dashboard(self): user = security_manager.find_user("gamma") diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index 4a329e4776..02be01e32f 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -190,11 +190,14 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi self.assertEqual(response.status_code, 404) @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") - def test_get_dashboard_datasets_not_allowed(self): + def test_get_draft_dashboard_datasets(self): + """ + All users should have access to dashboards without roles + """ self.login(username="gamma") uri = "api/v1/dashboard/world_health/datasets" response = self.get_assert_metric(uri, "get_datasets") - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) @pytest.mark.usefixtures("create_dashboards") def get_dashboard_by_slug(self): @@ -215,12 +218,15 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi self.assertEqual(response.status_code, 404) @pytest.mark.usefixtures("create_dashboards") - def get_dashboard_by_slug_not_allowed(self): + def get_draft_dashboard_by_slug(self): + """ + All users should have access to dashboards without roles + """ self.login(username="gamma") dashboard = self.dashboards[0] uri = f"api/v1/dashboard/{dashboard.slug}" response = self.get_assert_metric(uri, "get") - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 200) @pytest.mark.usefixtures("create_dashboards") def test_get_dashboard_charts(self): @@ -266,15 +272,15 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi self.assertEqual(response.status_code, 404) @pytest.mark.usefixtures("create_dashboards") - def test_get_dashboard_charts_not_allowed(self): + def test_get_draft_dashboard_charts(self): """ - Dashboard API: Test getting charts on a dashboard a user does not have access to + All users should have access to draft dashboards without roles """ self.login(username="gamma") dashboard = self.dashboards[0] uri = f"api/v1/dashboard/{dashboard.id}/charts" response = self.get_assert_metric(uri, "get_charts") - self.assertEqual(response.status_code, 404) + assert response.status_code == 200 @pytest.mark.usefixtures("create_dashboards") def test_get_dashboard_charts_empty(self): @@ -382,7 +388,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi self.login(username="gamma") uri = f"api/v1/dashboard/{dashboard.id}" rv = self.client.get(uri) - self.assertEqual(rv.status_code, 404) + self.assertEqual(rv.status_code, 200) # rollback changes db.session.delete(dashboard) db.session.commit() diff --git a/tests/dashboards/security/security_dataset_tests.py b/tests/dashboards/security/security_dataset_tests.py index 9edc2ab0a1..c7233ebd86 100644 --- a/tests/dashboards/security/security_dataset_tests.py +++ b/tests/dashboards/security/security_dataset_tests.py @@ -208,20 +208,6 @@ class TestDashboardDatasetSecurity(DashboardTestCase): finally: self.revoke_public_access_to_table(accessed_table) - def test_get_dashboard_api_no_data_access(self): - """ - Dashboard API: Test get dashboard without data access - """ - admin = self.get_user("admin") - dashboard = create_dashboard_to_db( - random_title(), random_slug(), owners=[admin] - ) - - self.login(username="gamma") - uri = DASHBOARD_API_URL_FORMAT.format(dashboard.id) - rv = self.client.get(uri) - self.assert404(rv) - def test_get_dashboards_api_no_data_access(self): """ Dashboard API: Test get dashboards no data access