refactor: Tag.update() should return None (#369)

integration/tests/posit/connect/test_tags.py

@@ -151,12 +151,12 @@ def test_tag_content_items(self):
         }
 
         # Update tag
-        tagDName = tagD.update(name="tagD_updated")
-        assert tagDName["name"] == "tagD_updated"
+        tagD.update(name="tagD_updated")
+        assert tagD["name"] == "tagD_updated"
         assert self.client.tags.get(tagD["id"])["name"] == "tagD_updated"
 
-        tagDParent = tagDName.update(parent=tagB)
-        assert tagDParent["parent_id"] == tagB["id"]
+        tagD.update(parent=tagB)
+        assert tagD["parent_id"] == tagB["id"]
         assert self.client.tags.get(tagD["id"])["parent_id"] == tagB["id"]
 
         # Cleanup

src/posit/connect/_utils.py

@@ -3,5 +3,28 @@
 from typing import Any
 
 
-def drop_none(x: dict[str, Any]) -> dict[str, Any]:
-    return {k: v for k, v in x.items() if v is not None}
+def update_dict_values(obj: dict[str, Any], /, **kwargs: Any) -> None:
+    """
+    Update the values of a dictionary.
+
+    This helper method exists as a workaround for the `dict.update` method. Sometimes, `super()` does not return the `dict` class and. If `super().update(**kwargs)` is called unintended behavior will occur.
+
+    Therefore, this helper method exists to update the `dict`'s values.
+
+    Parameters
+    ----------
+    obj : dict[str, Any]
+        The object to update.
+    kwargs : Any
+        The key-value pairs to update the object with.
+
+    See Also
+    --------
+    * https://github.com/posit-dev/posit-sdk-py/pull/366#discussion_r1887845267
+    """
+    # Could also be performed with:
+    # for key, value in kwargs.items():
+    #     obj[key] = value
+
+    # Use the `dict` class to explicity update the object in-place
+    dict.update(obj, **kwargs)

src/posit/connect/content.py

@@ -228,7 +228,7 @@ def restart(self) -> None:
                 f"Restart not supported for this application mode: {self['app_mode']}. Did you need to use the 'render()' method instead? Note that some application modes do not support 'render()' or 'restart()'.",
             )
 
-    def update(  # type: ignore[reportIncompatibleMethodOverride]
+    def update(
         self,
         **attrs: Unpack[ContentItem._Attrs],
     ) -> None:

src/posit/connect/env.py

@@ -131,7 +131,7 @@ def items(self):
             "Since environment variables may contain sensitive information, the values are not accessible outside of Connect.",
         )
 
-    def update(self, other=(), /, **kwargs: Optional[str]):
+    def update(self, other=(), /, **kwargs: Optional[str]) -> None:
         """
         Update environment variables.
 

src/posit/connect/metrics/usage.py

@@ -199,10 +199,7 @@ def find(self, **kwargs) -> List[UsageEvent]:
         for finder in finders:
             instance = finder(self._ctx)
             events.extend(
-                [
-                    UsageEvent.from_event(event)
-                    for event in instance.find(**kwargs)  # type: ignore[attr-defined]
-                ],
+                [UsageEvent.from_event(event) for event in instance.find(**kwargs)],
             )
         return events
 
@@ -252,7 +249,7 @@ def find_one(self, **kwargs) -> UsageEvent | None:
         finders = (visits.Visits, shiny_usage.ShinyUsage)
         for finder in finders:
             instance = finder(self._ctx)
-            event = instance.find_one(**kwargs)  # type: ignore[attr-defined]
+            event = instance.find_one(**kwargs)
             if event:
                 return UsageEvent.from_event(event)
         return None

src/posit/connect/repository.py

@@ -9,19 +9,18 @@
     runtime_checkable,
 )
 
+from ._utils import update_dict_values
 from .errors import ClientError
 from .resources import Resource, _Resource
 
 
 # ContentItem Repository uses a PATCH method, not a PUT for updating.
 class _ContentItemRepository(_Resource):
-    def update(self, **attributes):
+    def update(self, **attributes) -> None:
         response = self._ctx.client.patch(self._path, json=attributes)
         result = response.json()
-        # # Calling this method will call `_Resource.update` which will try to PUT to the path.
-        # super().update(**result)
-        # Instead, update the dict directly.
-        dict.update(self, **result)
+
+        update_dict_values(self, **result)
 
 
 @runtime_checkable

src/posit/connect/resources.py

@@ -85,7 +85,7 @@ def __init__(self, ctx: Context, path: str, **attributes):
     def destroy(self) -> None:
         self._ctx.client.delete(self._path)
 
-    def update(self, **attributes):  # type: ignore[reportIncompatibleMethodOverride]
+    def update(self, **attributes):  # pyright: ignore[reportIncompatibleMethodOverride]
         response = self._ctx.client.put(self._path, json=attributes)
         result = response.json()
         super().update(**result)

src/posit/connect/tags.py

@@ -5,6 +5,7 @@
 
 from typing_extensions import NotRequired, TypedDict, Unpack
 
+from ._utils import update_dict_values
 from .context import Context, ContextManager
 from .resources import Active
 
@@ -162,14 +163,14 @@ def destroy(self) -> None:
 
     # Allow for every combination of `name` and (`parent` or `parent_id`)
     @overload
-    def update(self, /, *, name: str = ..., parent: Tag | None = ...) -> Tag: ...
+    def update(self, /, *, name: str = ..., parent: Tag | None = ...) -> None: ...
     @overload
-    def update(self, /, *, name: str = ..., parent_id: str | None = ...) -> Tag: ...
+    def update(self, /, *, name: str = ..., parent_id: str | None = ...) -> None: ...
 
-    def update(  # pyright: ignore[reportIncompatibleMethodOverride] ; This method returns `Tag`. Parent method returns `None`
+    def update(
         self,
         **kwargs,
-    ) -> Tag:
+    ) -> None:
         """
         Update the tag.
 
@@ -214,7 +215,7 @@ def update(  # pyright: ignore[reportIncompatibleMethodOverride] ; This method r
         updated_kwargs = _update_parent_kwargs(kwargs)
         response = self._ctx.client.patch(self._path, json=updated_kwargs)
         result = response.json()
-        return Tag(self._ctx, self._path, **result)
+        update_dict_values(self, **result)
 
 
 class TagContentItems(ContextManager):

tests/posit/connect/test_packages.py

@@ -2,7 +2,7 @@
 
 from posit.connect.client import Client
 
-from .api import load_mock  # type: ignore
+from .api import load_mock
 
 
 class TestPackagesFindBy:

tests/posit/connect/test_tags.py

@@ -289,20 +289,17 @@ def test_update(self):
         tag33 = client.tags.get("33")
 
         # invoke
-        updated_tag33_0 = tag33.update(name="academy-updated", parent_id=None)
-        updated_tag33_1 = tag33.update(name="academy-updated", parent=None)
+        tag33.update(name="academy-updated", parent_id=None)
+        tag33.update(name="academy-updated", parent=None)
 
         parent_tag = Tag(client._ctx, "/v1/tags/1", id="42", name="Parent")
-        updated_tag33_2 = tag33.update(name="academy-updated", parent=parent_tag)
-        updated_tag33_3 = tag33.update(name="academy-updated", parent_id=parent_tag["id"])
+        tag33.update(name="academy-updated", parent=parent_tag)
+        tag33.update(name="academy-updated", parent_id=parent_tag["id"])
 
         # assert
         assert mock_get_33_tag.call_count == 1
         assert mock_update_33_tag.call_count == 4
 
-        for tag in [updated_tag33_0, updated_tag33_1, updated_tag33_2, updated_tag33_3]:
-            assert isinstance(tag, Tag)
-
         # Asserting updated values are deferred to integration testing
         # to avoid agreening with the mocked data