Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add "created by" to dashboard CRUD view #11030

Merged
merged 3 commits into from
Sep 24, 2020
Merged

feat: add "created by" to dashboard CRUD view #11030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b03d010
add created by to dashboard list and tests
Sep 24, 2020
d6a9f0c
add created by to dashboard api test
Sep 24, 2020
33d983c
update dashboard error message
Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions superset-frontend/cypress-base/cypress/integration/dashboard_list/filter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import { DASHBOARD_LIST } from './dashboard_list.helper';

describe('dashboard filters', () => {
describe('dashboard filters card view', () => {
beforeEach(() => {
cy.login();
cy.server();
Expand All @@ -36,14 +36,63 @@ describe('dashboard filters', () => {
cy.get('.ant-card').should('not.exist');
});

it('should filter by created by correctly', () => {
// filter by created by
cy.get('.Select__control').eq(1).click();
cy.get('.Select__menu').contains('alpha user').click();
cy.get('.ant-card').should('not.exist');
cy.get('.Select__control').eq(1).click();
cy.get('.Select__menu').contains('gamma user').click();
cy.get('.ant-card').should('not.exist');
});

it('should filter by published correctly', () => {
// filter by published
cy.get('.Select__control').eq(1).click();
cy.get('.Select__control').eq(2).click();
cy.get('.Select__menu').contains('Published').click();
cy.get('.ant-card').should('have.length', 2);
cy.get('.ant-card').first().contains('USA Births Names').should('exist');
cy.get('.Select__control').eq(1).click();
cy.get('.Select__control').eq(1).type('unpub{enter}');
cy.get('.Select__control').eq(2).click();
cy.get('.Select__control').eq(2).type('unpub{enter}');
cy.get('.ant-card').should('have.length', 2);
});
});

describe('dashboard filters list view', () => {
beforeEach(() => {
cy.login();
cy.server();
cy.visit(DASHBOARD_LIST);
});

it('should filter by owners correctly', () => {
// filter by owners
cy.get('.Select__control').first().click();
cy.get('.Select__menu').contains('alpha user').click();
cy.get('.table-row').should('not.exist');
cy.get('.Select__control').first().click();
cy.get('.Select__menu').contains('gamma user').click();
cy.get('.table-row').should('not.exist');
});

it('should filter by created by correctly', () => {
// filter by created by
cy.get('.Select__control').eq(1).click();
cy.get('.Select__menu').contains('alpha user').click();
cy.get('.table-row').should('not.exist');
cy.get('.Select__control').eq(1).click();
cy.get('.Select__menu').contains('gamma user').click();
cy.get('.table-row').should('not.exist');
});

it('should filter by published correctly', () => {
// filter by published
cy.get('.Select__control').eq(2).click();
cy.get('.Select__menu').contains('Published').click();
cy.get('.table-row').should('have.length', 2);
cy.get('.table-row').first().contains('USA Births Names').should('exist');
cy.get('.Select__control').eq(2).click();
cy.get('.Select__control').eq(2).type('unpub{enter}');
cy.get('.table-row').should('have.length', 2);
});
});
15 changes: 10 additions & 5 deletions superset-frontend/cypress-base/cypress/integration/dashboard_list/list_view.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,22 @@ describe('dashboard list view', () => {
cy.get('table[role="table"]').should('be.visible');
// check dashboard list view header
cy.get('th[role="columnheader"]:nth-child(2)').contains('Title');
cy.get('th[role="columnheader"]:nth-child(3)').contains('Owners');
cy.get('th[role="columnheader"]:nth-child(4)').contains('Modified By');
cy.get('th[role="columnheader"]:nth-child(5)').contains('Published');
cy.get('th[role="columnheader"]:nth-child(6)').contains('Modified');
cy.get('th[role="columnheader"]:nth-child(7)').contains('Actions');
cy.get('th[role="columnheader"]:nth-child(3)').contains('Modified By');
cy.get('th[role="columnheader"]:nth-child(4)').contains('Published');
cy.get('th[role="columnheader"]:nth-child(5)').contains('Modified');
cy.get('th[role="columnheader"]:nth-child(6)').contains('Created By');
cy.get('th[role="columnheader"]:nth-child(7)').contains('Owners');
cy.get('th[role="columnheader"]:nth-child(8)').contains('Actions');
cy.get('.table-row').should('have.length', 4);
});

it('should sort correctly', () => {
cy.get('th[role="columnheader"]:nth-child(2)').click();
cy.get('.table-row td:nth-child(2):eq(0)').contains('Tabbed Dashboard');
cy.get('th[role="columnheader"]:nth-child(3)').click();
cy.get('.table-row td:nth-child(2):eq(0)').contains('Tabbed Dashboard');
cy.get('th[role="columnheader"]:nth-child(5)').click();
cy.get('.table-row td:nth-child(2):eq(0)').contains("World Bank's Data");
cy.get('th[role="columnheader"]:nth-child(6)').click();
cy.get('.table-row td:nth-child(2):eq(0)').contains("World Bank's Data");
});
Expand Down
71 changes: 52 additions & 19 deletions superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ interface Dashboard {
url: string;
thumbnail_url: string;
owners: Owner[];
created_by: object;
}

function DashboardList(props: DashboardListProps) {
Expand Down Expand Up @@ -195,24 +196,7 @@ function DashboardList(props: DashboardListProps) {
Header: t('Title'),
accessor: 'dashboard_title',
},
{
Cell: ({
row: {
original: { owners },
},
}: any) => (
<ExpandableList
items={owners.map(
({ first_name: firstName, last_name: lastName }: any) =>
`${firstName} ${lastName}`,
)}
display={2}
/>
),
Header: t('Owners'),
accessor: 'owners',
disableSortBy: true,
},

{
Cell: ({
row: {
Expand Down Expand Up @@ -252,6 +236,35 @@ function DashboardList(props: DashboardListProps) {
hidden: true,
disableSortBy: true,
},
{
Cell: ({
row: {
original: { created_by: createdBy },
},
}: any) =>
createdBy ? `${createdBy.first_name} ${createdBy.last_name}` : '',
Header: t('Created By'),
accessor: 'created_by',
disableSortBy: true,
},
{
Cell: ({
row: {
original: { owners },
},
}: any) => (
<ExpandableList
items={owners.map(
({ first_name: firstName, last_name: lastName }: any) =>
`${firstName} ${lastName}`,
)}
display={2}
/>
),
Header: t('Owners'),
accessor: 'owners',
disableSortBy: true,
},
{
Cell: ({ row: { original } }: any) => {
const handleDelete = () => handleDashboardDelete(original);
Expand Down Expand Up @@ -329,7 +342,27 @@ function DashboardList(props: DashboardListProps) {
createErrorHandler(errMsg =>
props.addDangerToast(
t(
'An error occurred while fetching chart owner values: %s',
'An error occurred while fetching dashboard owner values: %s',
errMsg,
),
),
),
),
paginate: true,
},
{
Header: t('Created By'),
id: 'created_by',
input: 'select',
operator: 'rel_o_m',
unfilteredLabel: 'All',
fetchSelects: createFetchRelated(
'dashboard',
'created_by',
createErrorHandler(errMsg =>
props.addDangerToast(
t(
'An error occurred while fetching dashboard created by values: %s',
errMsg,
),
),
Expand Down
15 changes: 10 additions & 5 deletions superset/dashboards/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"changed_by_url",
"changed_on_utc",
"changed_on_delta_humanized",
"created_by.first_name",
"created_by.id",
"created_by.last_name",
"dashboard_title",
"owners.id",
"owners.username",
Expand All @@ -121,10 +124,11 @@ class DashboardRestApi(BaseSupersetModelRestApi):
]
list_select_columns = list_columns + ["changed_on", "changed_by_fk"]
order_columns = [
"dashboard_title",
"changed_by.first_name",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thks for the sorting!

"changed_on_delta_humanized",
"created_by.first_name",
"dashboard_title",
"published",
"changed_by.first_name",
]

add_columns = [
Expand All @@ -138,7 +142,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
]
edit_columns = add_columns

search_columns = ("dashboard_title", "slug", "owners", "published")
search_columns = ("dashboard_title", "slug", "owners", "published", "created_by")
search_filters = {"dashboard_title": [DashboardTitleOrSlugFilter]}
base_order = ("changed_on", "desc")

Expand All @@ -152,9 +156,10 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"owners": ("first_name", "asc"),
}
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners)
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners),
"created_by": RelatedFieldFilter("first_name", FilterRelatedOwners),
}
allowed_rel_fields = {"owners"}
allowed_rel_fields = {"owners", "created_by"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new accepted column_name on the related endpoint will actually fetch the same data has owners but it's probably more coherent to add it anyway, and we may want to filter just one of them in the future (not very probable). Anyway, looks good


openapi_spec_tag = "Dashboards"
apispec_parameter_schemas = {
Expand Down
5 changes: 4 additions & 1 deletion tests/dashboards/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def insert_dashboard(
dashboard_title: str,
slug: Optional[str],
owners: List[int],
created_by=None,
slices: Optional[List[Slice]] = None,
position_json: str = "",
css: str = "",
Expand All @@ -71,6 +72,7 @@ def insert_dashboard(
json_metadata=json_metadata,
slices=slices,
published=published,
created_by=created_by,
)
db.session.add(dashboard)
db.session.commit()
Expand All @@ -81,7 +83,7 @@ def test_get_dashboard(self):
Dashboard API: Test get dashboard
"""
admin = self.get_user("admin")
dashboard = self.insert_dashboard("title", "slug1", [admin.id])
dashboard = self.insert_dashboard("title", "slug1", [admin.id], admin)
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.get_assert_metric(uri, "get")
Expand All @@ -91,6 +93,7 @@ def test_get_dashboard(self):
"changed_by_name": "",
"changed_by_url": "",
"charts": [],
"created_by": {"id": 1, "first_name": "admin", "last_name": "user",},
"id": dashboard.id,
"css": "",
"dashboard_title": "title",
Expand Down