diff --git a/superset/commands/chart/update.py b/superset/commands/chart/update.py index d6b212d5ce861..904749445c25d 100644 --- a/superset/commands/chart/update.py +++ b/superset/commands/chart/update.py @@ -63,7 +63,7 @@ def run(self) -> Model: # Update tags if (tags := self._properties.pop("tags", None)) is not None: - update_tags(ObjectType.chart, self._model.id, self._model.tags, tags) + update_tags(ObjectType.CHART, self._model.id, self._model.tags, tags) if self._properties.get("query_context_generation") is None: self._properties["last_saved_at"] = datetime.now() @@ -79,6 +79,7 @@ def validate(self) -> None: # Validate if datasource_id is provided datasource_type is required datasource_id = self._properties.get("datasource_id") + datasource_type: str | None = None if datasource_id is not None: datasource_type = self._properties.get("datasource_type", "") if not datasource_type: @@ -106,12 +107,12 @@ def validate(self) -> None: # validate tags try: - validate_tags(ObjectType.chart, self._model.tags, tag_ids) + validate_tags(ObjectType.CHART, self._model.tags, tag_ids) except ValidationError as ex: exceptions.append(ex) # Validate/Populate datasource - if datasource_id is not None: + if datasource_id is not None and datasource_type is not None: try: datasource = get_datasource_by_id(datasource_id, datasource_type) self._properties["datasource_name"] = datasource.name diff --git a/superset/commands/dashboard/update.py b/superset/commands/dashboard/update.py index 15f5e5b5841b8..22601ee073b87 100644 --- a/superset/commands/dashboard/update.py +++ b/superset/commands/dashboard/update.py @@ -60,7 +60,7 @@ def run(self) -> Model: # Update tags if (tags := self._properties.pop("tags", None)) is not None: - update_tags(ObjectType.dashboard, self._model.id, self._model.tags, tags) + update_tags(ObjectType.DASHBOARD, self._model.id, self._model.tags, tags) dashboard = DashboardDAO.update(self._model, self._properties) if self._properties.get("json_metadata"): @@ -103,7 +103,7 @@ def validate(self) -> None: # validate tags try: - validate_tags(ObjectType.dashboard, self._model.tags, tag_ids) + validate_tags(ObjectType.DASHBOARD, self._model.tags, tag_ids) except ValidationError as ex: exceptions.append(ex) diff --git a/superset/commands/utils.py b/superset/commands/utils.py index 7486d657adc60..7da327b92d686 100644 --- a/superset/commands/utils.py +++ b/superset/commands/utils.py @@ -130,7 +130,7 @@ def validate_tags( return # No changes in the list - current_custom_tags = [tag.id for tag in current_tags if tag.type == TagType.custom] + current_custom_tags = [tag.id for tag in current_tags if tag.type == TagType.CUSTOM] if Counter(current_custom_tags) == Counter(new_tag_ids): return @@ -140,7 +140,7 @@ def validate_tags( or security_manager.can_access("can_tag", object_type.name.capitalize()) ): validation_error = ( - f"You do not have permission to manage tags on {object_type.name}s" + f"You do not have permission to manage tags on {object_type.name.lower()}s" ) raise TagForbiddenError(validation_error) @@ -169,9 +169,9 @@ def update_tags( :param new_tag_ids: list of tags specified in the update payload """ - current_custom_tags = [tag for tag in current_tags if tag.type == TagType.custom] + current_custom_tags = [tag for tag in current_tags if tag.type == TagType.CUSTOM] current_custom_tag_ids = [ - tag.id for tag in current_tags if tag.type == TagType.custom + tag.id for tag in current_tags if tag.type == TagType.CUSTOM ] tags_to_delete = [tag for tag in current_custom_tags if tag.id not in new_tag_ids] diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index fc00dd3bc2955..a09a2cf4a324c 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -218,7 +218,7 @@ def create_chart_with_tag(self, create_custom_tags): # noqa: F811 tag = db.session.query(Tag).filter(Tag.name == "first_tag").first() tag_association = TaggedObject( object_id=chart.id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tag, ) @@ -263,22 +263,22 @@ def create_charts_some_with_tags(self, create_custom_tags): # noqa: F811 tag_associations = [ TaggedObject( object_id=charts[0].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=charts[1].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), TaggedObject( object_id=charts[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=charts[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), ] @@ -2136,7 +2136,7 @@ def test_update_chart_add_tags_can_write_on_tag(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in chart.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in chart.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -2146,7 +2146,7 @@ def test_update_chart_add_tags_can_write_on_tag(self): model = db.session.query(Slice).get(chart.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == new_tags @pytest.mark.usefixtures("create_chart_with_tag") @@ -2162,7 +2162,7 @@ def test_update_chart_remove_tags_can_write_on_tag(self): ) # get existing tag and add a new one - new_tags = [tag.id for tag in chart.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in chart.tags if tag.type == TagType.CUSTOM] new_tags.pop() update_payload = {"tags": new_tags} @@ -2173,7 +2173,7 @@ def test_update_chart_remove_tags_can_write_on_tag(self): model = db.session.query(Slice).get(chart.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == new_tags @pytest.mark.usefixtures("create_chart_with_tag") @@ -2195,7 +2195,7 @@ def test_update_chart_add_tags_can_tag_on_chart(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in chart.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in chart.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -2205,7 +2205,7 @@ def test_update_chart_add_tags_can_tag_on_chart(self): model = db.session.query(Slice).get(chart.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == new_tags security_manager.add_permission_role(alpha_role, write_tags_perm) @@ -2235,7 +2235,7 @@ def test_update_chart_remove_tags_can_tag_on_chart(self): model = db.session.query(Slice).get(chart.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == [] security_manager.add_permission_role(alpha_role, write_tags_perm) @@ -2260,7 +2260,7 @@ def test_update_chart_add_tags_missing_permission(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in chart.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in chart.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -2321,7 +2321,7 @@ def test_update_chart_no_tag_changes(self): chart = ( db.session.query(Slice).filter(Slice.slice_name == "chart with tag").first() ) - existing_tags = [tag.id for tag in chart.tags if tag.type == TagType.custom] + existing_tags = [tag.id for tag in chart.tags if tag.type == TagType.CUSTOM] update_payload = {"tags": existing_tags} uri = f"api/v1/chart/{chart.id}" diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 6c6e11c8969c3..5f5f4814b883d 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -194,7 +194,7 @@ def create_dashboard_with_tag(self, create_custom_tags): # noqa: F811 tag = db.session.query(Tag).filter(Tag.name == "first_tag").first() tag_association = TaggedObject( object_id=dashboard.id, - object_type=ObjectType.dashboard, + object_type=ObjectType.DASHBOARD, tag=tag, ) @@ -245,22 +245,22 @@ def create_dashboards_some_with_tags(self, create_custom_tags): # noqa: F811 tag_associations = [ TaggedObject( object_id=dashboards[0].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=dashboards[1].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), TaggedObject( object_id=dashboards[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=dashboards[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), ] @@ -2804,7 +2804,7 @@ def test_update_dashboard_add_tags_can_write_on_tag(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -2814,7 +2814,7 @@ def test_update_dashboard_add_tags_can_write_on_tag(self): model = db.session.query(Dashboard).get(dashboard.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert sorted(tag_list) == sorted(new_tags) @pytest.mark.usefixtures("create_dashboard_with_tag") @@ -2832,7 +2832,7 @@ def test_update_dashboard_remove_tags_can_write_on_tag(self): ) # get existing tag and add a new one - new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.CUSTOM] new_tags.pop() update_payload = {"tags": new_tags} @@ -2843,7 +2843,7 @@ def test_update_dashboard_remove_tags_can_write_on_tag(self): model = db.session.query(Dashboard).get(dashboard.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == new_tags @pytest.mark.usefixtures("create_dashboard_with_tag") @@ -2866,7 +2866,7 @@ def test_update_dashboard_add_tags_can_tag_on_dashboard(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -2876,7 +2876,7 @@ def test_update_dashboard_add_tags_can_tag_on_dashboard(self): model = db.session.query(Dashboard).get(dashboard.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert sorted(tag_list) == sorted(new_tags) security_manager.add_permission_role(gamma_role, write_tags_perm) @@ -2907,7 +2907,7 @@ def test_update_dashboard_remove_tags_can_tag_on_dashboard(self): model = db.session.query(Dashboard).get(dashboard.id) # Clean up system tags - tag_list = [tag.id for tag in model.tags if tag.type == TagType.custom] + tag_list = [tag.id for tag in model.tags if tag.type == TagType.CUSTOM] assert tag_list == [] security_manager.add_permission_role(gamma_role, write_tags_perm) @@ -2935,7 +2935,7 @@ def test_update_dashboard_add_tags_missing_permission(self): new_tag = db.session.query(Tag).filter(Tag.name == "second_tag").one() # get existing tag and add a new one - new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.custom] + new_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.CUSTOM] new_tags.append(new_tag.id) update_payload = {"tags": new_tags} @@ -3004,7 +3004,7 @@ def test_update_dashboard_no_tag_changes(self): .filter(Dashboard.dashboard_title == "dash with tag") .first() ) - existing_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.custom] + existing_tags = [tag.id for tag in dashboard.tags if tag.type == TagType.CUSTOM] update_payload = {"tags": existing_tags} uri = f"api/v1/dashboard/{dashboard.id}" diff --git a/tests/integration_tests/queries/saved_queries/api_tests.py b/tests/integration_tests/queries/saved_queries/api_tests.py index aa2c931104e4e..5341f798ae6ce 100644 --- a/tests/integration_tests/queries/saved_queries/api_tests.py +++ b/tests/integration_tests/queries/saved_queries/api_tests.py @@ -163,22 +163,22 @@ def create_saved_queries_some_with_tags(self, create_custom_tags): # noqa: F811 tag_associations = [ TaggedObject( object_id=queries[0].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=queries[1].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), TaggedObject( object_id=queries[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["first_tag"], ), TaggedObject( object_id=queries[2].id, - object_type=ObjectType.chart, + object_type=ObjectType.CHART, tag=tags["second_tag"], ), ] diff --git a/tests/unit_tests/commands/test_utils.py b/tests/unit_tests/commands/test_utils.py index 810142d3d9591..3123de71fb42c 100644 --- a/tests/unit_tests/commands/test_utils.py +++ b/tests/unit_tests/commands/test_utils.py @@ -32,37 +32,37 @@ ) from superset.tags.models import ObjectType -OBJECT_TYPES = {ObjectType.chart, ObjectType.chart} +OBJECT_TYPES = {ObjectType.CHART, ObjectType.CHART} MOCK_TAGS = [ Tag( id=1, name="first", - type=TagType.custom, + type=TagType.CUSTOM, ), Tag( id=2, name="second", - type=TagType.custom, + type=TagType.CUSTOM, ), Tag( id=3, name="third", - type=TagType.custom, + type=TagType.CUSTOM, ), Tag( id=4, name="type:dashboard", - type=TagType.type, + type=TagType.TYPE, ), Tag( id=4, name="owner:1", - type=TagType.owner, + type=TagType.OWNER, ), Tag( id=4, name="avorited_by:2", - type=TagType.favorited_by, + type=TagType.FAVORITED_BY, ), ] @@ -256,7 +256,7 @@ def test_validate_tags_no_changes_can_write_on_tag(mock_sm, object_type): Test the ``validate_tags`` method when new_tags is equal to existing tags and user has permission to write on tag. """ - new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] validate_tags(object_type, MOCK_TAGS, new_tags) mock_sm.can_access.assert_not_called() @@ -268,7 +268,7 @@ def test_validate_tags_no_changes_can_tag_on_object(mock_sm, object_type): Test the ``validate_tags`` method when new_tags is equal to existing tags and user has permission to tag objects. """ - new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] validate_tags(object_type, MOCK_TAGS, new_tags) mock_sm.can_access.assert_not_called() @@ -280,7 +280,7 @@ def test_validate_tags_no_changes_missing_permission(mock_sm, object_type): Test the ``validate_tags`` method when new_tags is equal to existing tags the user doens't have the required perms. """ - new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag.id for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] validate_tags(object_type, MOCK_TAGS, new_tags) mock_sm.can_access.assert_not_called() @@ -295,11 +295,11 @@ def test_validate_tags_add_new_tags_can_write_on_tag( Test the ``validate_tags`` method when new_tags are added and user has permission to write on tag. """ - new_tag_ids = [tag.id for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tag_ids = [tag.id for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] new_tag = { "id": 10, "name": "New test tag", - "type": TagType.custom, + "type": TagType.CUSTOM, } new_tag_ids.append(new_tag["id"]) @@ -321,7 +321,7 @@ def test_validate_tags_add_new_tags_can_tag_on_object( Test the ``validate_tags`` method when new_tags are added and user has permission to tag objects. """ - current_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + current_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] new_tag = current_tags.pop() new_tag_ids = [tag.id for tag in current_tags] new_tag_ids.append(new_tag.id) @@ -380,9 +380,9 @@ def test_update_tags_adding_tags(mock_tag_dao, object_type): """ Test the ``update_tags`` method when adding tags. """ - current_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + current_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] new_tag = current_tags.pop() - new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] new_tag_ids = [tag.id for tag in new_tags] mock_tag_dao.find_by_ids.return_value = [new_tag] @@ -402,7 +402,7 @@ def test_update_tags_removing_tags(mock_tag_dao, object_type): """ Test the ``update_tags`` method when removing existing tags. """ - new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] tag_to_be_removed = new_tags.pop() new_tag_ids = [tag.id for tag in new_tags] @@ -420,9 +420,9 @@ def test_update_tags_adding_and_removing_tags(mock_tag_dao, object_type): """ Test the ``update_tags`` method when adding and removing existing tags. """ - new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] tag_to_be_removed = new_tags.pop() - new_tag = Tag(id=10, name="my new tag", type=TagType.custom) + new_tag = Tag(id=10, name="my new tag", type=TagType.CUSTOM) new_tags.append(new_tag) new_tag_ids = [tag.id for tag in new_tags] @@ -451,7 +451,7 @@ def test_update_tags_removing_all_tags(mock_tag_dao, object_type): [ call(object_type, 1, tag.name) for tag in MOCK_TAGS - if tag.type == TagType.custom + if tag.type == TagType.CUSTOM ] ) mock_tag_dao.create_custom_tagged_objects.assert_not_called() @@ -463,8 +463,8 @@ def test_update_tags_no_tags(mock_tag_dao, object_type): """ Test the ``update_tags`` method when the asset only has system tags. """ - system_tags = [tag for tag in MOCK_TAGS if tag.type != TagType.custom] - new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.custom] + system_tags = [tag for tag in MOCK_TAGS if tag.type != TagType.CUSTOM] + new_tags = [tag for tag in MOCK_TAGS if tag.type == TagType.CUSTOM] new_tag_ids = [tag.id for tag in new_tags] new_tag_names = [tag.name for tag in new_tags]