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

Merge Develop to Staging v24.51.0 #2648

Merged
merged 3 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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")

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)

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",
)
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)
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)

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)

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
5 changes: 3 additions & 2 deletions config/settings/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
# django-silk
# ------------------------------------------------------------------------------
# https://github.com/jazzband/django-silk#requirements
INSTALLED_APPS += ["silk"]
MIDDLEWARE += ["silk.middleware.SilkyMiddleware"]
if env("ENABLE_SILK", default=False):
INSTALLED_APPS += ["silk"]
MIDDLEWARE += ["silk.middleware.SilkyMiddleware"]
# https://github.com/jazzband/django-silk#profiling
SILKY_PYTHON_PROFILER = True

Expand Down
3 changes: 2 additions & 1 deletion docker/.local.env
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ DATABASE_URL=postgres://postgres:postgres@db:5432/care
REDIS_URL=redis://redis:6379
CELERY_BROKER_URL=redis://redis:6379/0

DJANGO_DEBUG=False
DJANGO_DEBUG=true
ATTACH_DEBUGGER=false

BUCKET_REGION=ap-south-1
BUCKET_KEY=key
Expand Down
4 changes: 2 additions & 2 deletions docker/dev.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ RUN ARCH=$(dpkg --print-architecture) && \
rm -rf typst.tar.xz typst-${TYPST_ARCH}

# use pipenv to manage virtualenv
RUN pip install pipenv==2024.4.0

RUN python -m venv /.venv
RUN --mount=type=cache,target=/root/.cache/pip pip install pipenv==2024.4.0

COPY Pipfile Pipfile.lock $APP_HOME/
RUN --mount=type=cache,target=/root/.cache/pip pipenv install --system --categories "packages dev-packages docs"

Expand Down
6 changes: 3 additions & 3 deletions scripts/start-dev.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ python manage.py collectstatic --noinput
python manage.py compilemessages -v 0

echo "starting server..."
if [[ "${DJANGO_DEBUG,,}" == "true" ]]; then
if [[ "${ATTACH_DEBUGGER}" == "true" ]]; then
echo "waiting for debugger..."
python -m debugpy --wait-for-client --listen 0.0.0.0:9876 manage.py runserver_plus 0.0.0.0:9000
python -m debugpy --wait-for-client --listen 0.0.0.0:9876 manage.py runserver_plus 0.0.0.0:9000 --print-sql
else
python manage.py runserver 0.0.0.0:9000
python manage.py runserver_plus 0.0.0.0:9000 --print-sql
fi
Loading