You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ta...@apache.org on 2020/09/30 20:53:35 UTC
[incubator-superset] branch master updated: feat: add created by
chart CRUD view (#11067)
This is an automated email from the ASF dual-hosted git repository.
tai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 5b284e6 feat: add created by chart CRUD view (#11067)
5b284e6 is described below
commit 5b284e67eba2d2d66fa7d5a1fd5bbdab4ec4fa0c
Author: Lily Kuang <li...@preset.io>
AuthorDate: Wed Sep 30 13:53:04 2020 -0700
feat: add created by chart CRUD view (#11067)
---
.../cypress/integration/chart_list/filter.test.ts | 76 ++++++++++++++++++++--
.../integration/chart_list/list_view.test.ts | 5 +-
.../integration/dashboard_list/filter.test.ts | 1 +
.../src/views/CRUD/chart/ChartList.tsx | 33 +++++++++-
superset/charts/api.py | 17 +++--
tests/charts/api_tests.py | 25 ++++---
6 files changed, 132 insertions(+), 25 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts
index b4b57c3..07f2491 100644
--- a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts
@@ -18,7 +18,7 @@
*/
import { CHART_LIST } from './chart_list.helper';
-describe('chart filters', () => {
+describe('chart card view filters', () => {
beforeEach(() => {
cy.login();
cy.server();
@@ -36,26 +36,88 @@ describe('chart 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 viz type correctly', () => {
// filter by viz type
- cy.get('.Select__control').eq(1).click();
+ cy.get('.Select__control').eq(2).click();
cy.get('.Select__menu').contains('area').click({ timeout: 5000 });
cy.get('.ant-card').its('length').should('be.gt', 0);
cy.get('.ant-card').contains("World's Pop Growth").should('exist');
- cy.get('.Select__control').eq(1).click();
- cy.get('.Select__control').eq(1).type('world_map{enter}');
+ cy.get('.Select__control').eq(2).click();
+ cy.get('.Select__control').eq(2).type('world_map{enter}');
cy.get('.ant-card').should('have.length', 1);
cy.get('.ant-card').contains('% Rural').should('exist');
});
it('should filter by datasource correctly', () => {
// filter by datasource
- cy.get('.Select__control').eq(2).click();
+ cy.get('.Select__control').eq(3).click();
cy.get('.Select__menu').contains('unicode_test').click();
cy.get('.ant-card').should('have.length', 1);
cy.get('.ant-card').contains('Unicode Cloud').should('exist');
- cy.get('.Select__control').eq(2).click();
- cy.get('.Select__control').eq(2).type('energy_usage{enter}{enter}');
+ cy.get('.Select__control').eq(3).click();
+ cy.get('.Select__control').eq(3).type('energy_usage{enter}{enter}');
cy.get('.ant-card').its('length').should('be.gt', 0);
});
});
+
+describe('chart list view filters', () => {
+ beforeEach(() => {
+ cy.login();
+ cy.server();
+ cy.visit(CHART_LIST);
+ cy.get('[data-test="list-view"]').click();
+ });
+
+ 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 viz type correctly', () => {
+ // filter by viz type
+ cy.get('.Select__control').eq(2).click();
+ cy.get('.Select__menu').contains('area').click({ timeout: 5000 });
+ cy.get('.table-row').its('length').should('be.gt', 0);
+ cy.get('.table-row').contains("World's Pop Growth").should('exist');
+ cy.get('.Select__control').eq(2).click();
+ cy.get('.Select__control').eq(2).type('world_map{enter}');
+ cy.get('.table-row').should('have.length', 1);
+ cy.get('.table-row').contains('% Rural').should('exist');
+ });
+
+ it('should filter by datasource correctly', () => {
+ // filter by datasource
+ cy.get('.Select__control').eq(3).click();
+ cy.get('.Select__menu').contains('unicode_test').click();
+ cy.get('.table-row').should('have.length', 1);
+ cy.get('.table-row').contains('Unicode Cloud').should('exist');
+ cy.get('.Select__control').eq(3).click();
+ cy.get('.Select__control').eq(3).type('energy_usage{enter}{enter}');
+ cy.get('.table-row').its('length').should('be.gt', 0);
+ });
+});
diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/list_view.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/list_view.test.ts
index 3f68151..1aedf2a 100644
--- a/superset-frontend/cypress-base/cypress/integration/chart_list/list_view.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/chart_list/list_view.test.ts
@@ -26,7 +26,7 @@ describe('chart list view', () => {
cy.get('[data-test="list-view"]').click();
});
- it.skip('should load rows', () => {
+ it('should load rows', () => {
cy.get('.chart-list-view');
cy.get('table[role="table"]').should('be.visible');
@@ -38,7 +38,8 @@ describe('chart list view', () => {
cy.get('th[role="columnheader"]:nth-child(4)').contains('Dataset');
cy.get('th[role="columnheader"]:nth-child(5)').contains('Modified By');
cy.get('th[role="columnheader"]:nth-child(6)').contains('Last Modified');
- cy.get('th[role="columnheader"]:nth-child(7)').contains('Actions');
+ cy.get('th[role="columnheader"]:nth-child(7)').contains('Created By');
+ cy.get('th[role="columnheader"]:nth-child(8)').contains('Actions');
cy.get('.table-row').should('have.length', 25);
});
diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard_list/filter.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard_list/filter.test.ts
index 60debb6..110560f 100644
--- a/superset-frontend/cypress-base/cypress/integration/dashboard_list/filter.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/dashboard_list/filter.test.ts
@@ -63,6 +63,7 @@ describe('dashboard filters list view', () => {
cy.login();
cy.server();
cy.visit(DASHBOARD_LIST);
+ cy.get('[data-test="list-view"]').click();
});
it('should filter by owners correctly', () => {
diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx
index 2cbc07c..3c29c37 100644
--- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx
+++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx
@@ -258,6 +258,17 @@ function ChartList(props: ChartListProps) {
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 } }: any) => {
const handleDelete = () => handleChartDelete(original);
const openEditModal = () => openChartEditModal(original);
@@ -324,7 +335,27 @@ function ChartList(props: ChartListProps) {
createErrorHandler(errMsg =>
props.addDangerToast(
t(
- 'An error occurred while fetching chart dataset values: %s',
+ 'An error occurred while fetching chart owners values: %s',
+ errMsg,
+ ),
+ ),
+ ),
+ ),
+ paginate: true,
+ },
+ {
+ Header: t('Created By'),
+ id: 'created_by',
+ input: 'select',
+ operator: 'rel_o_m',
+ unfilteredLabel: 'All',
+ fetchSelects: createFetchRelated(
+ 'chart',
+ 'created_by',
+ createErrorHandler(errMsg =>
+ props.addDangerToast(
+ t(
+ 'An error occurred while fetching chart created by values: %s',
errMsg,
),
),
diff --git a/superset/charts/api.py b/superset/charts/api.py
index fc65275..4999654 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -107,22 +107,25 @@ class ChartRestApi(BaseSupersetModelRestApi):
"changed_by_url",
"changed_on_delta_humanized",
"changed_on_utc",
+ "created_by.first_name",
+ "created_by.id",
+ "created_by.last_name",
"datasource_id",
"datasource_name_text",
"datasource_type",
"datasource_url",
"description",
"id",
+ "owners.first_name",
+ "owners.id",
+ "owners.last_name",
+ "owners.username",
"params",
"slice_name",
"table.default_endpoint",
"table.table_name",
"thumbnail_url",
"url",
- "owners.id",
- "owners.username",
- "owners.first_name",
- "owners.last_name",
"viz_type",
]
list_select_columns = list_columns + ["changed_by_fk", "changed_on"]
@@ -135,6 +138,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
"viz_type",
]
search_columns = [
+ "created_by",
"datasource_id",
"datasource_name",
"datasource_type",
@@ -172,10 +176,11 @@ class ChartRestApi(BaseSupersetModelRestApi):
}
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"}
def __init__(self) -> None:
if is_feature_enabled("THUMBNAILS"):
diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py
index 7127180..1d353be 100644
--- a/tests/charts/api_tests.py
+++ b/tests/charts/api_tests.py
@@ -48,6 +48,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
slice_name: str,
owners: List[int],
datasource_id: int,
+ created_by=None,
datasource_type: str = "table",
description: Optional[str] = None,
viz_type: Optional[str] = None,
@@ -62,15 +63,16 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
datasource_type, datasource_id, db.session
)
slice = Slice(
- slice_name=slice_name,
+ cache_timeout=cache_timeout,
+ created_by=created_by,
datasource_id=datasource.id,
datasource_name=datasource.name,
datasource_type=datasource.type,
- owners=obj_owners,
description=description,
- viz_type=viz_type,
+ owners=obj_owners,
params=params,
- cache_timeout=cache_timeout,
+ slice_name=slice_name,
+ viz_type=viz_type,
)
db.session.add(slice)
db.session.commit()
@@ -93,12 +95,12 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
"""
Chart API: Test delete bulk
"""
- admin_id = self.get_user("admin").id
+ admin = self.get_user("admin")
chart_count = 4
chart_ids = list()
for chart_name_index in range(chart_count):
chart_ids.append(
- self.insert_chart(f"title{chart_name_index}", [admin_id], 1).id
+ self.insert_chart(f"title{chart_name_index}", [admin.id], 1, admin).id
)
self.login(username="admin")
argument = chart_ids
@@ -365,7 +367,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
admin = self.get_user("admin")
gamma = self.get_user("gamma")
- chart_id = self.insert_chart("title", [admin.id], 1).id
+ chart_id = self.insert_chart("title", [admin.id], 1, admin).id
birth_names_table_id = SupersetTestCase.get_table_by_name("birth_names").id
chart_data = {
"slice_name": "title1_changed",
@@ -384,6 +386,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
related_dashboard = db.session.query(Dashboard).get(1)
+ self.assertEqual(model.created_by, admin)
self.assertEqual(model.slice_name, "title1_changed")
self.assertEqual(model.description, "description1")
self.assertIn(admin, model.owners)
@@ -902,7 +905,9 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
self.assertEqual(rv.status_code, 400)
def test_chart_data_with_invalid_datasource(self):
- """Chart data API: Test chart data query with invalid schema"""
+ """
+ Chart data API: Test chart data query with invalid schema
+ """
self.login(username="admin")
table = self.get_table_by_name("birth_names")
payload = get_query_context(table.name, table.id, table.type)
@@ -911,7 +916,9 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
self.assertEqual(rv.status_code, 400)
def test_chart_data_with_invalid_enum_value(self):
- """Chart data API: Test chart data query with invalid enum value"""
+ """
+ Chart data API: Test chart data query with invalid enum value
+ """
self.login(username="admin")
table = self.get_table_by_name("birth_names")
payload = get_query_context(table.name, table.id, table.type)