From 7c34c247c0a7ae5cdcd411b42be2fabc82dff37b Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 8 Apr 2019 10:27:13 -0700 Subject: [PATCH 1/9] Fix for tagging backend --- superset/models/tags.py | 4 +- superset/views/tags.py | 172 ++++++++++++++++++++++------------------ 2 files changed, 96 insertions(+), 80 deletions(-) diff --git a/superset/models/tags.py b/superset/models/tags.py index 897c189282406..9c30f55e537ac 100644 --- a/superset/models/tags.py +++ b/superset/models/tags.py @@ -123,7 +123,7 @@ def _add_owners(cls, session, target): tagged_object = TaggedObject( tag_id=tag.id, object_id=target.id, - object_type=ObjectTypes.chart, + object_type=cls.object_type, ) session.add(tagged_object) @@ -141,7 +141,7 @@ def after_insert(cls, mapper, connection, target): tagged_object = TaggedObject( tag_id=tag.id, object_id=target.id, - object_type=ObjectTypes.query, + object_type=cls.object_type, ) session.add(tagged_object) diff --git a/superset/views/tags.py b/superset/views/tags.py index fc34490c097d7..65cbbc9666459 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -30,7 +30,7 @@ from superset import app, appbuilder, db, utils from superset.jinja_context import current_user_id, current_username -import superset.models.core +from superset.models.core import Dashboard, Slice from superset.models.sql_lab import SavedQuery from superset.models.tags import ObjectTypes, Tag, TaggedObject, TagTypes from .base import BaseSupersetView, json_success @@ -57,33 +57,6 @@ def process_template(content): return template.render(context) -def get_name(obj): - if obj.Dashboard: - return obj.Dashboard.dashboard_title - elif obj.Slice: - return obj.Slice.slice_name - elif obj.SavedQuery: - return obj.SavedQuery.label - - -def get_creator(obj): - if obj.Dashboard: - return obj.Dashboard.creator() - elif obj.Slice: - return obj.Slice.creator() - elif obj.SavedQuery: - return obj.SavedQuery.creator() - - -def get_attribute(obj, attr): - if obj.Dashboard: - return getattr(obj.Dashboard, attr) - elif obj.Slice: - return getattr(obj.Slice, attr) - elif obj.SavedQuery: - return getattr(obj.SavedQuery, attr) - - class TagView(BaseSupersetView): @has_access_api @@ -91,11 +64,13 @@ class TagView(BaseSupersetView): def suggestions(self): query = ( db.session.query(TaggedObject) - .group_by(TaggedObject.tag_id) + .join(Tag) + .with_entities(TaggedObject.tag_id, Tag.name) + .group_by(TaggedObject.tag_id, Tag.name) .order_by(func.count().desc()) .all() ) - tags = [{'id': obj.tag.id, 'name': obj.tag.name} for obj in query] + tags = [{'id': id, 'name': name} for id, name in query] return json_success(json.dumps(tags)) @has_access_api @@ -157,60 +132,101 @@ def delete(self, object_type, object_id): @has_access_api @expose('/tagged_objects/', methods=['GET', 'POST']) def tagged_objects(self): - query = db.session.query( - TaggedObject, - superset.models.core.Dashboard, - superset.models.core.Slice, - SavedQuery, - ).join(Tag) - - tags = request.args.get('tags') + tags = [ + process_template(tag) + for tag in request.args.get('tags', '').split(',') if tag + ] if not tags: return json_success(json.dumps([])) - tags = [process_template(tag) for tag in tags.split(',')] - query = query.filter(Tag.name.in_(tags)) - # filter types - types = request.args.get('types') - if types: - query = query.filter(TaggedObject.object_type.in_(types.split(','))) - - # get names - query = query.outerjoin( - superset.models.core.Dashboard, - and_( - TaggedObject.object_id == superset.models.core.Dashboard.id, - TaggedObject.object_type == ObjectTypes.dashboard, - ), - ).outerjoin( - superset.models.core.Slice, - and_( - TaggedObject.object_id == superset.models.core.Slice.id, - TaggedObject.object_type == ObjectTypes.chart, - ), - ).outerjoin( - SavedQuery, - and_( - TaggedObject.object_id == SavedQuery.id, - TaggedObject.object_type == ObjectTypes.query, - ), - ).group_by(TaggedObject.object_id, TaggedObject.object_type) - - objects = [ - { - 'id': get_attribute(obj, 'id'), - 'type': obj.TaggedObject.object_type.name, - 'name': get_name(obj), - 'url': get_attribute(obj, 'url'), - 'changed_on': get_attribute(obj, 'changed_on'), - 'created_by': get_attribute(obj, 'created_by_fk'), - 'creator': get_creator(obj), - } - for obj in query if get_attribute(obj, 'id') + types = [ + type_ + for type_ in request.args.get('types', '').split(',') + if type_ ] - return json_success(json.dumps(objects, default=utils.core.json_int_dttm_ser)) + results = [] + + # dashboards + if not types or 'dashboard' in types: + dashboards = ( + db.session.query(Dashboard) + .join( + TaggedObject, + and_( + TaggedObject.object_id == Dashboard.id, + TaggedObject.object_type == ObjectTypes.dashboard, + ), + ) + .join(Tag, TaggedObject.tag_id == Tag.id) + .filter(Tag.name.in_(tags)) + ) + results.extend( + { + 'id': obj.id, + 'type': ObjectTypes.dashboard.name, + 'name': obj.dashboard_title, + 'url': obj.url, + 'changed_on': obj.changed_on, + 'created_by': obj.created_by_fk, + 'creator': obj.creator(), + } for obj in dashboards + ) + + # charts + if not types or 'chart' in types: + charts = ( + db.session.query(Slice) + .join( + TaggedObject, + and_( + TaggedObject.object_id == Slice.id, + TaggedObject.object_type == ObjectTypes.chart, + ), + ) + .join(Tag, TaggedObject.tag_id == Tag.id) + .filter(Tag.name.in_(tags)) + ) + results.extend( + { + 'id': obj.id, + 'type': ObjectTypes.chart.name, + 'name': obj.slice_name, + 'url': obj.url, + 'changed_on': obj.changed_on, + 'created_by': obj.created_by_fk, + 'creator': obj.creator(), + } for obj in charts + ) + + # saved queries + if not types or 'query' in types: + saved_queries = ( + db.session.query(SavedQuery) + .join( + TaggedObject, + and_( + TaggedObject.object_id == SavedQuery.id, + TaggedObject.object_type == ObjectTypes.query, + ), + ) + .join(Tag, TaggedObject.tag_id == Tag.id) + .filter(Tag.name.in_(tags)) + ) + results.extend( + { + 'id': obj.id, + 'type': ObjectTypes.query.name, + 'name': obj.label, + 'url': obj.url, + 'changed_on': obj.changed_on, + 'created_by': obj.created_by_fk, + 'creator': obj.creator(), + } for obj in saved_queries + ) + + return json_success(json.dumps(results, default=utils.core.json_int_dttm_ser)) app.url_map.converters['object_type'] = ObjectTypeConverter From 32aea34b42e248c909593fb907937963ed590abc Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 8 Apr 2019 10:28:55 -0700 Subject: [PATCH 2/9] WIP: tag frontend, needs feature flag --- package-lock.json | 16 ++ superset/assets/package-lock.json | 5 + superset/assets/package.json | 4 +- superset/assets/src/components/ObjectTags.css | 213 ++++++++++++++++++ superset/assets/src/components/ObjectTags.jsx | 143 ++++++++++++ .../src/dashboard/components/Header.jsx | 27 +++ .../explore/components/ExploreChartHeader.jsx | 36 +++ superset/assets/src/modules/utils.js | 7 + superset/assets/src/tags.js | 132 +++++++++++ superset/assets/src/utils/tags.js | 23 ++ superset/assets/src/welcome/TagsTable.jsx | 105 +++++++++ superset/assets/src/welcome/Welcome.jsx | 106 +++++++-- 12 files changed, 796 insertions(+), 21 deletions(-) create mode 100644 package-lock.json create mode 100644 superset/assets/src/components/ObjectTags.css create mode 100644 superset/assets/src/components/ObjectTags.jsx create mode 100644 superset/assets/src/tags.js create mode 100644 superset/assets/src/utils/tags.js create mode 100644 superset/assets/src/welcome/TagsTable.jsx diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 0000000000000..edb287f6a929f --- /dev/null +++ b/package-lock.json @@ -0,0 +1,16 @@ +{ + "requires": true, + "lockfileVersion": 1, + "dependencies": { + "dompurify": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-1.0.3.tgz", + "integrity": "sha512-7dks9EA59zmQ9UBH7QJmGCHNxGJ7chzB5RJObT+h/TGBDtp4pTgunOeT0NidTA5MAq22muWrW3XfqtpvIsoSmw==" + }, + "react-tag-autocomplete": { + "version": "5.9.0", + "resolved": "https://registry.npmjs.org/react-tag-autocomplete/-/react-tag-autocomplete-5.9.0.tgz", + "integrity": "sha512-u8d/V+vjlI0riO0mVI+F0GjTx75dck6gCmCJww/2QaRB2GqeY+fohwNohdkifkLeTQR3K8EGdkRe8+m1QseSiw==" + } + } +} diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json index cf656671b9cb8..68ee44cf6ed7c 100644 --- a/superset/assets/package-lock.json +++ b/superset/assets/package-lock.json @@ -16778,6 +16778,11 @@ "refractor": "^2.4.1" } }, + "react-tag-autocomplete": { + "version": "5.9.0", + "resolved": "https://registry.npmjs.org/react-tag-autocomplete/-/react-tag-autocomplete-5.9.0.tgz", + "integrity": "sha512-u8d/V+vjlI0riO0mVI+F0GjTx75dck6gCmCJww/2QaRB2GqeY+fohwNohdkifkLeTQR3K8EGdkRe8+m1QseSiw==" + }, "react-transition-group": { "version": "2.5.3", "resolved": "https://registry.npmjs.org/react-transition-group/-/react-transition-group-2.5.3.tgz", diff --git a/superset/assets/package.json b/superset/assets/package.json index 283bee87d5137..c3c9ae206f4d6 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -128,6 +128,7 @@ "react-split": "^2.0.4", "react-sticky": "^6.0.2", "react-syntax-highlighter": "^7.0.4", + "react-tag-autocomplete": "^5.9.0", "react-transition-group": "^2.5.3", "react-virtualized": "9.19.1", "react-virtualized-select": "^3.1.3", @@ -139,7 +140,8 @@ "shortid": "^2.2.6", "underscore": "^1.8.3", "urijs": "^1.18.10", - "viewport-mercator-project": "^6.1.1" + "viewport-mercator-project": "^6.1.1", + "whatwg-fetch": "^3.0.0" }, "devDependencies": { "@babel/cli": "^7.2.3", diff --git a/superset/assets/src/components/ObjectTags.css b/superset/assets/src/components/ObjectTags.css new file mode 100644 index 0000000000000..443c760e22e56 --- /dev/null +++ b/superset/assets/src/components/ObjectTags.css @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + *
+ *
+ * + *
+ *
diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index 31c74ad1a70c3..09ce22dfb65e6 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -29,6 +29,13 @@ import FaveStar from '../../components/FaveStar'; import TooltipWrapper from '../../components/TooltipWrapper'; import Timer from '../../components/Timer'; import CachedLabel from '../../components/CachedLabel'; +import ObjectTags from '../../components/ObjectTags'; +import { + addTag, + deleteTag, + fetchSuggestions, + fetchTags, +} from '../../tags'; const CHART_STATUS_MAP = { failed: 'danger', @@ -50,6 +57,27 @@ const propTypes = { }; class ExploreChartHeader extends React.PureComponent { + constructor(props) { + super(props); + + this.fetchTags = fetchTags.bind(this, { + objectType: 'chart', + objectId: props.chart.id, + includeTypes: false, + }); + this.fetchSuggestions = fetchSuggestions.bind(this, { + includeTypes: false, + }); + this.deleteTag = deleteTag.bind(this, { + objectType: 'chart', + objectId: props.chart.id, + }); + this.addTag = addTag.bind(this, { + objectType: 'chart', + objectId: props.chart.id, + includeTypes: false, + }); + } postChartFormData() { this.props.actions.postChartFormData(this.props.form_data, true, this.props.timeout, this.props.chart.id); @@ -95,6 +123,7 @@ class ExploreChartHeader extends React.PureComponent { latestQueryFormData, queryResponse } = this.props.chart; const chartSucceeded = ['success', 'rendered'].indexOf(this.props.chart.chartStatus) > 0; + console.log('HERE'); return (
} +
{chartSucceeded && queryResponse && { + if (response.ok) { + return response.json(); + } + throw new Error(response.text()); + }) + .then(json => callback( + json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) + .catch(text => error(text)); +} + +export function fetchSuggestions(options, callback, error) { + const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; + window.fetch('/tagview/tags/suggestions/') + .then((response) => { + if (response.ok) { + return response.json(); + } + throw new Error(response.text()); + }) + .then(json => callback( + json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) + .catch(text => error(text)); +} + +export function deleteTag(options, tag, callback, error) { + if (options.objectType === undefined || options.objectId === undefined) { + throw new Error('Need to specify objectType and objectId'); + } + const objectType = options.objectType; + const objectId = options.objectId; + + const url = `/tagview/tags/${objectType}/${objectId}/`; + const CSRF_TOKEN = getCSRFToken(); + window.fetch(url, { + body: JSON.stringify([tag]), + headers: { + 'content-type': 'application/json', + 'X-CSRFToken': CSRF_TOKEN, + }, + credentials: 'same-origin', + method: 'DELETE', + }) + .then((response) => { + if (response.ok) { + callback(response.text()); + } else { + error(response.text()); + } + }); +} + +export function addTag(options, tag, callback, error) { + if (options.objectType === undefined || options.objectId === undefined) { + throw new Error('Need to specify objectType and objectId'); + } + const objectType = options.objectType; + const objectId = options.objectId; + const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; + + if (tag.indexOf(':') !== -1 && !includeTypes) { + return; + } + const url = `/tagview/tags/${objectType}/${objectId}/`; + const CSRF_TOKEN = getCSRFToken(); + window.fetch(url, { + body: JSON.stringify([tag]), + headers: { + 'content-type': 'application/json', + 'X-CSRFToken': CSRF_TOKEN, + }, + credentials: 'same-origin', + method: 'POST', + }) + .then((response) => { + if (response.ok) { + callback(response.text()); + } else { + error(response.text()); + } + }); +} + +export function fetchObjects(options, callback) { + const tags = options.tags !== undefined ? options.tags : ''; + const types = options.types; + + let url = `/tagview/tagged_objects/?tags=${tags}`; + if (types) { + url += `&types=${types}`; + } + const CSRF_TOKEN = getCSRFToken(); + window.fetch(url, { + headers: { + 'X-CSRFToken': CSRF_TOKEN, + }, + credentials: 'same-origin', + }) + .then(response => response.json()) + .then(json => callback(json)); +} diff --git a/superset/assets/src/utils/tags.js b/superset/assets/src/utils/tags.js new file mode 100644 index 0000000000000..ee4c11782fe23 --- /dev/null +++ b/superset/assets/src/utils/tags.js @@ -0,0 +1,23 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const STANDARD_TAGS = [ + ['owner:{{ current_user_id() }}', 'Owned by me'], + ['favorited_by:{{ current_user_id() }}', 'Favorited by me'], +]; diff --git a/superset/assets/src/welcome/TagsTable.jsx b/superset/assets/src/welcome/TagsTable.jsx new file mode 100644 index 0000000000000..b32af8c03b943 --- /dev/null +++ b/superset/assets/src/welcome/TagsTable.jsx @@ -0,0 +1,105 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import PropTypes from 'prop-types'; +import moment from 'moment'; +import { Table, unsafe } from 'reactable-arc'; +import 'whatwg-fetch'; +import { t } from '@superset-ui/translation'; + +import { fetchObjects } from '../tags'; +import Loading from '../components/Loading'; +import '../../stylesheets/reactable-pagination.css'; + +const propTypes = { + search: PropTypes.string, +}; + +const defaultProps = { + search: '', +}; + +export default class TagsTable extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + objects: false, + }; + this.fetchResults = this.fetchResults.bind(this); + } + componentDidMount() { + this.fetchResults(this.props.search); + } + componentWillReceiveProps(newProps) { + if (this.props.search !== newProps.search) { + this.fetchResults(newProps.search); + } + } + fetchResults(search) { + fetchObjects({ tags: search, types: null }, (data) => { + const objects = { dashboard: [], chart: [], query: [] }; + data.forEach((object) => { + objects[object.type].push(object); + }); + this.setState({ objects }); + }); + } + renderTable(type) { + const data = this.state.objects[type].map(o => ({ + [type]: {o.name}, + creator: unsafe(o.creator), + modified: unsafe(moment.utc(o.changed_on).fromNow()), + })); + return ( + + ); + } + render() { + if (this.state.objects) { + return ( +
+

{t('Dashboards')}

+ {this.renderTable('dashboard')} +
+

{t('Charts')}

+ {this.renderTable('chart')} +
+

{t('Queries')}

+ {this.renderTable('query')} +
+ ); + } + return ; + } +} + +TagsTable.propTypes = propTypes; +TagsTable.defaultProps = defaultProps; diff --git a/superset/assets/src/welcome/Welcome.jsx b/superset/assets/src/welcome/Welcome.jsx index db1a632fd5f36..09f54834667f0 100644 --- a/superset/assets/src/welcome/Welcome.jsx +++ b/superset/assets/src/welcome/Welcome.jsx @@ -20,9 +20,14 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Panel, Row, Col, Tabs, Tab, FormControl } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; +import URI from 'urijs'; import RecentActivity from '../profile/components/RecentActivity'; import Favorites from '../profile/components/Favorites'; import DashboardTable from './DashboardTable'; +import SelectControl from '../explore/components/controls/SelectControl'; +import TagsTable from './TagsTable'; +import { fetchSuggestions } from '../tags'; +import { STANDARD_TAGS } from '../utils/tags'; const propTypes = { user: PropTypes.object.isRequired, @@ -31,38 +36,89 @@ const propTypes = { export default class Welcome extends React.PureComponent { constructor(props) { super(props); + + const parsedUrl = new URI(window.location); + const key = parsedUrl.fragment() || 'tags'; + const searchParams = parsedUrl.search(true); + const dashboardSearch = searchParams.search || ''; + const tagSearch = searchParams.tags || 'owner:{{ current_user_id() }}'; this.state = { - search: '', + key, + dashboardSearch, + tagSearch, + tagSuggestions: STANDARD_TAGS, }; - this.onSearchChange = this.onSearchChange.bind(this); + + this.handleSelect = this.handleSelect.bind(this); + this.onDashboardSearchChange = this.onDashboardSearchChange.bind(this); + this.onTagSearchChange = this.onTagSearchChange.bind(this); + } + componentDidMount() { + fetchSuggestions({ includeTypes: false }, (suggestions) => { + const tagSuggestions = [ + ...STANDARD_TAGS, + ...suggestions.map(tag => tag.name), + ]; + this.setState({ tagSuggestions }); + }); + } + onDashboardSearchChange(event) { + const dashboardSearch = event.target.value; + this.setState({ dashboardSearch }, () => this.updateURL('search', dashboardSearch)); + } + onTagSearchChange(tags) { + const tagSearch = tags.join(','); + this.setState({ tagSearch }, () => this.updateURL('tags', tagSearch)); } - onSearchChange(event) { - this.setState({ search: event.target.value }); + updateURL(key, value) { + const parsedUrl = new URI(window.location); + parsedUrl.search(data => ({ ...data, [key]: value })); + window.history.pushState({ value }, value, parsedUrl.href()); + } + handleSelect(key) { + // store selected tab in URL + window.history.pushState({ tab: key }, key, `#${key}`); + + this.setState({ key }); } render() { return (
- - + + -

{t('Dashboards')}

- -

{t('Tags')}

+ + +
+
- + - + + + +

{t('Favorites')}

+ +
+ + + +

{t('Recently Viewed')}

@@ -71,13 +127,23 @@ export default class Welcome extends React.PureComponent { - + -

{t('Favorites')}

+

{t('Dashboards')}

+ + +
- + From 69d7563e9aed1d2b4360ee4a33d50fbe5ea3c507 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 8 Apr 2019 11:17:44 -0700 Subject: [PATCH 3/9] Put tags behind feature flag --- .../src/dashboard/components/Header.jsx | 17 ++++--- .../explore/components/ExploreChartHeader.jsx | 18 +++---- superset/assets/src/featureFlags.ts | 1 + superset/assets/src/welcome/App.jsx | 2 + superset/assets/src/welcome/Welcome.jsx | 48 +++++++++++-------- 5 files changed, 50 insertions(+), 36 deletions(-) diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index 353f3a1daac0e..2e231be309937 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -22,6 +22,7 @@ import PropTypes from 'prop-types'; import { CategoricalColorNamespace } from '@superset-ui/color'; import { t } from '@superset-ui/translation'; +import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import HeaderActionsDropdown from './HeaderActionsDropdown'; import EditableTitle from '../../components/EditableTitle'; import Button from '../../components/Button'; @@ -319,13 +320,15 @@ class Header extends React.PureComponent { isStarred={this.props.isStarred} /> - + {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && + + }
diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index 09ce22dfb65e6..32cc9eec18ced 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -20,6 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { t } from '@superset-ui/translation'; +import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import { chartPropShape } from '../../dashboard/util/propShapes'; import ExploreActionButtons from './ExploreActionButtons'; import RowCountLabel from './RowCountLabel'; @@ -123,7 +124,6 @@ class ExploreChartHeader extends React.PureComponent { latestQueryFormData, queryResponse } = this.props.chart; const chartSucceeded = ['success', 'rendered'].indexOf(this.props.chart.chartStatus) > 0; - console.log('HERE'); return (
} - + {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && + + }
{chartSucceeded && queryResponse && - - - -

{t('Tags')}

- - - - - - -
- - - + {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && + + + +

{t('Tags')}

+ + + + + + +
+ + + + } From 1b827ddb20ee40aeb09f3d2ab1ee7faf89df62dc Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 30 Apr 2019 14:51:58 -0700 Subject: [PATCH 4/9] Remove whatwg-fetch --- superset/assets/package.json | 3 +- superset/assets/src/modules/utils.js | 7 --- superset/assets/src/tags.js | 86 +++++++--------------------- superset/views/tags.py | 5 +- 4 files changed, 26 insertions(+), 75 deletions(-) diff --git a/superset/assets/package.json b/superset/assets/package.json index 297830a12db33..c16ab0fcba3b3 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -139,8 +139,7 @@ "shortid": "^2.2.6", "underscore": "^1.8.3", "urijs": "^1.18.10", - "viewport-mercator-project": "^6.1.1", - "whatwg-fetch": "^3.0.0" + "viewport-mercator-project": "^6.1.1" }, "devDependencies": { "@babel/cli": "^7.2.3", diff --git a/superset/assets/src/modules/utils.js b/superset/assets/src/modules/utils.js index 73ba52f7ae27e..1e5f311c3719f 100644 --- a/superset/assets/src/modules/utils.js +++ b/superset/assets/src/modules/utils.js @@ -96,10 +96,3 @@ export function roundDecimal(number, precision) { } return roundedNumber; } - -export function getCSRFToken() { - if (document && document.getElementById('csrf_token')) { - return document.getElementById('csrf_token').value; - } - return ''; -} diff --git a/superset/assets/src/tags.js b/superset/assets/src/tags.js index 6d2f4a4adda06..db167fc5c5183 100644 --- a/superset/assets/src/tags.js +++ b/superset/assets/src/tags.js @@ -16,8 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import 'whatwg-fetch'; -import { getCSRFToken } from './modules/utils'; +import { SupersetClient } from '@superset-ui/connection'; export function fetchTags(options, callback, error) { if (options.objectType === undefined || options.objectId === undefined) { @@ -27,31 +26,18 @@ export function fetchTags(options, callback, error) { const objectId = options.objectId; const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; - const url = `/tagview/tags/${objectType}/${objectId}/`; - window.fetch(url) - .then((response) => { - if (response.ok) { - return response.json(); - } - throw new Error(response.text()); - }) - .then(json => callback( + SupersetClient.get({ endpoint: `/tagview/tags/${objectType}/${objectId}/` }) + .then(({ json }) => callback( json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) - .catch(text => error(text)); + .catch(response => error(response)); } export function fetchSuggestions(options, callback, error) { const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; - window.fetch('/tagview/tags/suggestions/') - .then((response) => { - if (response.ok) { - return response.json(); - } - throw new Error(response.text()); - }) - .then(json => callback( + SupersetClient.get({ endpoint: '/tagview/tags/suggestions/' }) + .then(({ json }) => callback( json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) - .catch(text => error(text)); + .catch(response => error(response)); } export function deleteTag(options, tag, callback, error) { @@ -61,24 +47,12 @@ export function deleteTag(options, tag, callback, error) { const objectType = options.objectType; const objectId = options.objectId; - const url = `/tagview/tags/${objectType}/${objectId}/`; - const CSRF_TOKEN = getCSRFToken(); - window.fetch(url, { - body: JSON.stringify([tag]), - headers: { - 'content-type': 'application/json', - 'X-CSRFToken': CSRF_TOKEN, - }, - credentials: 'same-origin', - method: 'DELETE', + SupersetClient.post({ + endpoint: `/tagview/tags/${objectType}/${objectId}/`, + postPayload: [tag], }) - .then((response) => { - if (response.ok) { - callback(response.text()); - } else { - error(response.text()); - } - }); + .then(() => callback()) + .catch(response => error(response)); } export function addTag(options, tag, callback, error) { @@ -92,27 +66,15 @@ export function addTag(options, tag, callback, error) { if (tag.indexOf(':') !== -1 && !includeTypes) { return; } - const url = `/tagview/tags/${objectType}/${objectId}/`; - const CSRF_TOKEN = getCSRFToken(); - window.fetch(url, { - body: JSON.stringify([tag]), - headers: { - 'content-type': 'application/json', - 'X-CSRFToken': CSRF_TOKEN, - }, - credentials: 'same-origin', - method: 'POST', + SupersetClient.post({ + endpoint: `/tagview/tags/${objectType}/${objectId}/`, + postPayload: [tag], }) - .then((response) => { - if (response.ok) { - callback(response.text()); - } else { - error(response.text()); - } - }); + .then(({ json }) => callback(json)) + .catch(response => error(response)); } -export function fetchObjects(options, callback) { +export function fetchObjects(options, callback, error) { const tags = options.tags !== undefined ? options.tags : ''; const types = options.types; @@ -120,13 +82,7 @@ export function fetchObjects(options, callback) { if (types) { url += `&types=${types}`; } - const CSRF_TOKEN = getCSRFToken(); - window.fetch(url, { - headers: { - 'X-CSRFToken': CSRF_TOKEN, - }, - credentials: 'same-origin', - }) - .then(response => response.json()) - .then(json => callback(json)); + SupersetClient.get({ endpoint: url }) + .then(({ json }) => callback(json)) + .catch(response => error(response)); } diff --git a/superset/views/tags.py b/superset/views/tags.py index cffec874836b1..3acf35352147c 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -119,7 +119,10 @@ def post(self, object_type, object_id): return Response(status=201) # 201 CREATED @has_access_api - @expose('/tags///', methods=['DELETE']) + @expose( + '/tags///', + methods=['POST', 'DELETE'], + ) def delete(self, object_type, object_id): """Remove tags from an object.""" tag_names = request.get_json(force=True) From 9871cf4b15275b9c91b81a58853159725911cb0a Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 1 May 2019 08:36:07 -0700 Subject: [PATCH 5/9] Fix tests --- superset/assets/src/dashboard/components/Header.jsx | 4 ++-- superset/assets/src/welcome/TagsTable.jsx | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index ce938ad112be4..9fdd9ca27b1f5 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -322,7 +322,7 @@ class Header extends React.PureComponent { isStarred={this.props.isStarred} /> - {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && + {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && ( - } + )}
diff --git a/superset/assets/src/welcome/TagsTable.jsx b/superset/assets/src/welcome/TagsTable.jsx index b32af8c03b943..051ac1c9857c2 100644 --- a/superset/assets/src/welcome/TagsTable.jsx +++ b/superset/assets/src/welcome/TagsTable.jsx @@ -20,7 +20,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import moment from 'moment'; import { Table, unsafe } from 'reactable-arc'; -import 'whatwg-fetch'; import { t } from '@superset-ui/translation'; import { fetchObjects } from '../tags'; From d8900c0617c7924dbc3180715a8e15ac6d7eb951 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 1 May 2019 09:14:21 -0700 Subject: [PATCH 6/9] Small fixes --- .../src/dashboard/components/Header.jsx | 14 ++++-- .../explore/components/ExploreChartHeader.jsx | 15 +++--- superset/assets/src/tags.js | 49 ++++++++----------- superset/views/tags.py | 5 +- 4 files changed, 40 insertions(+), 43 deletions(-) diff --git a/superset/assets/src/dashboard/components/Header.jsx b/superset/assets/src/dashboard/components/Header.jsx index 9fdd9ca27b1f5..e29f705ff223e 100644 --- a/superset/assets/src/dashboard/components/Header.jsx +++ b/superset/assets/src/dashboard/components/Header.jsx @@ -29,7 +29,13 @@ import Button from '../../components/Button'; import FaveStar from '../../components/FaveStar'; import ObjectTags from '../../components/ObjectTags'; import UndoRedoKeylisteners from './UndoRedoKeylisteners'; -import { addTag, deleteTag, fetchSuggestions, fetchTags } from '../../tags'; +import { + addTag, + deleteTag, + fetchSuggestions, + fetchTags, + OBJECT_TYPES, +} from '../../tags'; import { chartPropShape } from '../util/propShapes'; import { @@ -118,7 +124,7 @@ class Header extends React.PureComponent { this.overwriteDashboard = this.overwriteDashboard.bind(this); this.fetchTags = fetchTags.bind(this, { - objectType: 'dashboard', + objectType: OBJECT_TYPES.DASHBOARD, objectId: props.dashboardInfo.id, includeTypes: false, }); @@ -126,11 +132,11 @@ class Header extends React.PureComponent { includeTypes: false, }); this.deleteTag = deleteTag.bind(this, { - objectType: 'dashboard', + objectType: OBJECT_TYPES.DASHBOARD, objectId: props.dashboardInfo.id, }); this.addTag = addTag.bind(this, { - objectType: 'dashboard', + objectType: OBJECT_TYPES.DASHBOARD, objectId: props.dashboardInfo.id, includeTypes: false, }); diff --git a/superset/assets/src/explore/components/ExploreChartHeader.jsx b/superset/assets/src/explore/components/ExploreChartHeader.jsx index ba5126fc12fd6..7590ca1c0b29d 100644 --- a/superset/assets/src/explore/components/ExploreChartHeader.jsx +++ b/superset/assets/src/explore/components/ExploreChartHeader.jsx @@ -32,10 +32,11 @@ import Timer from '../../components/Timer'; import CachedLabel from '../../components/CachedLabel'; import ObjectTags from '../../components/ObjectTags'; import { - addTag, - deleteTag, - fetchSuggestions, - fetchTags, + addTag, + deleteTag, + fetchSuggestions, + fetchTags, + OBJECT_TYPES, } from '../../tags'; const CHART_STATUS_MAP = { @@ -62,7 +63,7 @@ class ExploreChartHeader extends React.PureComponent { super(props); this.fetchTags = fetchTags.bind(this, { - objectType: 'chart', + objectType: OBJECT_TYPES.CHART, objectId: props.chart.id, includeTypes: false, }); @@ -70,11 +71,11 @@ class ExploreChartHeader extends React.PureComponent { includeTypes: false, }); this.deleteTag = deleteTag.bind(this, { - objectType: 'chart', + objectType: OBJECT_TYPES.CHART, objectId: props.chart.id, }); this.addTag = addTag.bind(this, { - objectType: 'chart', + objectType: OBJECT_TYPES.CHART, objectId: props.chart.id, includeTypes: false, }); diff --git a/superset/assets/src/tags.js b/superset/assets/src/tags.js index db167fc5c5183..5e76edb818c3d 100644 --- a/superset/assets/src/tags.js +++ b/superset/assets/src/tags.js @@ -18,66 +18,59 @@ */ import { SupersetClient } from '@superset-ui/connection'; -export function fetchTags(options, callback, error) { - if (options.objectType === undefined || options.objectId === undefined) { +export const OBJECT_TYPES = Object.freeze({ + DASHBOARD: 'dashboard', + CHART: 'chart', + QUERY: 'query', +}); + +export function fetchTags({ objectType, objectId, includeTypes = false }, callback, error) { + if (objectType === undefined || objectId === undefined) { throw new Error('Need to specify objectType and objectId'); } - const objectType = options.objectType; - const objectId = options.objectId; - const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; - SupersetClient.get({ endpoint: `/tagview/tags/${objectType}/${objectId}/` }) .then(({ json }) => callback( json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) .catch(response => error(response)); } -export function fetchSuggestions(options, callback, error) { - const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; +export function fetchSuggestions({ includeTypes = false }, callback, error) { SupersetClient.get({ endpoint: '/tagview/tags/suggestions/' }) .then(({ json }) => callback( json.filter(tag => tag.name.indexOf(':') === -1 || includeTypes))) .catch(response => error(response)); } -export function deleteTag(options, tag, callback, error) { - if (options.objectType === undefined || options.objectId === undefined) { +export function deleteTag({ objectType, objectId }, tag, callback, error) { + if (objectType === undefined || objectId === undefined) { throw new Error('Need to specify objectType and objectId'); } - const objectType = options.objectType; - const objectId = options.objectId; - - SupersetClient.post({ + SupersetClient.delete({ endpoint: `/tagview/tags/${objectType}/${objectId}/`, - postPayload: [tag], + body: JSON.stringify([tag]), + parseMethod: 'text', }) - .then(() => callback()) + .then(({ text }) => callback(text)) .catch(response => error(response)); } -export function addTag(options, tag, callback, error) { - if (options.objectType === undefined || options.objectId === undefined) { +export function addTag({ objectType, objectId, includeTypes = false }, tag, callback, error) { + if (objectType === undefined || objectId === undefined) { throw new Error('Need to specify objectType and objectId'); } - const objectType = options.objectType; - const objectId = options.objectId; - const includeTypes = options.includeTypes !== undefined ? options.includeTypes : false; - if (tag.indexOf(':') !== -1 && !includeTypes) { return; } SupersetClient.post({ endpoint: `/tagview/tags/${objectType}/${objectId}/`, - postPayload: [tag], + body: JSON.stringify([tag]), + parseMethod: 'text', }) - .then(({ json }) => callback(json)) + .then(({ text }) => callback(text)) .catch(response => error(response)); } -export function fetchObjects(options, callback, error) { - const tags = options.tags !== undefined ? options.tags : ''; - const types = options.types; - +export function fetchObjects({ tags = '', types }, callback, error) { let url = `/tagview/tagged_objects/?tags=${tags}`; if (types) { url += `&types=${types}`; diff --git a/superset/views/tags.py b/superset/views/tags.py index 3acf35352147c..cffec874836b1 100644 --- a/superset/views/tags.py +++ b/superset/views/tags.py @@ -119,10 +119,7 @@ def post(self, object_type, object_id): return Response(status=201) # 201 CREATED @has_access_api - @expose( - '/tags///', - methods=['POST', 'DELETE'], - ) + @expose('/tags///', methods=['DELETE']) def delete(self, object_type, object_id): """Remove tags from an object.""" tag_names = request.get_json(force=True) From ae40ee6f81d72a2831e0cb5adc62c0036ab45811 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 1 May 2019 10:33:20 -0700 Subject: [PATCH 7/9] Fix unit test --- superset/assets/src/featureFlags.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/featureFlags.ts b/superset/assets/src/featureFlags.ts index 8a1060ab45f61..f55cd67656330 100644 --- a/superset/assets/src/featureFlags.ts +++ b/superset/assets/src/featureFlags.ts @@ -40,5 +40,5 @@ export function initFeatureFlags(featureFlags: FeatureFlagMap) { } export function isFeatureEnabled(feature: FeatureFlag) { - return !!window.featureFlags[feature]; + return window && window.featureFlags && !!window.featureFlags[feature]; } From 6e1333e6a3b958f9b45d76308f01c6270e2098ce Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 1 May 2019 11:06:49 -0700 Subject: [PATCH 8/9] Use less for stylesheet --- .../assets/src/components/{ObjectTags.css => ObjectTags.less} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename superset/assets/src/components/{ObjectTags.css => ObjectTags.less} (98%) diff --git a/superset/assets/src/components/ObjectTags.css b/superset/assets/src/components/ObjectTags.less similarity index 98% rename from superset/assets/src/components/ObjectTags.css rename to superset/assets/src/components/ObjectTags.less index 443c760e22e56..1225830b979ab 100644 --- a/superset/assets/src/components/ObjectTags.css +++ b/superset/assets/src/components/ObjectTags.less @@ -45,7 +45,7 @@ display: inline-block; padding: 1px 0 0 1px; margin: 0 10px; - border: 1px solid #F5F5F5; + border: 1px solid @gray-bg; border-radius: 1px; /* shared font styles */ @@ -76,7 +76,7 @@ box-sizing: border-box; margin: 0; padding: 6px 8px; - border: 1px solid #F5F5F5; + border: 1px solid @gray-bg; border-radius: 2px; background: #F1F1F1; From 7865afbdf93c3b35cf2d01bc8af21606eba73dcd Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 22 May 2019 14:37:29 -0700 Subject: [PATCH 9/9] Fix stylesheet --- superset/assets/src/components/ObjectTags.jsx | 2 +- superset/assets/src/components/ObjectTags.less | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/assets/src/components/ObjectTags.jsx b/superset/assets/src/components/ObjectTags.jsx index 5483715544f93..19ccdf5613f5a 100644 --- a/superset/assets/src/components/ObjectTags.jsx +++ b/superset/assets/src/components/ObjectTags.jsx @@ -23,7 +23,7 @@ import { Glyphicon, Label } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; import TooltipWrapper from './TooltipWrapper'; -import './ObjectTags.css'; +import './ObjectTags.less'; const propTypes = { fetchTags: PropTypes.func, diff --git a/superset/assets/src/components/ObjectTags.less b/superset/assets/src/components/ObjectTags.less index 1225830b979ab..78b71953a60a3 100644 --- a/superset/assets/src/components/ObjectTags.less +++ b/superset/assets/src/components/ObjectTags.less @@ -45,7 +45,7 @@ display: inline-block; padding: 1px 0 0 1px; margin: 0 10px; - border: 1px solid @gray-bg; + border: 1px solid #f5f5f5; border-radius: 1px; /* shared font styles */ @@ -76,7 +76,7 @@ box-sizing: border-box; margin: 0; padding: 6px 8px; - border: 1px solid @gray-bg; + border: 1px solid #f5f5f5; border-radius: 2px; background: #F1F1F1;