remove duplicated backend logic

superset-frontend/src/explore/components/SaveModal.test.jsx

@@ -64,7 +64,7 @@ describe('SaveModal', () => {
       }
       return arg;
     }),
-    form_data: { datasource: '107__table' },
+    form_data: { datasource: '107__table', url_params: { foo: 'bar' } },
   };
   const mockEvent = {
     target: {
@@ -159,7 +159,6 @@ describe('SaveModal', () => {
         Promise.resolve({
           dashboard_url: 'http://localhost/mock_dashboard/',
           slice: { slice_url: '/mock_slice/' },
-          url_params: { foo: 'bar' },
         }),
       );
     });
@@ -169,11 +168,11 @@ describe('SaveModal', () => {
       defaultProps.actions.saveSlice.restore();
     });
 
-    it('should save slice', () => {
+    it('should save slice without url_params in form_data', () => {
       const wrapper = getWrapper();
       wrapper.instance().saveOrOverwrite(true);
       const { args } = defaultProps.actions.saveSlice.getCall(0);
-      expect(args[0]).toEqual(defaultProps.form_data);
+      expect(args[0]).toEqual({ datasource: '107__table' });
     });
 
     it('existing dashboard', () => {

superset-frontend/src/explore/components/SaveModal.tsx

@@ -138,9 +138,10 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
     sliceParams.slice_name = this.state.newSliceName;
     sliceParams.save_to_dashboard_id = this.state.saveToDashboardId;
     sliceParams.new_dashboard_name = this.state.newDashboardName;
+    const { url_params, ...formData } = this.props.form_data || {};
 
     this.props.actions
-      .saveSlice(this.props.form_data, sliceParams)
+      .saveSlice(formData, sliceParams)
       .then((data: JsonObject) => {
         if (data.dashboard_id === null) {
           sessionStorage.removeItem(SK_DASHBOARD_ID);
@@ -149,7 +150,6 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
         }
         // Go to new slice url or dashboard url
         let url = gotodash ? data.dashboard_url : data.slice.slice_url;
-        const { url_params } = data;
         if (url_params) {
           const prefix = url.includes('?') ? '&' : '?';
           url = `${url}${prefix}${new URLSearchParams(url_params).toString()}`;

superset/charts/api.py

@@ -283,7 +283,7 @@ def post(self) -> Response:
             return self.response_400(message=error.messages)
         try:
             new_model = CreateChartCommand(g.user, item).run()
-            return self.response(201, id=new_model.id, result=new_model.to_json())
+            return self.response(201, id=new_model.id, result=item)
         except ChartInvalidError as ex:
             return self.response_422(message=ex.normalized_messages())
         except ChartCreateFailedError as ex:
@@ -354,9 +354,7 @@ def put(self, pk: int) -> Response:
             return self.response_400(message=error.messages)
         try:
             changed_model = UpdateChartCommand(g.user, pk, item).run()
-            response = self.response(
-                200, id=changed_model.id, result=changed_model.to_json()
-            )
+            response = self.response(200, id=changed_model.id, result=item)
         except ChartNotFoundError:
             response = self.response_404()
         except ChartForbiddenError:

superset/charts/commands/create.py

@@ -14,7 +14,6 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-import json
 import logging
 from datetime import datetime
 from typing import Any, Dict, List, Optional
@@ -28,7 +27,6 @@
     ChartInvalidError,
     DashboardsNotFoundValidationError,
 )
-from superset.charts.commands.utils import sanitize_metadata
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand, CreateMixin
 from superset.commands.utils import get_datasource_by_id
@@ -45,7 +43,6 @@ def __init__(self, user: User, data: Dict[str, Any]):
 
     def run(self) -> Model:
         self.validate()
-        sanitize_metadata(self)
         try:
             self._properties["last_saved_at"] = datetime.now()
             self._properties["last_saved_by"] = self._actor

superset/charts/commands/update.py

@@ -30,7 +30,6 @@
     DashboardsNotFoundValidationError,
     DatasourceTypeUpdateRequiredValidationError,
 )
-from superset.charts.commands.utils import sanitize_metadata
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand, UpdateMixin
 from superset.commands.utils import get_datasource_by_id
@@ -53,12 +52,11 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
     def __init__(self, user: User, model_id: int, data: Dict[str, Any]):
         self._actor = user
         self._model_id = model_id
-        self._model: Optional[Slice] = None
         self._properties = data.copy()
+        self._model: Optional[Slice] = None
 
     def run(self) -> Model:
         self.validate()
-        sanitize_metadata(self)
         try:
             if self._properties.get("query_context_generation") is None:
                 self._properties["last_saved_at"] = datetime.now()

superset/views/core.py

@@ -985,7 +985,6 @@ def save_or_overwrite_slice(
         slice_name = request.args.get("slice_name")
         action = request.args.get("action")
         form_data = get_form_data()[0]
-        url_params = form_data.pop("url_params", None)
 
         if action == "saveas":
             if "slice_id" in form_data:
@@ -1079,7 +1078,6 @@ def save_or_overwrite_slice(
             "slice": slc.data,
             "dashboard_url": dash.url if dash else None,
             "dashboard_id": dash.id if dash else None,
-            "url_params": url_params,
         }
 
         if dash and request.args.get("goto_dash") == "true":

tests/integration_tests/charts/api_tests.py

@@ -439,9 +439,7 @@ def test_create_chart(self):
             "description": "description1",
             "owners": [admin_id],
             "viz_type": "viz_type1",
-            "params": json.dumps(
-                {"viz_type": "viz_type1", "url_params": {"foo": "bar"}},
-            ),
+            "params": "1234",
             "cache_timeout": 1000,
             "datasource_id": 1,
             "datasource_type": "table",
@@ -454,10 +452,6 @@ def test_create_chart(self):
         rv = self.post_assert_metric(uri, chart_data, "post")
         self.assertEqual(rv.status_code, 201)
         data = json.loads(rv.data.decode("utf-8"))
-        result = data["result"]
-        assert result["viz_type"] == "viz_type1"
-        params = json.loads(result["params"])
-        assert params == {"viz_type": "viz_type1"}
         model = db.session.query(Slice).get(data.get("id"))
         db.session.delete(model)
         db.session.commit()
@@ -562,9 +556,7 @@ def test_update_chart(self):
             "description": "description1",
             "owners": [gamma.id],
             "viz_type": "viz_type1",
-            "params": json.dumps(
-                {"viz_type": "viz_type1", "url_params": {"foo": "bar"}},
-            ),
+            "params": """{"a": 1}""",
             "cache_timeout": 1000,
             "datasource_id": birth_names_table_id,
             "datasource_type": "table",
@@ -584,8 +576,7 @@ def test_update_chart(self):
         self.assertNotIn(admin, model.owners)
         self.assertIn(gamma, model.owners)
         self.assertEqual(model.viz_type, "viz_type1")
-        params = json.loads(model.params)
-        assert params == {"viz_type": "viz_type1"}
+        self.assertEqual(model.params, """{"a": 1}""")
         self.assertEqual(model.cache_timeout, 1000)
         self.assertEqual(model.datasource_id, birth_names_table_id)
         self.assertEqual(model.datasource_type, "table")

tests/integration_tests/charts/commands_tests.py

@@ -329,27 +329,24 @@ def test_import_v1_chart_validation(self):
         }
 
 
-class TestChartsUpdateCommand(SupersetTestCase):
+class TestChartsCreateCommand(SupersetTestCase):
     @patch("superset.views.base.g")
     @patch("superset.security.manager.g")
     @pytest.mark.usefixtures("load_energy_table_with_slice")
     def test_create_v1_response(self, mock_sm_g, mock_g):
-        """Test that the create chart command creates a chart XXX"""
+        """Test that the create chart command creates a chart"""
         actor = security_manager.find_user(username="admin")
         mock_g.user = mock_sm_g.user = actor
         chart_data = {
             "slice_name": "new chart",
             "description": "new description",
             "owners": [actor.id],
             "viz_type": "new_viz_type",
-            "params": json.dumps(
-                {"viz_type": "new_viz_type", "url_params": {"foo": "bar"}}
-            ),
+            "params": json.dumps({"viz_type": "new_viz_type"}),
             "cache_timeout": 1000,
             "datasource_id": 1,
             "datasource_type": "table",
         }
-
         command = CreateChartCommand(actor, chart_data)
         chart = command.run()
         chart = db.session.query(Slice).get(chart.id)
@@ -361,11 +358,13 @@ def test_create_v1_response(self, mock_sm_g, mock_g):
         db.session.delete(chart)
         db.session.commit()
 
+
+class TestChartsUpdateCommand(SupersetTestCase):
     @patch("superset.views.base.g")
     @patch("superset.security.manager.g")
     @pytest.mark.usefixtures("load_energy_table_with_slice")
     def test_update_v1_response(self, mock_sm_g, mock_g):
-        """Test that the update chart command updates properties"""
+        """Test that a chart command updates properties"""
         pk = db.session.query(Slice).all()[0].id
         actor = security_manager.find_user(username="admin")
         mock_g.user = mock_sm_g.user = actor
@@ -374,20 +373,13 @@ def test_update_v1_response(self, mock_sm_g, mock_g):
             "description": "test for update",
             "cache_timeout": None,
             "owners": [actor.id],
-            "viz_type": "update_viz_type",
-            "params": json.dumps(
-                {"viz_type": "update_viz_type", "url_params": {"foo": "bar"},}
-            ),
         }
         command = UpdateChartCommand(actor, model_id, json_obj)
         last_saved_before = db.session.query(Slice).get(pk).last_saved_at
         command.run()
         chart = db.session.query(Slice).get(pk)
         assert chart.last_saved_at != last_saved_before
         assert chart.last_saved_by == actor
-        assert chart.viz_type == "update_viz_type"
-        params = json.loads(chart.params)
-        assert params == {"viz_type": "update_viz_type"}
 
     @patch("superset.views.base.g")
     @patch("superset.security.manager.g")