You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/08/04 00:11:36 UTC

[GitHub] [incubator-superset] riahk opened a new pull request #10510: feat: add favorite star to dashboard and chart lists

riahk opened a new pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510


   ### SUMMARY
   - [x] Use svg for `FaveStar` icon and convert to TypeScript
   - [x] Fix button alignment on individual chart/dashboard header view
   - [x] Add favorite star to cells in dashboard and chart lists
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Before:
   <img width="538" alt="Screen Shot 2020-08-03 at 5 04 33 PM" src="https://user-images.githubusercontent.com/8216382/89238769-11538980-d5ac-11ea-8b45-e3fb73adbaa3.png">
   <img width="823" alt="Screen Shot 2020-08-03 at 5 05 01 PM" src="https://user-images.githubusercontent.com/8216382/89238776-17496a80-d5ac-11ea-821a-56de63de15af.png">
   <img width="1231" alt="Screen Shot 2020-08-03 at 5 02 40 PM" src="https://user-images.githubusercontent.com/8216382/89238788-20d2d280-d5ac-11ea-847e-7f557f7991e6.png">
   <img width="1229" alt="Screen Shot 2020-08-03 at 5 04 51 PM" src="https://user-images.githubusercontent.com/8216382/89238793-24665980-d5ac-11ea-82e2-938784e54574.png">
   
   After:
   <img width="969" alt="Screen Shot 2020-08-03 at 5 08 39 PM" src="https://user-images.githubusercontent.com/8216382/89238819-3b0cb080-d5ac-11ea-842b-1a032cc12016.png">
   <img width="814" alt="Screen Shot 2020-08-03 at 5 00 08 PM" src="https://user-images.githubusercontent.com/8216382/89238828-41029180-d5ac-11ea-846b-f5d94726ec16.png">
   <img width="1227" alt="Screen Shot 2020-08-03 at 5 00 28 PM" src="https://user-images.githubusercontent.com/8216382/89238833-45c74580-d5ac-11ea-8853-64158fdc682d.png">
   <img width="1227" alt="Screen Shot 2020-08-03 at 4 59 24 PM" src="https://user-images.githubusercontent.com/8216382/89238838-48c23600-d5ac-11ea-8f18-f4861d3990a0.png">
   
   ### TEST PLAN
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [x] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465261502



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {

Review comment:
       I think that makes sense, at least for now. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465246297



##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -0,0 +1,88 @@
+/**
+ * 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 { t } from '@superset-ui/translation';
+import TooltipWrapper from './TooltipWrapper';
+import Icon from './Icon';
+
+interface FaveStarProps {
+  itemId: number;
+  fetchFaveStar(id: number): any;
+  saveFaveStar(id: number, isStarred: boolean): any;
+  isStarred: boolean;
+  width?: number;
+  height?: number;
+  showTooltip?: boolean;
+}
+
+export default class FaveStar extends React.PureComponent<FaveStarProps> {

Review comment:
       thanks for the ts conversion! 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#issuecomment-679291783


   Impacts #8976


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465262527



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {
+          SupersetClient.get({
+            endpoint: `${FAVESTAR_BASE_URL}/${id}/count/`,
+          })
+            .then(({ json }) => {
+              const faves = {
+                ...this.state.favoriteStatus,
+              };
+
+              faves[id] = json.count > 0;
+
+              this.setState({
+                favoriteStatus: faves,
+              });
+            })
+            .catch(() => {});

Review comment:
       Toasts would be good, at the very least we should be giving a reason for favorite action failing as that could cause some confusion for the user. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465475834



##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -0,0 +1,88 @@
+/**

Review comment:
       nit (optional): for next time, let's use `git mv` when moving from `jsx` to `tsx` so that the file's git history stays attached to it. Now you're on the `git blame` for all the lines in the code :)

##########
File path: superset-frontend/src/components/FaveStar.tsx
##########
@@ -0,0 +1,88 @@
+/**

Review comment:
       nit (optional): for next time, let's use `git mv` when moving from `jsx` to `tsx` so that the file's git history stays attached to it. Now you're on the `git blame` for all the lines in the file :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465241504



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {
+          SupersetClient.get({
+            endpoint: `${FAVESTAR_BASE_URL}/${id}/count/`,
+          })
+            .then(({ json }) => {
+              const faves = {
+                ...this.state.favoriteStatus,
+              };
+
+              faves[id] = json.count > 0;
+
+              this.setState({
+                favoriteStatus: faves,
+              });
+            })
+            .catch(() => {});

Review comment:
       let's handle this exception. Swallowed exceptions make debugging very difficult. There should be other examples of handling promise errors in this component.  




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai merged pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai merged pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] riahk commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
riahk commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465249586



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {
+          SupersetClient.get({
+            endpoint: `${FAVESTAR_BASE_URL}/${id}/count/`,
+          })
+            .then(({ json }) => {
+              const faves = {
+                ...this.state.favoriteStatus,
+              };
+
+              faves[id] = json.count > 0;
+
+              this.setState({
+                favoriteStatus: faves,
+              });
+            })
+            .catch(() => {});

Review comment:
       What action should we be taking on an error? Should we be displaying a toast or just quietly logging the error?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter commented on pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#issuecomment-668869885


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=h1) Report
   > Merging [#10510](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c9cb723cab321b82c3153f1a2b9fde27e9270106&el=desc) will **increase** coverage by `0.14%`.
   > The diff coverage is `81.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10510/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10510      +/-   ##
   ==========================================
   + Coverage   70.85%   71.00%   +0.14%     
   ==========================================
     Files         604      605       +1     
     Lines       32400    32484      +84     
     Branches     3414     3438      +24     
   ==========================================
   + Hits        22956    23064     +108     
   + Misses       9332     9309      -23     
   + Partials      112      111       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `55.05% <80.00%> (+0.33%)` | :arrow_up: |
   | #javascript | `59.98% <75.40%> (+0.26%)` | :arrow_up: |
   | #python | `70.40% <ø> (+0.05%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/utils.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvdXRpbHMudHN4) | `64.10% <38.88%> (-21.62%)` | :arrow_down: |
   | [superset-frontend/src/components/FaveStar.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRmF2ZVN0YXIudHN4) | `100.00% <100.00%> (ø)` | |
   | [superset-frontend/src/components/Icon.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbi50c3g=) | `100.00% <100.00%> (ø)` | |
   | [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `55.78% <100.00%> (+0.30%)` | :arrow_up: |
   | [...tend/src/explore/components/ExploreChartHeader.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRIZWFkZXIuanN4) | `83.33% <100.00%> (+0.98%)` | :arrow_up: |
   | [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `71.09% <100.00%> (+1.67%)` | :arrow_up: |
   | [...rontend/src/views/CRUD/dashboard/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZExpc3QudHN4) | `70.07% <100.00%> (+1.74%)` | :arrow_up: |
   | [superset/views/alerts.py](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYWxlcnRzLnB5) | `70.83% <0.00%> (-29.17%)` | :arrow_down: |
   | [superset/examples/energy.py](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvZW5lcmd5LnB5) | `82.50% <0.00%> (-2.12%)` | :arrow_down: |
   | [superset/examples/unicode\_test\_data.py](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdW5pY29kZV90ZXN0X2RhdGEucHk=) | `80.00% <0.00%> (-1.64%)` | :arrow_down: |
   | ... and [46 more](https://codecov.io/gh/apache/incubator-superset/pull/10510/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=footer). Last update [c9cb723...07ce20c](https://codecov.io/gh/apache/incubator-superset/pull/10510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465245975



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {

Review comment:
       let's move these function definitions out of this function. Looks like they're pretty similar in both charts and dashboards (only a url change). Can we find a way to reuse this code? 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] nytai commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465365535



##########
File path: superset-frontend/src/views/CRUD/utils.tsx
##########
@@ -59,3 +59,56 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) {
     handleErrorFunc(parsedError.message);
   };
 }
+
+export function createFaveStarHandlers(
+  baseURL: string,
+  context: any,
+  handleErrorFunc: (message: string) => void,
+) {
+  const fetchFaveStar = (id: number) => {
+    SupersetClient.get({
+      endpoint: `${baseURL}/${id}/count/`,
+    })
+      .then(({ json }) => {
+        const faves = {
+          ...context.state.favoriteStatus,
+        };
+
+        faves[id] = json.count > 0;
+
+        context.setState({
+          favoriteStatus: faves,
+        });
+      })
+      .catch(() =>
+        handleErrorFunc('There was an error fetching the favorite status'),

Review comment:
       let's wrap this in a `t` string for i18n. 

##########
File path: superset-frontend/src/views/CRUD/utils.tsx
##########
@@ -59,3 +59,56 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) {
     handleErrorFunc(parsedError.message);
   };
 }
+
+export function createFaveStarHandlers(
+  baseURL: string,
+  context: any,
+  handleErrorFunc: (message: string) => void,
+) {
+  const fetchFaveStar = (id: number) => {
+    SupersetClient.get({
+      endpoint: `${baseURL}/${id}/count/`,
+    })
+      .then(({ json }) => {
+        const faves = {
+          ...context.state.favoriteStatus,
+        };
+
+        faves[id] = json.count > 0;
+
+        context.setState({
+          favoriteStatus: faves,
+        });
+      })
+      .catch(() =>
+        handleErrorFunc('There was an error fetching the favorite status'),
+      );
+  };
+
+  const saveFaveStar = (id: number, isStarred: boolean) => {
+    const urlSuffix = isStarred ? 'unselect' : 'select';
+
+    SupersetClient.get({
+      endpoint: `${baseURL}/${id}/${urlSuffix}/`,
+    })
+      .then(() => {
+        const faves = {
+          ...context.state.favoriteStatus,
+        };
+
+        faves[id] = !isStarred;
+
+        context.setState({
+          favoriteStatus: faves,
+        });
+      })
+      .catch(() =>
+        handleErrorFunc('There was an error saving the favorite status'),

Review comment:
       same here




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] riahk commented on a change in pull request #10510: feat: add favorite star to dashboard and chart lists

Posted by GitBox <gi...@apache.org>.
riahk commented on a change in pull request #10510:
URL: https://github.com/apache/incubator-superset/pull/10510#discussion_r465253895



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
##########
@@ -106,6 +110,61 @@ class DashboardList extends React.PureComponent<Props, State> {
   initialSort = [{ id: 'changed_on_delta_humanized', desc: true }];
 
   columns = [
+    {
+      Cell: ({ row: { original } }: any) => {
+        // TODO: return null if no user is logged in
+        const fetchFaveStar = (id: number) => {

Review comment:
       Would it make sense to add them to `src/views/CRUD/utils.tsx`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org