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