diff --git a/superset-frontend/src/features/allEntities/AllEntitiesTable.test.tsx b/superset-frontend/src/features/allEntities/AllEntitiesTable.test.tsx index 7143c908879..fe9dfde55e3 100644 --- a/superset-frontend/src/features/allEntities/AllEntitiesTable.test.tsx +++ b/superset-frontend/src/features/allEntities/AllEntitiesTable.test.tsx @@ -91,12 +91,13 @@ describe('AllEntitiesTable', () => { jest.restoreAllMocks(); }); - it('renders when empty', () => { + it('renders when empty with button to tag if user has perm', () => { render( , { useRouter: true }, ); @@ -108,25 +109,70 @@ describe('AllEntitiesTable', () => { expect(screen.getByText('Add tag to entities')).toBeInTheDocument(); }); + it('renders when empty without button to tag if user does not have perm', () => { + render( + , + { useRouter: true }, + ); + + expect( + screen.getByText('No entities have this tag currently assigned'), + ).toBeInTheDocument(); + + expect(screen.queryByText('Add tag to entities')).not.toBeInTheDocument(); + }); + it('renders the correct tags for each object type, excluding the current tag', () => { render( , { useRouter: true }, ); + expect(screen.getByText('Dashboards')).toBeInTheDocument(); expect(screen.getByText('Sales Dashboard')).toBeInTheDocument(); expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Charts')).toBeInTheDocument(); expect(screen.getByText('Monthly Revenue')).toBeInTheDocument(); expect(screen.getByText('Revenue')).toBeInTheDocument(); + expect(screen.getByText('Queries')).toBeInTheDocument(); expect(screen.getByText('User Engagement')).toBeInTheDocument(); expect(screen.getByText('Engagement')).toBeInTheDocument(); expect(screen.queryByText('Current Tag')).not.toBeInTheDocument(); }); + + it('Only list asset types that have entities', () => { + const mockObjects = { + dashboard: [], + chart: [mockObjectsWithTags.chart[0]], + query: [], + }; + + render( + , + { useRouter: true }, + ); + + expect(screen.queryByText('Dashboards')).not.toBeInTheDocument(); + expect(screen.getByText('Charts')).toBeInTheDocument(); + expect(screen.getByText('Monthly Revenue')).toBeInTheDocument(); + expect(screen.queryByText('Queries')).not.toBeInTheDocument(); + }); }); diff --git a/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx b/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx index 71088d3f809..c5261e96efc 100644 --- a/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx +++ b/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx @@ -53,20 +53,22 @@ interface AllEntitiesTableProps { search?: string; setShowTagModal: (show: boolean) => void; objects: TaggedObjects; + canEditTag: boolean; } export default function AllEntitiesTable({ search = '', setShowTagModal, objects, + canEditTag, }: AllEntitiesTableProps) { type objectType = 'dashboard' | 'chart' | 'query'; const [tagId] = useQueryParam('id', NumberParam); - const showListViewObjs = - objects.dashboard.length > 0 || - objects.chart.length > 0 || - objects.query.length > 0; + const showDashboardList = objects.dashboard.length > 0; + const showChartList = objects.chart.length > 0; + const showQueryList = objects.query.length > 0; + const showListViewObjs = showDashboardList || showChartList || showQueryList; const renderTable = (type: objectType) => { const data = objects[type].map((o: TaggedObject) => ({ @@ -134,20 +136,34 @@ export default function AllEntitiesTable({ {showListViewObjs ? ( <> -
{t('Dashboards')}
- {renderTable('dashboard')} -
{t('Charts')}
- {renderTable('chart')} -
{t('Queries')}
- {renderTable('query')} + {showDashboardList && ( + <> +
{t('Dashboards')}
+ {renderTable('dashboard')} + + )} + {showChartList && ( + <> +
{t('Charts')}
+ {renderTable('chart')} + + )} + {showQueryList && ( + <> +
{t('Queries')}
+ {renderTable('query')} + + )} ) : ( setShowTagModal(true)} - buttonText={t('Add tag to entities')} + {...(canEditTag && { + buttonAction: () => setShowTagModal(true), + buttonText: t('Add tag to entities'), + })} /> )}
diff --git a/superset-frontend/src/pages/AllEntities/index.tsx b/superset-frontend/src/pages/AllEntities/index.tsx index 77391746ee2..40e1f552ea8 100644 --- a/superset-frontend/src/pages/AllEntities/index.tsx +++ b/superset-frontend/src/pages/AllEntities/index.tsx @@ -35,6 +35,9 @@ import { fetchObjectsByTagIds, fetchSingleTag } from 'src/features/tags/tags'; import Loading from 'src/components/Loading'; import getOwnerName from 'src/utils/getOwnerName'; import { TaggedObject, TaggedObjects } from 'src/types/TaggedObject'; +import { findPermission } from 'src/utils/findPermission'; +import { useSelector } from 'react-redux'; +import { RootState } from 'src/dashboard/types'; const additionalItemsStyles = (theme: SupersetTheme) => css` display: flex; @@ -100,6 +103,10 @@ function AllEntities() { query: [], }); + const canEditTag = useSelector((state: RootState) => + findPermission('can_write', 'Tag', state.user?.roles), + ); + const editableTitleProps = { title: tag?.name || '', placeholder: 'testing', @@ -211,14 +218,16 @@ function AllEntities() { } rightPanelAdditionalItems={ <> - + {canEditTag && ( + + )} } menuDropdownProps={{ @@ -232,6 +241,7 @@ function AllEntities() { search={tag?.name || ''} setShowTagModal={setShowTagModal} objects={objects} + canEditTag={canEditTag} /> diff --git a/superset/views/all_entities.py b/superset/views/all_entities.py index 5ca7e41e1d7..d78c6385898 100644 --- a/superset/views/all_entities.py +++ b/superset/views/all_entities.py @@ -22,6 +22,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.decorators import has_access from superset import is_feature_enabled +from superset.constants import RouteMethod from superset.superset_typing import FlaskResponse from superset.tags.models import Tag from superset.views.base import SupersetModelView @@ -33,7 +34,7 @@ class TaggedObjectsModelView(SupersetModelView): route_base = "/superset/all_entities" datamodel = SQLAInterface(Tag) class_permission_name = "Tags" - include_route_methods = {"list"} + include_route_methods = {RouteMethod.LIST} @has_access @expose("/")