lyndsiWilliams commented on code in PR #20876:
URL: https://github.com/apache/superset/pull/20876#discussion_r1043856735
##########
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:
##########
@@ -350,6 +362,25 @@ const PropertiesModal = ({
updateMetadata: false,
});
+ if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+ // update tags
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.DASHBOARD,
+ objectId: dashboardId,
+ includeTypes: false,
+ },
+ (currentTags: TagType[]) => updateTags(currentTags, tags),
+ error => {
+ handleErrorResponse(error);
+ },
+ );
+ } catch (error: any) {
Review Comment:
Can this `any` be defined?
##########
superset-frontend/src/components/Tags/TagsList.tsx:
##########
@@ -0,0 +1,112 @@
+/**
+ * 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, { useMemo, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import TagType from 'src/types/TagType';
+import Tag from './Tag';
+
+export type TagsListProps = {
+ tags: TagType[];
+ editable?: boolean;
+ /**
+ * OnDelete:
+ * Only applies when editable is true
+ * Callback for when a tag is deleted
+ */
+ onDelete?: ((index: number) => void) | undefined;
+ maxTags?: number | undefined;
+};
+
+const TagsDiv = styled.div`
+ max-width: 100%;
+ display: flex;
+ flex-direction: row;
+ flex-wrap: wrap;
+`;
+
+const TagsList = ({
+ tags,
+ editable = false,
+ onDelete,
+ maxTags,
+}: TagsListProps) => {
+ const [tempMaxTags, setTempMaxTags] = useState<number | undefined>(maxTags);
+
+ const handleDelete = (index: number) => {
+ onDelete?.(index);
+ };
+
+ const expand = () => setTempMaxTags(undefined);
+
+ const collapse = () => setTempMaxTags(maxTags);
+
+ const tagsIsLong: boolean | null = useMemo(
+ () => (tempMaxTags ? tags.length > tempMaxTags : null),
+ [tags.length, tempMaxTags],
+ );
+
+ const extraTags: number | null = useMemo(
+ () =>
+ typeof tempMaxTags === 'number' ? tags.length - tempMaxTags + 1 : null,
+ [tagsIsLong, tags.length, tempMaxTags],
+ );
+
+ return (
+ <TagsDiv className="tag-list">
+ {tagsIsLong === true && typeof tempMaxTags === 'number' ? (
Review Comment:
```suggestion
{tagsIsLong && typeof tempMaxTags === 'number' ? (
```
This should still check for truthiness without `=== true`
##########
superset-frontend/src/views/CRUD/allentities/AllEntities.tsx:
##########
@@ -0,0 +1,95 @@
+/**
+ * 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, { useEffect, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import Tag from 'src/types/TagType';
+import { StringParam, useQueryParam } from 'use-query-params';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SelectControl from 'src/explore/components/controls/SelectControl';
+import { fetchSuggestions } from 'src/tags';
+import AllEntitiesTable from './AllEntitiesTable';
+
+const AllEntitiesContainer = styled.div`
+ background-color: ${({ theme }) => theme.colors.grayscale.light4};
+ .select-control {
+ margin-left: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-right: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+ }
+ .select-control-label {
+ text-transform: uppercase;
+ font-size: ${({ theme }) => theme.gridUnit * 3}px;
+ color: ${({ theme }) => theme.colors.grayscale.base};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 1}px;
+ }
+`;
+
+const AllEntitiesNav = styled.div`
+ height: 50px;
+ background-color: ${({ theme }) => theme.colors.grayscale.light5};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
+ .navbar-brand {
+ margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+ font-weight: ${({ theme }) => theme.typography.weights.bold};
+ }
+`;
Review Comment:
The superset theme's gridUnit should be used for the height value, also you
can call theme once instead of multiple times by passing it in like this:
```suggestion
const AllEntitiesNav = styled.div`
${({ theme }) => `
height: ${theme.gridUnit * 12.5}px;
background-color: ${theme.colors.grayscale.light5};
margin-bottom: ${theme.gridUnit * 4}px;
.navbar-brand {
margin-left: ${theme.gridUnit * 2}px;
font-weight: ${theme.typography.weights.bold};
}
};
`;
```
##########
superset-frontend/src/views/CRUD/allentities/AllEntitiesTable.tsx:
##########
@@ -0,0 +1,121 @@
+/**
+ * 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, { useState, useEffect } from 'react';
+import moment from 'moment';
+import { t, styled } from '@superset-ui/core';
+import TableView, { EmptyWrapperType } from 'src/components/TableView';
+import { fetchObjects } from '../../../tags';
+import Loading from '../../../components/Loading';
+
+const AllEntitiesTableContainer = styled.div`
+ text-align: left;
+ border-radius: ${({ theme }) => theme.gridUnit * 1}px 0;
+ margin: 0 ${({ theme }) => theme.gridUnit * 4}px;
+ .table {
+ table-layout: fixed;
+ }
+ .td {
+ width: 33%;
+ }
+`;
+
+interface TaggedObject {
+ id: number;
+ type: string;
+ name: string;
+ url: string;
+ changed_on: moment.MomentInput;
+ created_by: number | undefined;
+ creator: string;
+}
+
+interface TaggedObjects {
+ dashboard: TaggedObject[];
+ chart: TaggedObject[];
+ query: TaggedObject[];
+}
+
+interface AllEntitiesTableProps {
+ search?: string;
+}
+
+export default function AllEntitiesTable({
+ search = '',
+}: AllEntitiesTableProps) {
+ const [objects, setObjects] = useState<TaggedObjects>({
+ dashboard: [],
+ chart: [],
+ query: [],
+ });
+
+ useEffect(() => {
+ fetchObjects(
+ { tags: search, types: null },
+ (data: TaggedObject[]) => {
+ const objects = { dashboard: [], chart: [], query: [] };
+ data.forEach(object => {
+ objects[object.type].push(object);
+ });
+ setObjects(objects);
+ },
+ (error: Response) => {
+ console.log(error.json());
+ },
+ );
+ }, [search]);
+
+ const renderTable = (type: any) => {
+ const data = objects[type].map((o: TaggedObject) => ({
+ [type]: <a href={o.url}>{o.name}</a>,
+ // eslint-disable-next-line react/no-danger
+ modified: moment.utc(o.changed_on).fromNow(),
Review Comment:
Is there a way to implement this without triggering the `react/no-danger`
lint rule?
##########
superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx:
##########
@@ -339,6 +347,25 @@ function DashboardList(props: DashboardListProps) {
disableSortBy: true,
size: 'xl',
},
+ {
+ Cell: ({
+ row: {
+ original: { tags = [] },
+ },
+ }: any) => (
Review Comment:
Resurfacing this, can this `any` be defined?
##########
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:
##########
@@ -350,6 +362,25 @@ const PropertiesModal = ({
updateMetadata: false,
});
+ if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+ // update tags
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.DASHBOARD,
+ objectId: dashboardId,
+ includeTypes: false,
+ },
+ (currentTags: TagType[]) => updateTags(currentTags, tags),
+ error => {
+ handleErrorResponse(error);
+ },
+ );
+ } catch (error: any) {
+ console.log(error);
Review Comment:
There is a logging utility in superset-frontend that can be used here
instead of using `console.log`:
`import { logging } from '@superset-ui/core';`
Then you can change this line to `logging.log(error);`
##########
superset-frontend/src/views/CRUD/allentities/AllEntities.tsx:
##########
@@ -0,0 +1,95 @@
+/**
+ * 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, { useEffect, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import Tag from 'src/types/TagType';
+import { StringParam, useQueryParam } from 'use-query-params';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SelectControl from 'src/explore/components/controls/SelectControl';
+import { fetchSuggestions } from 'src/tags';
+import AllEntitiesTable from './AllEntitiesTable';
+
+const AllEntitiesContainer = styled.div`
+ background-color: ${({ theme }) => theme.colors.grayscale.light4};
+ .select-control {
+ margin-left: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-right: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+ }
+ .select-control-label {
+ text-transform: uppercase;
+ font-size: ${({ theme }) => theme.gridUnit * 3}px;
+ color: ${({ theme }) => theme.colors.grayscale.base};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 1}px;
+ }
+`;
Review Comment:
You can call theme once instead of multiple times by passing it in this way:
```suggestion
const AllEntitiesContainer = styled.div`
${({ theme }) => `
background-color: ${theme.colors.grayscale.light4};
.select-control {
margin-left: ${theme.gridUnit * 4}px;
margin-right: ${theme.gridUnit * 4}px;
margin-bottom: ${theme.gridUnit * 2}px;
}
.select-control-label {
text-transform: uppercase;
font-size: ${theme.gridUnit * 3}px;
color: ${theme.colors.grayscale.base};
margin-bottom: ${theme.gridUnit * 1}px;
}
};
`;
```
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -148,6 +167,25 @@ function PropertiesModal({
}[]
).map(o => o.value);
}
+ if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+ // update tags
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.CHART,
+ objectId: slice.slice_id,
+ includeTypes: false,
+ },
+ (currentTags: TagType[]) => updateTags(currentTags, tags),
+ error => {
+ showError(error);
+ },
+ );
+ } catch (error: any) {
+ console.log(error);
Review Comment:
There is a logging utility in superset-frontend that can be used here
instead of using `console.log`:
`import { logging } from '@superset-ui/core';`
Then you can change this line to `logging.log(error);`
##########
superset-frontend/src/views/CRUD/allentities/AllEntities.tsx:
##########
@@ -0,0 +1,95 @@
+/**
+ * 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, { useEffect, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import Tag from 'src/types/TagType';
+import { StringParam, useQueryParam } from 'use-query-params';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SelectControl from 'src/explore/components/controls/SelectControl';
+import { fetchSuggestions } from 'src/tags';
+import AllEntitiesTable from './AllEntitiesTable';
+
+const AllEntitiesContainer = styled.div`
+ background-color: ${({ theme }) => theme.colors.grayscale.light4};
+ .select-control {
+ margin-left: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-right: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+ }
+ .select-control-label {
+ text-transform: uppercase;
+ font-size: ${({ theme }) => theme.gridUnit * 3}px;
+ color: ${({ theme }) => theme.colors.grayscale.base};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 1}px;
+ }
+`;
+
+const AllEntitiesNav = styled.div`
+ height: 50px;
+ background-color: ${({ theme }) => theme.colors.grayscale.light5};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
+ .navbar-brand {
+ margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+ font-weight: ${({ theme }) => theme.typography.weights.bold};
+ }
+`;
+
+function AllEntities() {
+ const [tagSuggestions, setTagSuggestions] = useState<string[]>();
+ const [tagsQuery, setTagsQuery] = useQueryParam('tags', StringParam);
+
+ useEffect(() => {
+ fetchSuggestions(
+ { includeTypes: false },
+ (suggestions: Tag[]) => {
+ const tagSuggestions = [...suggestions.map(tag => tag.name)];
+ setTagSuggestions(tagSuggestions);
+ },
+ (error: Response) => {
+ console.log(error.json());
+ },
+ );
+ }, [tagsQuery]);
+
+ const onTagSearchChange = (tags: Tag[]) => {
+ const tagSearch = tags.join(',');
+ setTagsQuery(tagSearch);
+ };
+
+ return (
+ <AllEntitiesContainer>
+ <AllEntitiesNav>
+ <span className="navbar-brand">All Entities</span>
+ </AllEntitiesNav>
+ <div className="select-control">
+ <div className="select-control-label">search by tags</div>
Review Comment:
These lines need translation: `import { t } from '@superset-ui/core';`
```suggestion
<span className="navbar-brand">{t('All Entities')}</span>
</AllEntitiesNav>
<div className="select-control">
<div className="select-control-label">{t('search by tags')}</div>
```
##########
superset-frontend/src/components/Tags/Tag.tsx:
##########
@@ -0,0 +1,84 @@
+/**
+ * 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 { styled } from '@superset-ui/core';
+import TagType from 'src/types/TagType';
+import AntdTag from 'antd/lib/tag';
+import React, { useMemo } from 'react';
+import { Tooltip } from 'src/components/Tooltip';
+
+const StyledTag = styled(AntdTag)`
+ margin-top: ${({ theme }) => theme.gridUnit}px;
+ margin-bottom: ${({ theme }) => theme.gridUnit}px;
+ font-size: ${({ theme }) => theme.typography.sizes.s}px;
+`;
Review Comment:
You can call theme once instead of multiple times by passing it in like this:
```suggestion
const StyledTag = styled(AntdTag)`
${({ theme }) => `
margin-top: ${theme.gridUnit}px;
margin-bottom: ${theme.gridUnit}px;
font-size: ${theme.typography.sizes.s}px;
};
`;
```
##########
superset-frontend/src/views/CRUD/allentities/AllEntitiesTable.tsx:
##########
@@ -0,0 +1,121 @@
+/**
+ * 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, { useState, useEffect } from 'react';
+import moment from 'moment';
+import { t, styled } from '@superset-ui/core';
+import TableView, { EmptyWrapperType } from 'src/components/TableView';
+import { fetchObjects } from '../../../tags';
+import Loading from '../../../components/Loading';
+
+const AllEntitiesTableContainer = styled.div`
+ text-align: left;
+ border-radius: ${({ theme }) => theme.gridUnit * 1}px 0;
+ margin: 0 ${({ theme }) => theme.gridUnit * 4}px;
+ .table {
+ table-layout: fixed;
+ }
+ .td {
+ width: 33%;
+ }
+`;
+
+interface TaggedObject {
+ id: number;
+ type: string;
+ name: string;
+ url: string;
+ changed_on: moment.MomentInput;
+ created_by: number | undefined;
+ creator: string;
+}
+
+interface TaggedObjects {
+ dashboard: TaggedObject[];
+ chart: TaggedObject[];
+ query: TaggedObject[];
+}
+
+interface AllEntitiesTableProps {
+ search?: string;
+}
+
+export default function AllEntitiesTable({
+ search = '',
+}: AllEntitiesTableProps) {
+ const [objects, setObjects] = useState<TaggedObjects>({
+ dashboard: [],
+ chart: [],
+ query: [],
+ });
+
+ useEffect(() => {
+ fetchObjects(
+ { tags: search, types: null },
+ (data: TaggedObject[]) => {
+ const objects = { dashboard: [], chart: [], query: [] };
+ data.forEach(object => {
+ objects[object.type].push(object);
+ });
+ setObjects(objects);
+ },
+ (error: Response) => {
+ console.log(error.json());
+ },
+ );
+ }, [search]);
+
+ const renderTable = (type: any) => {
Review Comment:
Can this `any` be defined?
##########
superset-frontend/src/types/TagType.ts:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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 interface TagType {
+ id?: string | number;
+ type?: string | number;
+ editable?: boolean;
+ onDelete?: any;
+ onClick?: any;
Review Comment:
Can these `any`s be defined?
##########
superset-frontend/src/views/CRUD/allentities/AllEntities.tsx:
##########
@@ -0,0 +1,95 @@
+/**
+ * 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, { useEffect, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import Tag from 'src/types/TagType';
+import { StringParam, useQueryParam } from 'use-query-params';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SelectControl from 'src/explore/components/controls/SelectControl';
+import { fetchSuggestions } from 'src/tags';
+import AllEntitiesTable from './AllEntitiesTable';
+
+const AllEntitiesContainer = styled.div`
+ background-color: ${({ theme }) => theme.colors.grayscale.light4};
+ .select-control {
+ margin-left: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-right: ${({ theme }) => theme.gridUnit * 4}px;
+ margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+ }
+ .select-control-label {
+ text-transform: uppercase;
+ font-size: ${({ theme }) => theme.gridUnit * 3}px;
+ color: ${({ theme }) => theme.colors.grayscale.base};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 1}px;
+ }
+`;
+
+const AllEntitiesNav = styled.div`
+ height: 50px;
+ background-color: ${({ theme }) => theme.colors.grayscale.light5};
+ margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
+ .navbar-brand {
+ margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+ font-weight: ${({ theme }) => theme.typography.weights.bold};
+ }
+`;
+
+function AllEntities() {
+ const [tagSuggestions, setTagSuggestions] = useState<string[]>();
+ const [tagsQuery, setTagsQuery] = useQueryParam('tags', StringParam);
+
+ useEffect(() => {
+ fetchSuggestions(
+ { includeTypes: false },
+ (suggestions: Tag[]) => {
+ const tagSuggestions = [...suggestions.map(tag => tag.name)];
+ setTagSuggestions(tagSuggestions);
+ },
+ (error: Response) => {
+ console.log(error.json());
Review Comment:
There is a logging utility in superset-frontend that can be used here
instead of using `console.log`:
`import { logging } from '@superset-ui/core';`
Then you can change this line to `logging.log(error.json());`
##########
superset-frontend/src/views/CRUD/allentities/AllEntitiesTable.tsx:
##########
@@ -0,0 +1,121 @@
+/**
+ * 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, { useState, useEffect } from 'react';
+import moment from 'moment';
+import { t, styled } from '@superset-ui/core';
+import TableView, { EmptyWrapperType } from 'src/components/TableView';
+import { fetchObjects } from '../../../tags';
+import Loading from '../../../components/Loading';
+
+const AllEntitiesTableContainer = styled.div`
+ text-align: left;
+ border-radius: ${({ theme }) => theme.gridUnit * 1}px 0;
+ margin: 0 ${({ theme }) => theme.gridUnit * 4}px;
+ .table {
+ table-layout: fixed;
+ }
+ .td {
+ width: 33%;
+ }
+`;
+
+interface TaggedObject {
+ id: number;
+ type: string;
+ name: string;
+ url: string;
+ changed_on: moment.MomentInput;
+ created_by: number | undefined;
+ creator: string;
+}
+
+interface TaggedObjects {
+ dashboard: TaggedObject[];
+ chart: TaggedObject[];
+ query: TaggedObject[];
+}
+
+interface AllEntitiesTableProps {
+ search?: string;
+}
+
+export default function AllEntitiesTable({
+ search = '',
+}: AllEntitiesTableProps) {
+ const [objects, setObjects] = useState<TaggedObjects>({
+ dashboard: [],
+ chart: [],
+ query: [],
+ });
+
+ useEffect(() => {
+ fetchObjects(
+ { tags: search, types: null },
+ (data: TaggedObject[]) => {
+ const objects = { dashboard: [], chart: [], query: [] };
+ data.forEach(object => {
+ objects[object.type].push(object);
+ });
+ setObjects(objects);
+ },
+ (error: Response) => {
+ console.log(error.json());
Review Comment:
There is a logging utility in superset-frontend that can be used here
instead of using `console.log`:
`import { logging } from '@superset-ui/core';`
Then you can change this line to `logging.log(error.json());`
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -183,6 +222,72 @@ function PropertiesModal({
setName(slice.slice_name || '');
}, [slice.slice_name]);
+ useEffect(() => {
+ if (!isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) return;
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.CHART,
+ objectId: slice.slice_id,
+ includeTypes: false,
+ },
+ (tags: TagType[]) => setTags(tags),
+ error => {
+ showError(error);
+ },
+ );
+ } catch (error: any) {
+ console.log(error);
Review Comment:
There is a logging utility in superset-frontend that can be used here
instead of using `console.log`:
`import { logging } from '@superset-ui/core';`
Then you can change this line to `logging.log(error);`
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
Review Comment:
This function seems a little scary to use, is there another way that this
can be done?
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
+
+ const canDelete = hasPerm('can_write');
+
+ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
+
+ function handleTagsDelete(
+ tags: Tag[],
+ callback: (text: string) => void,
+ error: (text: string) => void,
+ ) {
+ // TODO what permissions need to be checked here?
+ deleteTags(tags, callback, error);
+ refreshData();
+ }
+
+ const columns = useMemo(
+ () => [
+ {
+ Cell: ({
+ row: {
+ original: { name: tagName },
+ },
+ }: any) => (
+ <AntdTag>
+ <Link to={`/superset/all_entities/?tags=${tagName}`}>
+ {tagName}
+ </Link>
+ </AntdTag>
+ ),
+ Header: t('Name'),
+ accessor: 'name',
+ },
+ {
+ Cell: ({
+ row: {
+ original: { changed_on_delta_humanized: changedOn },
+ },
+ }: any) => <span className="no-wrap">{changedOn}</span>,
+ Header: t('Modified'),
+ accessor: 'changed_on_delta_humanized',
+ size: 'xl',
+ },
+ {
+ Cell: ({
+ row: {
+ original: { created_by: createdBy },
+ },
+ }: any) => (createdBy ? <FacePile users={[createdBy]} /> : ''),
+ Header: t('Created by'),
+ accessor: 'created_by',
+ disableSortBy: true,
+ size: 'xl',
+ },
+ {
+ Cell: ({ row: { original } }: any) => {
Review Comment:
Can this `any` be defined?
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
+
+ const canDelete = hasPerm('can_write');
+
+ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
+
+ function handleTagsDelete(
+ tags: Tag[],
+ callback: (text: string) => void,
+ error: (text: string) => void,
+ ) {
+ // TODO what permissions need to be checked here?
+ deleteTags(tags, callback, error);
+ refreshData();
+ }
+
+ const columns = useMemo(
+ () => [
+ {
+ Cell: ({
+ row: {
+ original: { name: tagName },
+ },
+ }: any) => (
+ <AntdTag>
+ <Link to={`/superset/all_entities/?tags=${tagName}`}>
+ {tagName}
+ </Link>
+ </AntdTag>
+ ),
+ Header: t('Name'),
+ accessor: 'name',
+ },
+ {
+ Cell: ({
+ row: {
+ original: { changed_on_delta_humanized: changedOn },
+ },
+ }: any) => <span className="no-wrap">{changedOn}</span>,
Review Comment:
Can this `any` be defined?
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
+
+ const canDelete = hasPerm('can_write');
+
+ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
+
+ function handleTagsDelete(
+ tags: Tag[],
+ callback: (text: string) => void,
+ error: (text: string) => void,
+ ) {
+ // TODO what permissions need to be checked here?
+ deleteTags(tags, callback, error);
+ refreshData();
+ }
+
+ const columns = useMemo(
+ () => [
+ {
+ Cell: ({
+ row: {
+ original: { name: tagName },
+ },
+ }: any) => (
Review Comment:
Can this `any` be defined?
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
+
+ const canDelete = hasPerm('can_write');
+
+ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
+
+ function handleTagsDelete(
+ tags: Tag[],
+ callback: (text: string) => void,
+ error: (text: string) => void,
+ ) {
+ // TODO what permissions need to be checked here?
Review Comment:
Will this TODO be addressed in this PR or a future one?
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -148,6 +170,25 @@ function PropertiesModal({
}[]
).map(o => o.value);
}
+ if (isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) {
+ // update tags
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.CHART,
+ objectId: slice.slice_id,
+ includeTypes: false,
+ },
+ (currentTags: TagType[]) => updateTags(currentTags, tags),
+ () => {
+ /* TODO: handle error */
+ },
+ );
+ } catch (error: any) {
Review Comment:
Resurfacing this, can this `any` be defined?
##########
superset-frontend/src/views/CRUD/tags/TagList.tsx:
##########
@@ -0,0 +1,332 @@
+/**
+ * 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 { t } from '@superset-ui/core';
+import React, { useMemo, useCallback } from 'react';
+import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import {
+ createFetchRelated,
+ createErrorHandler,
+ Actions,
+} from 'src/views/CRUD/utils';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
+import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
+import ListView, {
+ ListViewProps,
+ Filters,
+ FilterOperator,
+} from 'src/components/ListView';
+import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import Icons from 'src/components/Icons';
+import { Tooltip } from 'src/components/Tooltip';
+import FacePile from 'src/components/FacePile';
+import { Link } from 'react-router-dom';
+import { deleteTags } from 'src/tags';
+import { Tag as AntdTag } from 'antd';
+import { Tag } from '../types';
+import TagCard from './TagCard';
+
+const PAGE_SIZE = 25;
+
+interface TagListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function TagList(props: TagListProps) {
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
+
+ const {
+ state: {
+ loading,
+ resourceCount: tagCount,
+ resourceCollection: tags,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ toggleBulkSelect,
+ refreshData,
+ } = useListViewResource<Tag>('tag', t('tag'), addDangerToast);
+
+ // TODO: Fix usage of localStorage keying on the user id
+ const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
+
+ const canDelete = hasPerm('can_write');
+
+ const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
+
+ function handleTagsDelete(
+ tags: Tag[],
+ callback: (text: string) => void,
+ error: (text: string) => void,
+ ) {
+ // TODO what permissions need to be checked here?
+ deleteTags(tags, callback, error);
+ refreshData();
+ }
+
+ const columns = useMemo(
+ () => [
+ {
+ Cell: ({
+ row: {
+ original: { name: tagName },
+ },
+ }: any) => (
+ <AntdTag>
+ <Link to={`/superset/all_entities/?tags=${tagName}`}>
+ {tagName}
+ </Link>
+ </AntdTag>
+ ),
+ Header: t('Name'),
+ accessor: 'name',
+ },
+ {
+ Cell: ({
+ row: {
+ original: { changed_on_delta_humanized: changedOn },
+ },
+ }: any) => <span className="no-wrap">{changedOn}</span>,
+ Header: t('Modified'),
+ accessor: 'changed_on_delta_humanized',
+ size: 'xl',
+ },
+ {
+ Cell: ({
+ row: {
+ original: { created_by: createdBy },
+ },
+ }: any) => (createdBy ? <FacePile users={[createdBy]} /> : ''),
+ Header: t('Created by'),
+ accessor: 'created_by',
+ disableSortBy: true,
+ size: 'xl',
+ },
+ {
+ Cell: ({ row: { original } }: any) => {
+ const handleDelete = () =>
+ handleTagsDelete([original], addSuccessToast, addDangerToast);
+ return (
+ <Actions className="actions">
+ {canDelete && (
+ <ConfirmStatusChange
+ title={t('Please confirm')}
+ description={
+ <>
+ {t('Are you sure you want to delete')}{' '}
+ <b>{original.dashboard_title}</b>?
+ </>
+ }
+ onConfirm={handleDelete}
+ >
+ {confirmDelete => (
+ <Tooltip
+ id="delete-action-tooltip"
+ title={t('Delete')}
+ placement="bottom"
+ >
+ <span
+ role="button"
+ tabIndex={0}
+ className="action-button"
+ onClick={confirmDelete}
+ >
+ {/* fix icon name */}
Review Comment:
Will this comment be addressed in this PR or a future one?
##########
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx:
##########
@@ -521,6 +557,67 @@ const PropertiesModal = ({
}
}, [dashboardInfo, dashboardTitle, form]);
+ useEffect(() => {
+ if (!isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) return;
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.DASHBOARD,
+ objectId: dashboardId,
+ includeTypes: false,
+ },
+ (tags: TagType[]) => setTags(tags),
+ () => {
+ /* TODO: handle error */
+ },
+ );
+ } catch (error: any) {
Review Comment:
Resurfacing this, can this `any` be defined?
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -314,6 +421,26 @@ function PropertiesModal({
)}
</StyledHelpBlock>
</FormItem>
+ {isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM) && (
+ <h3 style={{ marginTop: '1em' }}>{t('Tags')}</h3>
Review Comment:
Resurfacing this, `css` is used for inline styles as per [the frontend code
style
guidelines](https://github.com/apache/superset/wiki/Frontend-Style-Guidelines).
##########
superset-frontend/src/views/CRUD/chart/ChartList.tsx:
##########
@@ -366,6 +370,27 @@ function ChartList(props: ChartListProps) {
disableSortBy: true,
size: 'xl',
},
+ {
+ Cell: ({
+ row: {
+ original: { tags = [] },
+ },
+ }: any) => (
Review Comment:
Resurfacing this, can this `any` be defined?
##########
superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx:
##########
@@ -362,6 +364,20 @@ function SavedQueryList({
accessor: 'changed_on_delta_humanized',
size: 'xl',
},
+ {
+ Cell: ({
+ row: {
+ original: { tags = [] },
+ },
+ }: any) => (
Review Comment:
Resurfacing this, can this `any` be defined?
##########
superset-frontend/src/explore/components/PropertiesModal/index.tsx:
##########
@@ -182,6 +224,71 @@ function PropertiesModal({
setName(slice.slice_name || '');
}, [slice.slice_name]);
+ useEffect(() => {
+ if (!isFeatureEnabled(FeatureFlag.TAGGING_SYSTEM)) return;
+ try {
+ fetchTags(
+ {
+ objectType: OBJECT_TYPES.CHART,
+ objectId: slice.slice_id,
+ includeTypes: false,
+ },
+ (tags: TagType[]) => setTags(tags),
+ () => {
+ /* TODO: handle error */
+ },
+ );
+ } catch (error: any) {
Review Comment:
Resurfacing this, can this `any` be defined?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
