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/02/26 20:00:36 UTC

[GitHub] [incubator-superset] suddjian opened a new pull request #9211: show edit modal on dashboards list view

suddjian opened a new pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211
 
 
   ### CATEGORY
   
   Choose one
   
   - [ ] Bug Fix
   - [x] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   This changes the dashboard react list view to use the react-based edit modal, instead of linking to the CRUD-based edit page.
   
   A detail of this implementation is that after editing a dashboard, it will stay in the list even if it is no longer applicable based on the chosen filters. This is intended, as removing a dashboard that has been just edited from the list could be an annoying behavior.
   
   I had to remove the "owners" accessor column as it doesn't actually work correctly when a dashboard has `owners` populated. React complains about trying to render an object. Owners will need a component rather than just an accessor.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   After clicking the edit button
   <img width="1017" alt="Screen Shot 2020-02-26 at 11 51 47 AM" src="https://user-images.githubusercontent.com/1858430/75382042-a5e3ee00-588e-11ea-8861-40ebf88c0f90.png">
   
   After saving the dashboard
   <img width="1025" alt="Screen Shot 2020-02-26 at 11 51 56 AM" src="https://user-images.githubusercontent.com/1858430/75382060-a8dede80-588e-11ea-8ea4-bc8dd819f8d5.png">
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   - Added a simple unit test
   - Smoke tested
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] 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
   
   ### REVIEWERS
   @nytai 

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r388477623
 
 

 ##########
 File path: superset/views/dashboard/api.py
 ##########
 @@ -142,19 +142,26 @@ class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi):
 
     class_permission_name = "DashboardModelView"
     show_columns = [
+        "id",
         "charts",
         "css",
         "dashboard_title",
         "json_metadata",
         "owners.id",
         "owners.username",
+        "changed_by_name",
+        "changed_by_url",
+        "changed_by.username",
+        "changed_on",
         "position_json",
         "published",
+        "url",
         "slug",
         "table_names",
     ]
     order_columns = ["dashboard_title", "changed_on", "published", "changed_by_fk"]
     list_columns = [
+        "json_metadata",
 
 Review comment:
   I ended up simplifying the contract for `PropertiesModal`, now it just takes a dashboard id and fetches everything it needs on its own. Not ideal, it'd be nicer to pass a dashboard object as a prop, but we don't have a super consistent data model for a dashboard in every place that it's used, so it's probably the best option 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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387356541
 
 

 ##########
 File path: superset/views/dashboard/api.py
 ##########
 @@ -142,19 +142,26 @@ class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi):
 
     class_permission_name = "DashboardModelView"
     show_columns = [
+        "id",
         "charts",
         "css",
         "dashboard_title",
         "json_metadata",
         "owners.id",
         "owners.username",
+        "changed_by_name",
+        "changed_by_url",
+        "changed_by.username",
+        "changed_on",
         "position_json",
         "published",
+        "url",
         "slug",
         "table_names",
     ]
     order_columns = ["dashboard_title", "changed_on", "published", "changed_by_fk"]
     list_columns = [
+        "json_metadata",
 
 Review comment:
   That's a good point, we could use the metadata from the modal's fetch call.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387358386
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   happy to merge this, this shouldn't affect perf much. However, we should figure out how to configure the PUT response

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r389955371
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   I remember this now. The data they return uses the `edit_model_schema` Marshmallow schema which is the same as the one used to parse incoming edited fields from the client. IMO a better design would be to return the entire object as it would from the `GET` endpoint, but that's a larger scale API design decision.

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387356006
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   I think there was some field that wasn't returned from the `PUT` endpoint. I wasn't sure how to add that field and eventually just did this to make it work. If you think it's worth the effort I'll take another pass at it, otherwise I can add a comment documenting why it's there.

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r389955371
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   I remember this now. The API endpoint uses the `edit_model_schema` Marshmallow schema when returning the edited object, which is the same as the one used to parse incoming edited fields from the client. IMO a better design would be to return the entire object as it would from the `GET` endpoint, but that's a larger scale API design decision.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387358548
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   might be `edit_columns`

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] rusackas merged pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
rusackas merged pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211
 
 
   

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] codecov-io commented on issue #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#issuecomment-597826981
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9211?src=pr&el=h1) Report
   > Merging [#9211](https://codecov.io/gh/apache/incubator-superset/pull/9211?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cd4605e4c1f1716dc182a6686991e971df924920&el=desc) will **increase** coverage by `0.21%`.
   > The diff coverage is `52.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/9211/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/9211?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9211      +/-   ##
   ==========================================
   + Coverage   58.92%   59.14%   +0.21%     
   ==========================================
     Files         372      373       +1     
     Lines       11999    12057      +58     
     Branches     2940     2961      +21     
   ==========================================
   + Hits         7071     7131      +60     
   + Misses       4750     4747       -3     
   - Partials      178      179       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9211?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...erset-frontend/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci5qc3g=) | `41.79% <ø> (ø)` | |
   | [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `45.83% <50.00%> (+39.27%)` | :arrow_up: |
   | [...frontend/src/views/dashboardList/DashboardList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2Rhc2hib2FyZExpc3QvRGFzaGJvYXJkTGlzdC50c3g=) | `63.63% <54.16%> (+4.28%)` | :arrow_up: |
   | [...erset-frontend/src/SqlLab/components/ResultSet.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC5qc3g=) | `75.70% <0.00%> (-1.86%)` | :arrow_down: |
   | [...t-frontend/src/dashboard/actions/dashboardState.js](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2Rhc2hib2FyZFN0YXRlLmpz) | `30.66% <0.00%> (-1.06%)` | :arrow_down: |
   | [...uperset-frontend/src/views/chartList/ChartList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2NoYXJ0TGlzdC9DaGFydExpc3QudHN4) | `62.72% <0.00%> (-0.67%)` | :arrow_down: |
   | [superset-frontend/src/chart/Chart.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0LmpzeA==) | `10.41% <0.00%> (ø)` | |
   | [superset-frontend/src/setup/setupClient.js](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ2xpZW50Lmpz) | `0.00% <0.00%> (ø)` | |
   | [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `61.25% <0.00%> (ø)` | |
   | ... and [10 more](https://codecov.io/gh/apache/incubator-superset/pull/9211/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9211?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/9211?src=pr&el=footer). Last update [cd4605e...b3ff8fc](https://codecov.io/gh/apache/incubator-superset/pull/9211?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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387350179
 
 

 ##########
 File path: superset/views/dashboard/api.py
 ##########
 @@ -142,19 +142,26 @@ class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi):
 
     class_permission_name = "DashboardModelView"
     show_columns = [
+        "id",
         "charts",
         "css",
         "dashboard_title",
         "json_metadata",
         "owners.id",
         "owners.username",
+        "changed_by_name",
+        "changed_by_url",
+        "changed_by.username",
+        "changed_on",
         "position_json",
         "published",
+        "url",
         "slug",
         "table_names",
     ]
     order_columns = ["dashboard_title", "changed_on", "published", "changed_by_fk"]
     list_columns = [
+        "json_metadata",
 
 Review comment:
   Is this property consumed in the listview? Seems like we're fetching the full dashboard show payload on edit.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r387351674
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   Is there a reason we're making this call instead of just updating from `edits`? 

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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] suddjian commented on a change in pull request #9211: show edit modal on dashboards list view

Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9211: show edit modal on dashboards list view
URL: https://github.com/apache/incubator-superset/pull/9211#discussion_r389968916
 
 

 ##########
 File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
 ##########
 @@ -251,8 +250,33 @@ class DashboardList extends React.PureComponent<Props, State> {
     return Boolean(this.state.permissions.find(p => p === perm));
   };
 
-  handleDashboardEdit = ({ id }: { id: number }) => {
-    window.location.assign(`/dashboard/edit/${id}`);
+  openDashboardEditModal = (dashboard: Dashboard) => {
+    this.setState({
+      dashboardToEdit: dashboard,
+    });
+  };
+
+  handleDashboardEdit = (edits: any, original: Dashboard) => {
+    this.setState({ loading: true });
+    return SupersetClient.get({
+      endpoint: `/api/v1/dashboard/${original.id}`,
 
 Review comment:
   In this particular case, the impact is that `slug` can be changed, which changes the `url`, but only `slug` is returned from the endpoint.

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


With regards,
Apache Git Services

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