Skip to content

Commit 13a4be6

Browse files
committed
feat: Allow user defaluting on the course-archive-status-detail api.
If the client does not send a user id on create, default to the current user for the creating of an archive status row. Add relevant tests and also update some formatting issue(wish I had done that separately but I can't easly undo it.)
1 parent 25c626a commit 13a4be6

File tree

6 files changed

+219
-27
lines changed

6 files changed

+219
-27
lines changed

backend/sample_plugin/serializers.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,25 @@
22
Serializers for the sample_plugin app.
33
"""
44

5+
from django.contrib.auth import get_user_model
56
from rest_framework import serializers
67

78
from sample_plugin.models import CourseArchiveStatus
89

10+
User = get_user_model()
11+
912

1013
class CourseArchiveStatusSerializer(serializers.ModelSerializer):
1114
"""
1215
Serializer for the CourseArchiveStatus model.
1316
"""
1417

18+
user = serializers.PrimaryKeyRelatedField(
19+
queryset=User.objects.all(),
20+
default=serializers.CurrentUserDefault(),
21+
required=False,
22+
)
23+
1524
class Meta:
1625
"""
1726
Meta class for CourseArchiveStatusSerializer.

backend/sample_plugin/settings/production.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Production settings for the sample_plugin application.
33
"""
4+
45
from sample_plugin.settings.common import plugin_settings as common_settings
56

67

backend/sample_plugin/settings/test.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Test settings for the sample_plugin application.
33
"""
4+
45
from sample_plugin.settings.common import plugin_settings as common_settings
56

67

backend/sample_plugin/views.py

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ class CourseArchiveStatusViewSet(viewsets.ModelViewSet):
7777
serializer_class = CourseArchiveStatusSerializer
7878
permission_classes = [IsOwnerOrStaffSuperuser]
7979
pagination_class = CourseArchiveStatusPagination
80-
throttle_classes = [CourseArchiveStatusThrottle, ]
80+
throttle_classes = [
81+
CourseArchiveStatusThrottle,
82+
]
8183
filter_backends = [DjangoFilterBackend, filters.OrderingFilter]
8284
filterset_fields = ["course_id", "user", "is_archived"]
8385
ordering_fields = [
@@ -142,29 +144,26 @@ def perform_create(self, serializer):
142144
"""
143145
Perform creation of a new CourseArchiveStatus.
144146
145-
Sets the user to the requesting user if not specified.
146-
Sets archive_date and archived_by if is_archived is True.
147-
"""
148-
data = serializer.validated_data.copy()
149-
150-
# Set user to requesting user if not specified
151-
if "user" not in data:
152-
data["user"] = self.request.user
153-
# Only allow staff/superusers to create records for other users
154-
elif data["user"] != self.request.user and not (
155-
self.request.user.is_staff or self.request.user.is_superuser
156-
):
157-
logger.warning(
158-
"Permission denied: User %s tried to create a record for user %s",
159-
self.request.user.username,
160-
data["user"].username,
161-
)
162-
raise PermissionDenied(
163-
"You do not have permission to create records for other users."
164-
)
147+
Validates permission for user override and sets archive_date if needed.
148+
"""
149+
# Check if user was explicitly provided and differs from current user
150+
if "user" in self.request.data:
151+
requested_user_id = self.request.data["user"]
152+
if requested_user_id != self.request.user.id and not (
153+
self.request.user.is_staff or self.request.user.is_superuser
154+
):
155+
logger.warning(
156+
"Permission denied: User %s tried to create a record for user %s",
157+
self.request.user.username,
158+
requested_user_id,
159+
)
160+
raise PermissionDenied(
161+
"You do not have permission to create records for other users."
162+
)
165163

166164
# Set archive_date if is_archived is True
167-
if data.get("is_archived", False):
165+
data = {}
166+
if serializer.validated_data.get("is_archived", False):
168167
data["archive_date"] = timezone.now()
169168

170169
# Create the record
@@ -184,18 +183,33 @@ def perform_update(self, serializer):
184183
"""
185184
Perform update of an existing CourseArchiveStatus.
186185
187-
Updates archive_date and archived_by if is_archived changes.
186+
Validates permission for user override and updates archive_date if needed.
188187
"""
189188
instance = serializer.instance
190-
data = serializer.validated_data.copy()
189+
190+
# Check if user was explicitly provided and differs from current user
191+
if "user" in self.request.data:
192+
requested_user_id = self.request.data["user"]
193+
if requested_user_id != self.request.user.id and not (
194+
self.request.user.is_staff or self.request.user.is_superuser
195+
):
196+
logger.warning(
197+
"Permission denied: User %s tried to update a record for user %s",
198+
self.request.user.username,
199+
requested_user_id,
200+
)
201+
raise PermissionDenied(
202+
"You do not have permission to update records for other users."
203+
)
191204

192205
# Handle archive_date if is_archived changes
193-
if "is_archived" in data:
206+
data = {}
207+
if "is_archived" in serializer.validated_data:
194208
# If changing from not archived to archived
195-
if data["is_archived"] and not instance.is_archived:
209+
if serializer.validated_data["is_archived"] and not instance.is_archived:
196210
data["archive_date"] = timezone.now()
197211
# If changing from archived to not archived
198-
elif not data["is_archived"] and instance.is_archived:
212+
elif not serializer.validated_data["is_archived"] and instance.is_archived:
199213
data["archive_date"] = None
200214

201215
# Update the record

backend/tests/__init__.py

Whitespace-only changes.

backend/tests/test_api.py

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,170 @@ def test_staff_can_update_other_user_course_archive_status(
277277

278278
assert response.status_code == status.HTTP_200_OK
279279
assert response.data["is_archived"] is True
280+
281+
282+
# New tests for optional user field behavior
283+
@pytest.mark.django_db
284+
def test_create_course_archive_status_without_user_field(api_client, user, course_key):
285+
"""
286+
Test that a user can create a course archive status without specifying user field.
287+
The user field should default to the current user.
288+
"""
289+
api_client.force_authenticate(user=user)
290+
url = reverse("sample_plugin:course-archive-status-list")
291+
data = {
292+
"course_id": str(course_key),
293+
"is_archived": True,
294+
}
295+
# Note: No "user" field in data
296+
response = api_client.post(url, data, format="json")
297+
298+
if response.status_code != status.HTTP_201_CREATED:
299+
print(f"Response status: {response.status_code}")
300+
print(f"Response data: {response.data}")
301+
302+
assert response.status_code == status.HTTP_201_CREATED
303+
assert response.data["course_id"] == str(course_key)
304+
assert response.data["user"] == user.id
305+
assert response.data["is_archived"] is True
306+
assert response.data["archive_date"] is not None
307+
308+
# Verify in database
309+
course_archive_status = CourseArchiveStatus.objects.get(
310+
course_id=course_key, user=user
311+
)
312+
assert course_archive_status.is_archived is True
313+
assert course_archive_status.user == user
314+
315+
316+
@pytest.mark.django_db
317+
def test_update_course_archive_status_without_user_field(api_client, user, course_archive_status):
318+
"""
319+
Test that a user can update their course archive status without specifying user field.
320+
The user field should remain unchanged.
321+
"""
322+
api_client.force_authenticate(user=user)
323+
url = reverse(
324+
"sample_plugin:course-archive-status-detail", args=[course_archive_status.id]
325+
)
326+
data = {"is_archived": True}
327+
# Note: No "user" field in data
328+
response = api_client.patch(url, data, format="json")
329+
330+
assert response.status_code == status.HTTP_200_OK
331+
assert response.data["is_archived"] is True
332+
assert response.data["user"] == user.id
333+
assert response.data["archive_date"] is not None
334+
335+
# Verify in database
336+
course_archive_status.refresh_from_db()
337+
assert course_archive_status.is_archived is True
338+
assert course_archive_status.user == user
339+
340+
341+
@pytest.mark.django_db
342+
def test_staff_create_with_explicit_user_override(
343+
api_client, staff_user, user, course_key
344+
):
345+
"""
346+
Test that staff can explicitly set user field to override default behavior.
347+
"""
348+
api_client.force_authenticate(user=staff_user)
349+
url = reverse("sample_plugin:course-archive-status-list")
350+
data = {
351+
"course_id": str(course_key),
352+
"user": user.id,
353+
"is_archived": True,
354+
}
355+
response = api_client.post(url, data, format="json")
356+
357+
assert response.status_code == status.HTTP_201_CREATED
358+
assert response.data["course_id"] == str(course_key)
359+
assert response.data["user"] == user.id # Should be the specified user, not staff_user
360+
assert response.data["is_archived"] is True
361+
362+
# Verify in database
363+
course_archive_status = CourseArchiveStatus.objects.get(
364+
course_id=course_key, user=user
365+
)
366+
assert course_archive_status.user == user
367+
assert course_archive_status.user != staff_user
368+
369+
370+
@pytest.mark.django_db
371+
def test_staff_update_with_explicit_user_override(
372+
api_client, staff_user, user, another_user, course_key
373+
):
374+
"""
375+
Test that staff can explicitly change user field when updating.
376+
"""
377+
# Create initial record for user
378+
initial_status = CourseArchiveStatus.objects.create(
379+
course_id=course_key, user=user, is_archived=False
380+
)
381+
382+
api_client.force_authenticate(user=staff_user)
383+
url = reverse(
384+
"sample_plugin:course-archive-status-detail", args=[initial_status.id]
385+
)
386+
data = {
387+
"user": another_user.id,
388+
"is_archived": True,
389+
}
390+
response = api_client.patch(url, data, format="json")
391+
392+
assert response.status_code == status.HTTP_200_OK
393+
assert response.data["user"] == another_user.id # Should be changed to another_user
394+
assert response.data["is_archived"] is True
395+
396+
# Verify in database
397+
initial_status.refresh_from_db()
398+
assert initial_status.user == another_user
399+
assert initial_status.is_archived is True
400+
401+
402+
@pytest.mark.django_db
403+
def test_regular_user_cannot_override_user_field_create(
404+
api_client, user, another_user, course_key
405+
):
406+
"""
407+
Test that regular users cannot override user field to create records for other users.
408+
"""
409+
api_client.force_authenticate(user=user)
410+
url = reverse("sample_plugin:course-archive-status-list")
411+
data = {
412+
"course_id": str(course_key),
413+
"user": another_user.id, # Try to create for another user
414+
"is_archived": True,
415+
}
416+
response = api_client.post(url, data, format="json")
417+
418+
assert response.status_code == status.HTTP_403_FORBIDDEN
419+
420+
421+
@pytest.mark.django_db
422+
def test_staff_create_without_user_field_defaults_to_current_user(
423+
api_client, staff_user, course_key
424+
):
425+
"""
426+
Test that even staff users get records created for themselves when no user specified.
427+
"""
428+
api_client.force_authenticate(user=staff_user)
429+
url = reverse("sample_plugin:course-archive-status-list")
430+
data = {
431+
"course_id": str(course_key),
432+
"is_archived": True,
433+
}
434+
# Note: No "user" field in data
435+
response = api_client.post(url, data, format="json")
436+
437+
assert response.status_code == status.HTTP_201_CREATED
438+
assert response.data["course_id"] == str(course_key)
439+
assert response.data["user"] == staff_user.id # Should default to current user (staff)
440+
assert response.data["is_archived"] is True
441+
442+
# Verify in database
443+
course_archive_status = CourseArchiveStatus.objects.get(
444+
course_id=course_key, user=staff_user
445+
)
446+
assert course_archive_status.user == staff_user

0 commit comments

Comments
 (0)