Merge pull request #90 from zest-derm/samm/parent-accessors

Unify parent accessor methods

pyproject.toml

@@ -20,6 +20,7 @@ classifiers = [
 python = "^3.7"
 requests = "^2.28.1"
 portalocker = "^2.7.0"
+inflection = "^0.5.1"
 
 [tool.poetry.group.dev]
 optional = true

test/test_base.py

@@ -1,20 +1,45 @@
 import inspect
 
+import pytest
+
+from welkin.models import Patient, __all__
 from welkin.models.base import Collection
 from welkin.pagination import PageIterator
 
 
-def test_collection_pageable(client):
-    for v in vars(client).values():
-        try:
-            if not issubclass(v, Collection):
-                continue
-        except TypeError:
-            continue
+@pytest.mark.parametrize("class_name", __all__)
+def test_collection_pageable(client, class_name: str):
+    cls = getattr(client, class_name)
+    if not issubclass(cls, Collection):
+        return
+
+    assert hasattr(cls, "get"), f"{cls} has no get method"
+    assert issubclass(cls.iterator, PageIterator)
+
+    method = cls.get
+    args = inspect.getfullargspec(method)
+    assert args.varargs is not None, f"{method} must accept variable args"
+    assert args.varkw is not None, f"{method} must accept variable kwargs"
+
+
+@pytest.mark.parametrize("class_name", __all__)
+def test_method_args(client, class_name: str):
+    cls = getattr(client, class_name)
+
+    for method_name, method in inspect.getmembers(cls, predicate=inspect.isfunction):
+        if method_name.startswith("__"):
+            continue  # skip dunder methods
+
+        if hasattr(method, "__wrapped__"):
+            method = method.__wrapped__  # unwrap decorated functions
+
+        if method.__qualname__ != f"{class_name}.{method_name}":
+            continue  # skip methods from parent classes
 
-        assert hasattr(v, "get"), f"{v} has no get method"
-        assert issubclass(v.iterator, PageIterator)
+        args = inspect.getfullargspec(method)
 
-        args = inspect.getfullargspec(v.get)
-        assert args.varargs is not None, f"{v}.get must accept variable args"
-        assert args.varkw is not None, f"{v}.get must accept variable kwargs"
+        if cls in Patient.subresources:
+            try:
+                assert args.args.index("patient_id") == 1, f"patient_id must be first"
+            except ValueError:
+                pytest.fail(f"{method} must accept patient_id")

test/test_util.py

@@ -9,6 +9,9 @@
     clean_json_list,
     clean_request_params,
     clean_request_payload,
+    find_model_id,
+    to_camel_case,
+    to_snake_case,
 )
 
 
@@ -114,3 +117,49 @@ def test_clean_date(base_date, base_date_str):
 def test_clean_datetime(dt, expected, request):
     cleaned = clean_datetime(request.getfixturevalue(dt))
     assert cleaned == request.getfixturevalue(expected)
+
+
+@pytest.mark.parametrize(
+    "input",
+    [
+        "foo_bar",
+        "fooBar",
+        "FooBar",
+        "FOO_BAR",
+        "fooBAR",
+        "FOOBar",
+    ],
+)
+def test_case_converters(input):
+    assert to_camel_case(input) == "fooBar"
+    assert to_snake_case(input) == "foo_bar"
+
+
+def test_find_model_id(client):
+    patient = client.Patient()
+    encounter = patient.Encounter()
+
+    with pytest.raises(AttributeError):
+        find_model_id(encounter, "Patient")
+
+    patient.id = "123"
+    assert find_model_id(patient, "Patient") == patient.id
+    assert find_model_id(encounter, "Patient") == patient.id
+
+    del patient.id
+    encounter.patientId = "456"
+    assert find_model_id(encounter, "Patient") == encounter.patientId
+
+
+def test_model_id(client):
+    encounter = client.Patient().Encounter()
+    with pytest.raises(TypeError) as exc_info:
+        encounter.get()
+
+    assert "missing 1 required positional argument" in exc_info.value.args[0]
+
+    disposition = encounter.EncounterDisposition()
+    with pytest.raises(TypeError) as exc_info:
+        disposition.get()
+
+    assert "missing 2 required positional arguments" in exc_info.value.args[0]

welkin/models/assessment.py

@@ -1,43 +1,33 @@
-from sys import modules
-
 from welkin.models.base import Collection, Resource
-from welkin.models.util import EncounterSubResource, patient_id
 from welkin.pagination import PageableIterator
+from welkin.util import model_id
 
 
-class Assessment(Resource, EncounterSubResource):
-    def create(self, patient_id: str = None, encounter_id: str = None):
-        patient_id, encounter_id = self.get_patient_encounter_id(
-            patient_id, encounter_id
-        )
+class Assessment(Resource):
+    @model_id("Patient", "Encounter")
+    def create(self, patient_id: str, encounter_id: str):
         return super().post(
             f"{self._client.instance}/patients/{patient_id}/encounters/{encounter_id}"
             "/assessments"
         )
 
-    def get(self, patient_id: str = None, encounter_id: str = None):
-        patient_id, encounter_id = self.get_patient_encounter_id(
-            patient_id, encounter_id
-        )
+    @model_id("Patient", "Encounter")
+    def get(self, patient_id: str, encounter_id: str):
         return super().get(
             f"{self._client.instance}/patients/{patient_id}/encounters/{encounter_id}/"
             f"assessments/{self.id}"
         )
 
-    def update(self, patient_id: str = None, encounter_id: str = None, **kwargs):
-        patient_id, encounter_id = self.get_patient_encounter_id(
-            patient_id, encounter_id
-        )
+    @model_id("Patient", "Encounter")
+    def update(self, patient_id: str, encounter_id: str, **kwargs):
         return super().patch(
             f"{self._client.instance}/patients/{patient_id}/encounters/{encounter_id}/"
             f"assessments/{self.id}",
             kwargs,
         )
 
-    def delete(self, patient_id: str = None, encounter_id: str = None):
-        patient_id, encounter_id = self.get_patient_encounter_id(
-            patient_id, encounter_id
-        )
+    @model_id("Patient", "Encounter")
+    def delete(self, patient_id: str, encounter_id: str):
         return super().delete(
             f"{self._client.instance}/patients/{patient_id}/encounters/{encounter_id}/"
             f"assessments/{self.id}"
@@ -48,34 +38,19 @@ class Assessments(Collection):
     resource = Assessment
     iterator = PageableIterator
 
-    def get(self, patient_id: str = None, encounter_id: str = None, *args, **kwargs):
-
-        root = f"{self._client.instance}/patients/"
-        if self._parent:
-            encounter_id = self._parent.id
-            if isinstance(
-                self._parent._parent, getattr(modules["welkin.models"], "Patient")
-            ):
-                patient_id = self._parent._parent.id
-            elif hasattr(self._parent, "patientId"):
-                patient_id = self._parent.patientId
-            else:
-                # this is the related_data = True case on encounters
-                patient_id = self._parent.encounter.patientId
-
-        path = f"{patient_id}/encounters/{encounter_id}/assessments"
-
-        return super().get(f"{root}{path}", *args, **kwargs)
+    @model_id("Patient", "Encounter")
+    def get(self, patient_id: str, encounter_id: str, *args, **kwargs):
+        return super().get(
+            f"{self._client.instance}/patients/{patient_id}/encounters/{encounter_id}/"
+            f"assessments",
+            *args,
+            **kwargs,
+        )
 
 
 class AssessmentRecordAnswers(Resource):
-    def update(self, patient_id: str = None, assessment_record_id: str = None):
-        if not assessment_record_id:
-            assessment_record_id = self._parent.id
-
-        if not patient_id:
-            patient_id = self._parent.get_patient_id(patient_id)
-
+    @model_id("Patient", "AssessmentRecord")
+    def update(self, patient_id: str, assessment_record_id: str):
         return super().put(
             f"{self._client.instance}/patients/{patient_id}/"
             f"assessment-records/{assessment_record_id}/answers"
@@ -85,43 +60,41 @@ def update(self, patient_id: str = None, assessment_record_id: str = None):
 class AssessmentRecord(Resource):
     subresources = [AssessmentRecordAnswers]
 
-    @patient_id
-    def create(self, patient_id: str = None):
+    @model_id("Patient")
+    def create(self, patient_id: str):
         return super().post(
             f"{self._client.instance}/patients/{patient_id}/assessment-records"
         )
 
-    @patient_id
-    def get(self, patient_id: str = None):
+    @model_id("Patient")
+    def get(self, patient_id: str):
         return super().get(
             f"{self._client.instance}/patients/"
             f"{patient_id}/assessment-records/{self.id}"
         )
 
-    @patient_id
-    def update(self, patient_id: str = None):
+    @model_id("Patient")
+    def update(self, patient_id: str):
         return super().put(
             f"{self._client.instance}/patients/"
             f"{patient_id}/assessment-records/{self.id}"
         )
 
-    @patient_id
-    def delete(self, patient_id: str = None):
+    @model_id("Patient")
+    def delete(self, patient_id: str):
         return super().delete(
             f"{self._client.instance}/patients/"
             f"{patient_id}/assessment-records/{self.id}",
         )
 
-    def get_patient_id(self, patient_id):
-        return patient_id if patient_id else self._parent.id
-
 
 class AssessmentRecords(Collection):
     resource = AssessmentRecord
     iterator = PageableIterator
 
-    @patient_id
-    def get(self, patient_id: str = None, **kwargs):
-        path = f"{self._client.instance}/patients/{patient_id}/assessment-records"
-
-        return super().get(path, **kwargs)
+    @model_id("Patient")
+    def get(self, patient_id: str, **kwargs):
+        return super().get(
+            f"{self._client.instance}/patients/{patient_id}/assessment-records",
+            **kwargs,
+        )

welkin/models/base.py

@@ -45,6 +45,9 @@ def __getattr__(self, name):
     def __setattr__(self, name, value):
         super().__setitem__(name, value)
 
+    def __delattr__(self, name):
+        super().__delitem__(name)
+
     def __str__(self):
         id = getattr(self, "id", "")
 

welkin/models/care_plan.py

@@ -1,13 +1,10 @@
 from welkin.models.base import Resource
-from welkin.models.util import find_patient_id_in_parents
+from welkin.util import model_id
 
 
 class CarePlanOverview(Resource):
-    def update(self, patient_id: str = None):
-        if not patient_id:
-            # self._parent -> CarePlan
-            # CarePlan._parent -> Patient
-            patient_id = find_patient_id_in_parents(self)
+    @model_id("Patient")
+    def update(self, patient_id: str):
         return super().put(
             f"{self._client.instance}/patients/{patient_id}/care-plan/overview"
         )
@@ -16,16 +13,12 @@ def update(self, patient_id: str = None):
 class CarePlan(Resource):
     subresources = [CarePlanOverview]
 
-    def create(self, patient_id: str = None):
-        if not patient_id:
-            # self._parent -> Patient
-            patient_id = find_patient_id_in_parents(self)
+    @model_id("Patient")
+    def create(self, patient_id: str):
         return super().post(
             f"{self._client.instance}/patients/{patient_id}/care-plan/overview"
         )
 
-    def get(self, patient_id: str = None):
-        if not patient_id:
-            # self._parent -> Patient
-            patient_id = find_patient_id_in_parents(self)
+    @model_id("Patient")
+    def get(self, patient_id: str):
         return super().get(f"{self._client.instance}/patients/{patient_id}/care-plan")