From 5a099e0762ff26dbace04e2a11557c351bc00541 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Wed, 1 Mar 2023 13:32:04 +0200 Subject: [PATCH] fix(dao): use explicit id filter (#23246) --- superset/dao/base.py | 4 ++-- tests/integration_tests/charts/api_tests.py | 18 +++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/superset/dao/base.py b/superset/dao/base.py index 28cfdf2cc625e..0cffb0f334598 100644 --- a/superset/dao/base.py +++ b/superset/dao/base.py @@ -65,9 +65,9 @@ def find_by_id( query = cls.base_filter( # pylint: disable=not-callable cls.id_column_name, data_model ).apply(query, None) - id_filter = {cls.id_column_name: model_id} + id_column = getattr(cls.model_cls, cls.id_column_name) try: - return query.filter_by(**id_filter).one_or_none() + return query.filter(id_column == model_id).one_or_none() except StatementError: # can happen if int is passed instead of a string or similar return None diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 965a9c137ba87..38fa1b7a6c9d3 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -609,17 +609,21 @@ def test_update_chart_new_owner_not_admin(self): """ Chart API: Test update set new owner implicitly adds logged in owner """ - gamma = self.get_user("gamma") + gamma = self.get_user("gamma_no_csv") alpha = self.get_user("alpha") - chart_id = self.insert_chart("title", [alpha.id], 1).id - chart_data = {"slice_name": "title1_changed", "owners": [gamma.id]} - self.login(username="alpha") + chart_id = self.insert_chart("title", [gamma.id], 1).id + chart_data = { + "slice_name": (new_name := "title1_changed"), + "owners": [alpha.id], + } + self.login(username=gamma.username) uri = f"api/v1/chart/{chart_id}" rv = self.put_assert_metric(uri, chart_data, "put") - self.assertEqual(rv.status_code, 200) + assert rv.status_code == 200 model = db.session.query(Slice).get(chart_id) - self.assertIn(alpha, model.owners) - self.assertIn(gamma, model.owners) + assert model.slice_name == new_name + assert alpha in model.owners + assert gamma in model.owners db.session.delete(model) db.session.commit()