This is an automated email from the ASF dual-hosted git repository.

graceguo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 4f4367e  feat: prevent co-edit dashboard collision (#11220)
4f4367e is described below

commit 4f4367edf37cbd70fdeac276d066ae5c0bf95ed0
Author: Grace Guo <[email protected]>
AuthorDate: Mon Oct 12 17:58:32 2020 -0700

    feat: prevent co-edit dashboard collision (#11220)
    
    * feat: prevent co-edit dashboard collision
    
    * fix comments
---
 .../src/dashboard/components/Header.jsx              |  1 +
 .../src/dashboard/components/SaveModal.jsx           |  1 +
 .../src/dashboard/reducers/getInitialState.js        |  1 +
 superset/models/dashboard.py                         |  1 +
 superset/views/core.py                               | 20 ++++++++++++++++++++
 tests/dashboard_tests.py                             | 15 +++++++++++++++
 6 files changed, 39 insertions(+)

diff --git a/superset-frontend/src/dashboard/components/Header.jsx 
b/superset-frontend/src/dashboard/components/Header.jsx
index c986a31..2dba655 100644
--- a/superset-frontend/src/dashboard/components/Header.jsx
+++ b/superset-frontend/src/dashboard/components/Header.jsx
@@ -290,6 +290,7 @@ class Header extends React.PureComponent {
       label_colors: labelColors,
       dashboard_title: dashboardTitle,
       refresh_frequency: refreshFrequency,
+      last_modified_time: dashboardInfo.lastModifiedTime,
     };
 
     // make sure positions data less than DB storage limitation:
diff --git a/superset-frontend/src/dashboard/components/SaveModal.jsx 
b/superset-frontend/src/dashboard/components/SaveModal.jsx
index 3e69d53..03dafb5 100644
--- a/superset-frontend/src/dashboard/components/SaveModal.jsx
+++ b/superset-frontend/src/dashboard/components/SaveModal.jsx
@@ -129,6 +129,7 @@ class SaveModal extends React.PureComponent {
         saveType === SAVE_TYPE_NEWDASHBOARD ? newDashName : dashboardTitle,
       duplicate_slices: this.state.duplicateSlices,
       refresh_frequency: refreshFrequency,
+      last_modified_time: dashboardInfo.lastModifiedTime,
     };
 
     if (saveType === SAVE_TYPE_NEWDASHBOARD && !newDashName) {
diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js 
b/superset-frontend/src/dashboard/reducers/getInitialState.js
index d9ddc59..3e5b6ea 100644
--- a/superset-frontend/src/dashboard/reducers/getInitialState.js
+++ b/superset-frontend/src/dashboard/reducers/getInitialState.js
@@ -266,6 +266,7 @@ export default function getInitialState(bootstrapData) {
       id: dashboard.id,
       slug: dashboard.slug,
       metadata: dashboard.metadata,
+      lastModifiedTime: dashboard.last_modified_time,
       userId: user_id,
       dash_edit_perm: dashboard.dash_edit_perm,
       dash_save_perm: dashboard.dash_save_perm,
diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py
index 8648479..56ca1ba 100644
--- a/superset/models/dashboard.py
+++ b/superset/models/dashboard.py
@@ -237,6 +237,7 @@ class Dashboard(  # pylint: 
disable=too-many-instance-attributes
             "slug": self.slug,
             "slices": [slc.data for slc in self.slices],
             "position_json": positions,
+            "last_modified_time": 
self.changed_on.replace(microsecond=0).timestamp(),
         }
 
     @property  # type: ignore
diff --git a/superset/views/core.py b/superset/views/core.py
index fd62a0e..c6847b7 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -1019,6 +1019,11 @@ class Superset(BaseSupersetView):  # pylint: 
disable=too-many-public-methods
         """Copy dashboard"""
         session = db.session()
         data = json.loads(request.form["data"])
+        # client-side send back last_modified_time which was set when
+        # the dashboard was open. it was use to avoid mid-air collision.
+        # remove it to avoid confusion.
+        data.pop("last_modified_time", None)
+
         dash = models.Dashboard()
         original_dash = session.query(Dashboard).get(dashboard_id)
 
@@ -1065,6 +1070,21 @@ class Superset(BaseSupersetView):  # pylint: 
disable=too-many-public-methods
         dash = session.query(Dashboard).get(dashboard_id)
         check_ownership(dash, raise_if_false=True)
         data = json.loads(request.form["data"])
+        # client-side send back last_modified_time which was set when
+        # the dashboard was open. it was use to avoid mid-air collision.
+        remote_last_modified_time = data.get("last_modified_time")
+        current_last_modified_time = 
dash.changed_on.replace(microsecond=0).timestamp()
+        if remote_last_modified_time < current_last_modified_time:
+            return json_error_response(
+                __(
+                    "This dashboard was changed recently. "
+                    "Please reload dashboard to get latest version."
+                ),
+                412,
+            )
+        # remove to avoid confusion.
+        data.pop("last_modified_time", None)
+
         DashboardDAO.set_dash_metadata(dash, data)
         session.merge(dash)
         session.commit()
diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py
index 27926e4..3888cf3 100644
--- a/tests/dashboard_tests.py
+++ b/tests/dashboard_tests.py
@@ -16,6 +16,7 @@
 # under the License.
 # isort:skip_file
 """Unit tests for Superset"""
+from datetime import datetime
 import json
 import unittest
 from random import random
@@ -89,6 +90,8 @@ class TestDashboard(SupersetTestCase):
             "expanded_slices": {},
             "positions": positions,
             "dashboard_title": dash.dashboard_title,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
         url = "/superset/save_dash/{}/".format(dash.id)
         resp = self.get_resp(url, data=dict(data=json.dumps(data)))
@@ -107,6 +110,8 @@ class TestDashboard(SupersetTestCase):
             "positions": positions,
             "dashboard_title": dash.dashboard_title,
             "default_filters": default_filters,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
 
         url = "/superset/save_dash/{}/".format(dash.id)
@@ -134,6 +139,8 @@ class TestDashboard(SupersetTestCase):
             "positions": positions,
             "dashboard_title": dash.dashboard_title,
             "default_filters": default_filters,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
 
         url = "/superset/save_dash/{}/".format(dash.id)
@@ -154,6 +161,8 @@ class TestDashboard(SupersetTestCase):
             "expanded_slices": {},
             "positions": positions,
             "dashboard_title": "new title",
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
         url = "/superset/save_dash/{}/".format(dash.id)
         self.get_resp(url, data=dict(data=json.dumps(data)))
@@ -176,6 +185,8 @@ class TestDashboard(SupersetTestCase):
             "color_namespace": "Color Namespace Test",
             "color_scheme": "Color Scheme Test",
             "label_colors": new_label_colors,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
         url = "/superset/save_dash/{}/".format(dash.id)
         self.get_resp(url, data=dict(data=json.dumps(data)))
@@ -203,6 +214,8 @@ class TestDashboard(SupersetTestCase):
             "color_namespace": "Color Namespace Test",
             "color_scheme": "Color Scheme Test",
             "label_colors": new_label_colors,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
 
         # Save changes to Births dashboard and retrieve updated dash
@@ -271,6 +284,8 @@ class TestDashboard(SupersetTestCase):
             "expanded_slices": {},
             "positions": positions,
             "dashboard_title": dash.dashboard_title,
+            # set a further modified_time for unit test
+            "last_modified_time": datetime.now().timestamp() + 1000,
         }
 
         # save dash

Reply via email to