fix: remove custom delete method for labels

JohnVillalovos · JohnVillalovos · commit 0841a2a686c6 · 2022-02-01T09:53:21.000-08:00
The usage of deleting was incorrect according to the current API. Remove custom `delete()` method as not needed. Add tests to show it works with labels needing to be encoded. Also enable the test_group_labels() test function. Previously it was disabled. Add ability to do a `get()` for group labels. Closes: #1867
diff --git a/gitlab/v4/objects/labels.py b/gitlab/v4/objects/labels.py
@@ -1,11 +1,10 @@
-from typing import Any, cast, Dict, Optional, TYPE_CHECKING, Union
+from typing import Any, cast, Dict, Optional, Union
 
 from gitlab import exceptions as exc
 from gitlab.base import RequiredOptional, RESTManager, RESTObject
 from gitlab.mixins import (
     CreateMixin,
     DeleteMixin,
-    ListMixin,
     ObjectDeleteMixin,
     PromoteMixin,
     RetrieveMixin,
@@ -47,7 +46,9 @@ def save(self, **kwargs: Any) -> None:
         self._update_attrs(server_data)
 
 
-class GroupLabelManager(ListMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTManager):
+class GroupLabelManager(
+    RetrieveMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTManager
+):
     _path = "/groups/{group_id}/labels"
     _obj_cls = GroupLabel
     _from_parent_attrs = {"group_id": "id"}
@@ -58,6 +59,9 @@ class GroupLabelManager(ListMixin, CreateMixin, UpdateMixin, DeleteMixin, RESTMa
         required=("name",), optional=("new_name", "color", "description", "priority")
     )
 
+    def get(self, id: Union[str, int], lazy: bool = False, **kwargs: Any) -> GroupLabel:
+        return cast(GroupLabel, super().get(id=id, lazy=lazy, **kwargs))
+
     # Update without ID.
     # NOTE(jlvillal): Signature doesn't match UpdateMixin.update() so ignore
     # type error
@@ -78,25 +82,6 @@ def update(  # type: ignore
             new_data["name"] = name
         return super().update(id=None, new_data=new_data, **kwargs)
 
-    # Delete without ID.
-    @exc.on_http_error(exc.GitlabDeleteError)
-    # NOTE(jlvillal): Signature doesn't match DeleteMixin.delete() so ignore
-    # type error
-    def delete(self, name: str, **kwargs: Any) -> None:  # type: ignore
-        """Delete a Label on the server.
-
-        Args:
-            name: The name of the label
-            **kwargs: Extra options to send to the server (e.g. sudo)
-
-        Raises:
-            GitlabAuthenticationError: If authentication is not correct
-            GitlabDeleteError: If the server cannot perform the request
-        """
-        if TYPE_CHECKING:
-            assert self.path is not None
-        self.gitlab.http_delete(self.path, query_data={"name": name}, **kwargs)
-
 
 class ProjectLabel(
     PromoteMixin, SubscribableMixin, SaveMixin, ObjectDeleteMixin, RESTObject
@@ -162,22 +147,3 @@ def update(  # type: ignore
         if name:
             new_data["name"] = name
         return super().update(id=None, new_data=new_data, **kwargs)
-
-    # Delete without ID.
-    @exc.on_http_error(exc.GitlabDeleteError)
-    # NOTE(jlvillal): Signature doesn't match DeleteMixin.delete() so ignore
-    # type error
-    def delete(self, name: str, **kwargs: Any) -> None:  # type: ignore
-        """Delete a Label on the server.
-
-        Args:
-            name: The name of the label
-            **kwargs: Extra options to send to the server (e.g. sudo)
-
-        Raises:
-            GitlabAuthenticationError: If authentication is not correct
-            GitlabDeleteError: If the server cannot perform the request
-        """
-        if TYPE_CHECKING:
-            assert self.path is not None
-        self.gitlab.http_delete(self.path, query_data={"name": name}, **kwargs)
diff --git a/tests/functional/api/test_groups.py b/tests/functional/api/test_groups.py
@@ -104,7 +104,6 @@ def test_groups(gl):
     group2.members.delete(gl.user.id)
 
 
-@pytest.mark.skip(reason="Commented out in legacy test")
 def test_group_labels(group):
     group.labels.create({"name": "foo", "description": "bar", "color": "#112233"})
     label = group.labels.get("foo")
@@ -116,6 +115,12 @@ def test_group_labels(group):
     assert label.description == "baz"
     assert len(group.labels.list()) == 1
 
+    label.new_name = "Label:that requires:encoding"
+    label.save()
+    assert label.name == "Label:that requires:encoding"
+    label = group.labels.get("Label:that requires:encoding")
+    assert label.name == "Label:that requires:encoding"
+
     label.delete()
     assert len(group.labels.list()) == 0
 
diff --git a/tests/functional/api/test_projects.py b/tests/functional/api/test_projects.py
@@ -146,9 +146,11 @@ def test_project_labels(project):
     label = project.labels.get("label")
     assert label == labels[0]
 
-    label.new_name = "labelupdated"
+    label.new_name = "Label:that requires:encoding"
     label.save()
-    assert label.name == "labelupdated"
+    assert label.name == "Label:that requires:encoding"
+    label = project.labels.get("Label:that requires:encoding")
+    assert label.name == "Label:that requires:encoding"
 
     label.subscribe()
     assert label.subscribed is True
