Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified User Route for User Management Redesign #2595

Merged
merged 14 commits into from
Dec 10, 2024
1 change: 1 addition & 0 deletions care/facility/api/viewsets/facility_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class UserFilter(filters.FilterSet):
choices=[(key, key) for key in User.TYPE_VALUE_MAP],
coerce=lambda role: User.TYPE_VALUE_MAP[role],
)
username = filters.CharFilter(field_name="username", lookup_expr="icontains")
vigneshhari marked this conversation as resolved.
Show resolved Hide resolved

class Meta:
model = User
Expand Down
12 changes: 12 additions & 0 deletions care/facility/tests/test_facilityuser_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ def setUpTestData(cls) -> None:
cls.facility2 = cls.create_facility(
cls.super_user, cls.district, cls.local_body
)
cls.user2 = cls.create_user(
"dummystaff", cls.district, home_facility=cls.facility2
)

def setUp(self) -> None:
self.client.force_authenticate(self.super_user)
Expand All @@ -32,6 +35,15 @@ def test_get_queryset_with_prefetching(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertNumQueries(2)

def test_get_queryset_with_search(self):
response = self.client.get(
f"/api/v1/facility/{self.facility2.external_id}/get_users/?username={self.user2.username}"
)

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(len(response.data["results"]), 1)
self.assertEqual(response.data["results"][0]["username"], self.user2.username)
Jacobjeevan marked this conversation as resolved.
Show resolved Hide resolved

def test_link_new_facility(self):
response = self.client.get("/api/v1/facility/")

Expand Down
2 changes: 2 additions & 0 deletions care/users/api/serializers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ class Meta:
"pf_auth",
"read_profile_picture_url",
"user_flags",
"last_login",
Jacobjeevan marked this conversation as resolved.
Show resolved Hide resolved
)
read_only_fields = (
"is_superuser",
Expand All @@ -347,6 +348,7 @@ class Meta:
"pf_endpoint",
"pf_p256dh",
"pf_auth",
"last_login",
)

extra_kwargs = {"url": {"lookup_field": "username"}}
Expand Down
3 changes: 3 additions & 0 deletions care/users/api/viewsets/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ def get_queryset(self):

def get_object(self) -> User:
try:
if self.action == "retrieve":
username = self.kwargs.get("username")
return get_object_or_404(User, username=username)
sainak marked this conversation as resolved.
Show resolved Hide resolved
return super().get_object()
except Http404 as e:
error = "User not found"
Expand Down
2 changes: 1 addition & 1 deletion care/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ def has_read_permission(request):
return True

def has_object_read_permission(self, request):
return request.user.is_superuser or self == request.user
return True

@staticmethod
def has_write_permission(request):
Expand Down
83 changes: 75 additions & 8 deletions care/users/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def setUpTestData(cls) -> None:
cls.user = cls.create_user("staff1", cls.district)
cls.user_data = cls.get_user_data(cls.district, 40)

cls.data_2 = cls.get_user_data(cls.district)
cls.data_2.update({"username": "user_2", "password": "password"})
cls.user_2 = cls.create_user(**cls.data_2)

def setUp(self):
self.client.force_authenticate(self.super_user)

Expand Down Expand Up @@ -51,6 +55,7 @@ def get_detail_representation(self, obj=None) -> dict:
"video_connect_link": obj.video_connect_link,
"read_profile_picture_url": obj.profile_picture_url,
"user_flags": [],
"last_login": obj.last_login,
**self.get_local_body_district_state_representation(obj),
}

Expand All @@ -67,9 +72,11 @@ def test_superuser_can_view(self):
data = self.user_data.copy()
data["date_of_birth"] = str(data["date_of_birth"])
data.pop("password")
user_data = self.get_detail_representation(self.user)
user_data.pop("created_by")
self.assertDictEqual(
res_data_json,
self.get_detail_representation(self.user),
user_data,
)

def test_superuser_can_modify(self):
Expand Down Expand Up @@ -137,10 +144,38 @@ def setUpTestData(cls) -> None:
cls.user_2 = cls.create_user(**cls.data_2)

cls.data_3 = cls.get_user_data(cls.district)
cls.data_3.update({"username": "user_3", "password": "password"})
cls.data_3.update(
{
"username": "user_3",
"password": "password",
"user_type": User.TYPE_VALUE_MAP["Doctor"],
}
)
cls.user_3 = cls.create_user(**cls.data_3)
cls.link_user_with_facility(cls.user_3, cls.facility, cls.super_user)

cls.data_4 = cls.get_user_data(cls.district)
cls.data_4.update(
{
"username": "user_4",
"password": "password",
"user_type": User.TYPE_VALUE_MAP["DistrictAdmin"],
}
)
cls.user_4 = cls.create_user(**cls.data_4)
cls.link_user_with_facility(cls.user_4, cls.facility, cls.super_user)

cls.data_5 = cls.get_user_data(cls.district)
cls.data_5.update(
{
"username": "user_5",
"password": "password",
"user_type": User.TYPE_VALUE_MAP["WardAdmin"],
}
)
cls.user_5 = cls.create_user(**cls.data_5)
cls.link_user_with_facility(cls.user_5, cls.facility, cls.super_user)

def test_user_can_access_url(self):
"""Test user can access the url by location"""
username = self.user.username
Expand All @@ -152,7 +187,7 @@ def test_user_can_read_all_users_within_accessible_facility(self):
response = self.client.get("/api/v1/users/")
self.assertEqual(response.status_code, status.HTTP_200_OK)
res_data_json = response.json()
self.assertEqual(res_data_json["count"], 2)
self.assertEqual(res_data_json["count"], 3)
results = res_data_json["results"]
self.assertIn(self.user.id, {r["id"] for r in results})
self.assertIn(self.user_3.id, {r["id"] for r in results})
Expand All @@ -176,12 +211,12 @@ def test_user_can_modify_themselves(self):
User.objects.get(username=username).date_of_birth, date(2005, 4, 1)
)

def test_user_cannot_read_others(self):
"""Test 1 user can read the attributes of the other user"""
username = self.data_2["username"]
def test_user_can_read_others(self):
"""Test 1 user can read the attributes of any other user"""
username = self.user_2.username
response = self.client.get(f"/api/v1/users/{username}/")
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
self.assertEqual(response.json()["detail"], "User not found")
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["first_name"], self.user_2.first_name)
Jacobjeevan marked this conversation as resolved.
Show resolved Hide resolved

def test_user_cannot_modify_others(self):
"""Test a user can't modify others"""
Expand All @@ -207,6 +242,38 @@ def test_user_cannot_delete_others(self):
User.objects.get(username=self.data_2[field]).username,
)

def test_user_cannot_change_password_of_others(self):
"""Test a user cannot change password of others"""
username = self.data_2["username"]
password = self.data_2["password"]
response = self.client.put(
"/api/v1/password_change/",
{
"username": username,
"old_password": password,
"new_password": "password2",
},
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

Jacobjeevan marked this conversation as resolved.
Show resolved Hide resolved
def test_user_with_districtadmin_access_can_modify_others(self):
"""Test a user with district admin access can modify others underneath the hierarchy"""
self.client.force_authenticate(self.user_4)
username = self.data_2["username"]
response = self.client.patch(
f"/api/v1/users/{username}/",
{
"date_of_birth": date(2005, 4, 1),
},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["date_of_birth"], "2005-04-01")

def test_user_gets_error_when_accessing_user_details_with_invalid_username(self):
"""Test a user gets error when accessing user details with invalid username"""
response = self.client.get("/api/v1/users/foobar/")
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)


class TestUserFilter(TestUtils, APITestCase):
@classmethod
Expand Down
6 changes: 3 additions & 3 deletions care/utils/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def get_local_body_district_state_representation(self, obj):
response.update(self.get_state_representation(getattr(obj, "state", None)))
return response

def get_local_body_representation(self, local_body: LocalBody):
def get_local_body_representation(self, local_body: LocalBody | None):
if local_body is None:
return {"local_body": None, "local_body_object": None}
return {
Expand All @@ -778,7 +778,7 @@ def get_local_body_representation(self, local_body: LocalBody):
},
}

def get_district_representation(self, district: District):
def get_district_representation(self, district: District | None):
if district is None:
return {"district": None, "district_object": None}
return {
Expand All @@ -790,7 +790,7 @@ def get_district_representation(self, district: District):
},
}

def get_state_representation(self, state: State):
def get_state_representation(self, state: State | None):
if state is None:
return {"state": None, "state_object": None}
return {"state": state.id, "state_object": {"id": state.id, "name": state.name}}
Expand Down
Loading