From 07c9310cd82b7f588617583710b56267ae039a4d Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Mon, 29 Jul 2024 20:45:54 -0700 Subject: [PATCH 1/7] implemented backend logic for retrieving and updating a user profile --- csm_web/frontend/src/components/App.tsx | 3 + csm_web/frontend/src/utils/queries/base.tsx | 2 + ...er_bio_user_pronouns_user_pronunciation.py | 27 +++++ csm_web/scheduler/models.py | 18 +-- csm_web/scheduler/serializers.py | 11 +- csm_web/scheduler/urls.py | 4 +- csm_web/scheduler/views/__init__.py | 2 +- csm_web/scheduler/views/user.py | 107 ++++++++++++++++-- docker-compose.yml | 4 + 9 files changed, 158 insertions(+), 20 deletions(-) create mode 100644 csm_web/scheduler/migrations/0033_user_bio_user_pronouns_user_pronunciation.py diff --git a/csm_web/frontend/src/components/App.tsx b/csm_web/frontend/src/components/App.tsx index 841364f2..51a2b012 100644 --- a/csm_web/frontend/src/components/App.tsx +++ b/csm_web/frontend/src/components/App.tsx @@ -41,6 +41,9 @@ const App = () => { } /> } /> } /> + { + // TODO: add route for profiles (/user/:id/* element = {UserProfile}) + } } /> diff --git a/csm_web/frontend/src/utils/queries/base.tsx b/csm_web/frontend/src/utils/queries/base.tsx index a9dd33ce..39dda134 100644 --- a/csm_web/frontend/src/utils/queries/base.tsx +++ b/csm_web/frontend/src/utils/queries/base.tsx @@ -35,6 +35,7 @@ export const useProfiles = (): UseQueryResult => { return queryResult; }; +// TODO: move to new queries/users.tsx /** * Hook to get the user's info. */ @@ -57,6 +58,7 @@ export const useUserInfo = (): UseQueryResult => { return queryResult; }; +// TODO: move to new queries/users.tsx /** * Hook to get a list of all user emails. */ diff --git a/csm_web/scheduler/migrations/0033_user_bio_user_pronouns_user_pronunciation.py b/csm_web/scheduler/migrations/0033_user_bio_user_pronouns_user_pronunciation.py new file mode 100644 index 00000000..c3546eac --- /dev/null +++ b/csm_web/scheduler/migrations/0033_user_bio_user_pronouns_user_pronunciation.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.7 on 2024-07-20 05:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("scheduler", "0032_word_of_the_day"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="bio", + field=models.CharField(default="", max_length=500), + ), + migrations.AddField( + model_name="user", + name="pronouns", + field=models.CharField(default="", max_length=20), + ), + migrations.AddField( + model_name="user", + name="pronunciation", + field=models.CharField(default="", max_length=50), + ), + ] diff --git a/csm_web/scheduler/models.py b/csm_web/scheduler/models.py index 9e367ab6..9ac5cb71 100644 --- a/csm_web/scheduler/models.py +++ b/csm_web/scheduler/models.py @@ -51,6 +51,12 @@ def week_bounds(date): class User(AbstractUser): priority_enrollment = models.DateTimeField(null=True, blank=True) + pronouns = models.CharField(max_length=20, default="", blank=True) + pronunciation = models.CharField(max_length=50, default="", blank=True) + # profile_pic = models.ImageField(upload_to='profiles/', null=True, blank=True) + # if profile picture is implemented + bio = models.CharField(max_length=500, default="", blank=True) + def can_enroll_in_course(self, course, bypass_enrollment_time=False): """Determine whether this user is allowed to enroll in the given course.""" # check restricted first @@ -260,19 +266,15 @@ def save(self, *args, **kwargs): ): if settings.DJANGO_ENV != settings.DEVELOPMENT: logger.info( - ( - " SO automatically created for student" - " %s in course %s for date %s" - ), + " SO automatically created for student" + " %s in course %s for date %s", self.user.email, course.name, now.date(), ) logger.info( - ( - " Attendance automatically created for student" - " %s in course %s for date %s" - ), + " Attendance automatically created for student" + " %s in course %s for date %s", self.user.email, course.name, now.date(), diff --git a/csm_web/scheduler/serializers.py b/csm_web/scheduler/serializers.py index cd31f741..0ebdd045 100644 --- a/csm_web/scheduler/serializers.py +++ b/csm_web/scheduler/serializers.py @@ -167,7 +167,16 @@ class Meta: class UserSerializer(serializers.ModelSerializer): class Meta: model = User - fields = ("id", "email", "first_name", "last_name", "priority_enrollment") + fields = ( + "id", + "email", + "first_name", + "last_name", + "priority_enrollment", + "bio", + "pronunciation", + "pronouns", + ) class ProfileSerializer(serializers.Serializer): # pylint: disable=abstract-method diff --git a/csm_web/scheduler/urls.py b/csm_web/scheduler/urls.py index 8c60cd33..1df15e71 100644 --- a/csm_web/scheduler/urls.py +++ b/csm_web/scheduler/urls.py @@ -15,7 +15,9 @@ urlpatterns = router.urls urlpatterns += [ - path("userinfo/", views.userinfo, name="userinfo"), + path("user_info/", views.user_info, name="user_info"), + path("user//", views.user_retrieve, name="user_retrieve"), + path("user//update/", views.user_update, name="user_update"), path("matcher/active/", views.matcher.active), path("matcher//slots/", views.matcher.slots), path("matcher//preferences/", views.matcher.preferences), diff --git a/csm_web/scheduler/views/__init__.py b/csm_web/scheduler/views/__init__.py index 55ed65f3..e59b9594 100644 --- a/csm_web/scheduler/views/__init__.py +++ b/csm_web/scheduler/views/__init__.py @@ -6,4 +6,4 @@ from .section import SectionViewSet from .spacetime import SpacetimeViewSet from .student import StudentViewSet -from .user import UserViewSet, userinfo +from .user import UserViewSet, user_info, user_retrieve, user_update diff --git a/csm_web/scheduler/views/user.py b/csm_web/scheduler/views/user.py index aeef9440..14372bb6 100644 --- a/csm_web/scheduler/views/user.py +++ b/csm_web/scheduler/views/user.py @@ -1,11 +1,13 @@ -from rest_framework.exceptions import PermissionDenied -from rest_framework.response import Response from rest_framework import status from rest_framework.decorators import api_view +from rest_framework.exceptions import PermissionDenied +from rest_framework.response import Response +from scheduler.serializers import UserSerializer +from ..models import Coordinator, Mentor, Student, User from .utils import viewset_with -from ..models import Coordinator, User -from scheduler.serializers import UserSerializer + +# can create pytest to test this class UserViewSet(*viewset_with("list")): @@ -13,9 +15,12 @@ class UserViewSet(*viewset_with("list")): queryset = User.objects.all() def list(self, request): + """ + Lists the emails of all users in the system. Only accessible by coordinators and superusers. + """ if not ( - request.user.is_superuser - or Coordinator.objects.filter(user=request.user).exists() + # request.user.is_superuser or + Coordinator.objects.filter(user=request.user).exists() ): raise PermissionDenied( "Only coordinators and superusers may view the user email list" @@ -23,12 +28,96 @@ def list(self, request): return Response(self.queryset.order_by("email").values_list("email", flat=True)) +def has_permission(request_user, target_user): + """ + Returns True if the user has permission to access or edit the target user's profile + """ + # Does not account for users who are both mentors and students of different courses + # need separation of concerns: + # coordinators/mentor should only have coordinator/mentor access for their course + # Check if request_user is a mentor + if Mentor.objects.filter(user=request_user).exists(): + mentor_courses = Mentor.objects.filter(user=request_user).values_list( + "course", flat=True + ) + + if Student.objects.filter(user=target_user, course__in=mentor_courses).exists(): + return True + if Mentor.objects.filter(user=target_user, course__in=mentor_courses).exists(): + return True + + # Check if request_user is a student in the same course as target_user + # Students in the same section can see each other + if ( + Student.objects.filter(user=request_user).exists() + and Student.objects.filter(user=target_user).exists() + ): + request_user_courses = Student.objects.filter(user=request_user).values_list( + "course", flat=True + ) + target_user_courses = Student.objects.filter(user=target_user).values_list( + "course", flat=True + ) + + if set(request_user_courses) & set(target_user_courses): + return True + + # Coordinator access + if Coordinator.objects.filter( + user=request_user + ).exists(): # or if request_user.is_superuser + return True + + # Request user accessing their own profile + if request_user == target_user: + return True + + return False + + @api_view(["GET"]) -def userinfo(request): +def user_retrieve(request, pk): """ - Get user info for request user + Retrieve user profile. Only accessible by superusers and the user themselves. + """ + try: + user = User.objects.get(pk=pk) + except User.DoesNotExist: + return Response({"detail": "Not found."}, status=status.HTTP_404_NOT_FOUND) + + if not has_permission(request.user, user): + raise PermissionDenied("You do not have permission to access this profile") - TODO: perhaps replace this with a viewset when we establish profiles + serializer = UserSerializer(user) + return Response(serializer.data) + + +@api_view(["PUT"]) +def user_update(request, pk): + """ + Update user profile. Only accessible by Coordinators and the user themselves. + """ + try: + user = User.objects.get(pk=pk) + except User.DoesNotExist: + return Response({"detail": "Not found."}, status=status.HTTP_404_NOT_FOUND) + + if not ( + (request.user == user) or Coordinator.objects.filter(user=request.user).exists() + ): + raise PermissionDenied("You do not have permission to edit this profile") + + serializer = UserSerializer(user, data=request.data, partial=True) + if serializer.is_valid(): + serializer.save() + return Response(serializer.data) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + +@api_view(["GET"]) +def user_info(request): + """ + Get user info for request user """ serializer = UserSerializer(request.user) return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/docker-compose.yml b/docker-compose.yml index 9bcf67a8..eb0966c8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -52,6 +52,10 @@ services: source: ./ target: /opt/csm_web read_only: true + # output from migrations + - type: bind + source: ./csm_web/scheduler/migrations/ + target: /opt/csm_web/csm_web/scheduler/migrations/ depends_on: postgres: condition: service_healthy From 9b95742f1bd5c35e1b9ad3eaf5440bd01fd1c2a7 Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Mon, 5 Aug 2024 17:56:31 -0700 Subject: [PATCH 2/7] fixed the permissions so that there is separation of concerns, + fixedthe update --- csm_web/scheduler/views/user.py | 85 ++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/csm_web/scheduler/views/user.py b/csm_web/scheduler/views/user.py index 14372bb6..c041603f 100644 --- a/csm_web/scheduler/views/user.py +++ b/csm_web/scheduler/views/user.py @@ -32,10 +32,36 @@ def has_permission(request_user, target_user): """ Returns True if the user has permission to access or edit the target user's profile """ - # Does not account for users who are both mentors and students of different courses - # need separation of concerns: - # coordinators/mentor should only have coordinator/mentor access for their course - # Check if request_user is a mentor + + # if request_user.is_superuser: return True + if request_user == target_user: + return True + + # if requestor is a student, get all the sections they are in + # if the target user is a student in any of those sections, return True + if Student.objects.filter(user=request_user).exists(): + if Student.objects.filter(user=target_user).exists(): + request_user_sections = Student.objects.filter( + user=request_user + ).values_list("section", flat=True) + target_user_sections = Student.objects.filter(user=target_user).values_list( + "section", flat=True + ) + if set(request_user_sections) & set(target_user_sections): + return True + + # if the target user is a mentor in any of the courses the student is in, return True + if Mentor.objects.filter(user=target_user).exists(): + target_user_courses = Mentor.objects.filter(user=target_user).values_list( + "course", flat=True + ) + if Student.objects.filter( + user=request_user, course__in=target_user_courses + ).exists(): + return True + + # if requestor is a mentor, get all the courses they mentor + # if the target user is a student or mentor in any of those courses, return True if Mentor.objects.filter(user=request_user).exists(): mentor_courses = Mentor.objects.filter(user=request_user).values_list( "course", flat=True @@ -46,31 +72,24 @@ def has_permission(request_user, target_user): if Mentor.objects.filter(user=target_user, course__in=mentor_courses).exists(): return True - # Check if request_user is a student in the same course as target_user - # Students in the same section can see each other - if ( - Student.objects.filter(user=request_user).exists() - and Student.objects.filter(user=target_user).exists() - ): - request_user_courses = Student.objects.filter(user=request_user).values_list( + # if requestor is a coordinator, get all the courses they coordinate + # if the target user is a student or mentor in any of those courses, return True + if Coordinator.objects.filter(user=request_user).exists(): + coordinator_courses = Coordinator.objects.filter(user=request_user).values_list( "course", flat=True ) - target_user_courses = Student.objects.filter(user=target_user).values_list( - "course", flat=True - ) - - if set(request_user_courses) & set(target_user_courses): + if Student.objects.filter( + user=target_user, course__in=coordinator_courses + ).exists(): + return True + if Mentor.objects.filter( + user=target_user, course__in=coordinator_courses + ).exists(): + return True + if Coordinator.objects.filter( + user=target_user, course__in=coordinator_courses + ).exists(): return True - - # Coordinator access - if Coordinator.objects.filter( - user=request_user - ).exists(): # or if request_user.is_superuser - return True - - # Request user accessing their own profile - if request_user == target_user: - return True return False @@ -97,14 +116,22 @@ def user_update(request, pk): """ Update user profile. Only accessible by Coordinators and the user themselves. """ + # raise PermissionDenied("You do not have permission to edit this profile") try: user = User.objects.get(pk=pk) except User.DoesNotExist: return Response({"detail": "Not found."}, status=status.HTTP_404_NOT_FOUND) - if not ( - (request.user == user) or Coordinator.objects.filter(user=request.user).exists() - ): + if not request.user == user: + raise PermissionDenied("You do not have permission to edit this profile") + + coordinator_courses = Coordinator.objects.filter(user=request.user).values_list( + "course", flat=True + ) + + if not Coordinator.objects.filter( + user=request.user, course_in=coordinator_courses + ).exists(): raise PermissionDenied("You do not have permission to edit this profile") serializer = UserSerializer(user, data=request.data, partial=True) From e013332289b2f87f4f7aa06171f127096fb240b0 Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Tue, 6 Aug 2024 21:26:37 -0700 Subject: [PATCH 3/7] wrote tests for user.py and fixed bugs, passing all 10 --- csm_web/frontend/src/utils/queries/base.tsx | 4 +- csm_web/scheduler/models.py | 1 + csm_web/scheduler/tests/models/test_user.py | 206 +++++++++++++++++++- csm_web/scheduler/urls.py | 2 +- csm_web/scheduler/views/user.py | 54 ++--- cypress/e2e/course/restricted-courses.cy.ts | 8 +- 6 files changed, 242 insertions(+), 33 deletions(-) diff --git a/csm_web/frontend/src/utils/queries/base.tsx b/csm_web/frontend/src/utils/queries/base.tsx index 39dda134..6acf0518 100644 --- a/csm_web/frontend/src/utils/queries/base.tsx +++ b/csm_web/frontend/src/utils/queries/base.tsx @@ -41,9 +41,9 @@ export const useProfiles = (): UseQueryResult => { */ export const useUserInfo = (): UseQueryResult => { const queryResult = useQuery( - ["userinfo"], + ["user"], async () => { - const response = await fetchNormalized("/userinfo"); + const response = await fetchNormalized("/user"); if (response.ok) { return await response.json(); } else { diff --git a/csm_web/scheduler/models.py b/csm_web/scheduler/models.py index 9ac5cb71..e7174803 100644 --- a/csm_web/scheduler/models.py +++ b/csm_web/scheduler/models.py @@ -53,6 +53,7 @@ class User(AbstractUser): pronouns = models.CharField(max_length=20, default="", blank=True) pronunciation = models.CharField(max_length=50, default="", blank=True) + # xTODO: configure to use the Django Settings bucket backend # profile_pic = models.ImageField(upload_to='profiles/', null=True, blank=True) # if profile picture is implemented bio = models.CharField(max_length=500, default="", blank=True) diff --git a/csm_web/scheduler/tests/models/test_user.py b/csm_web/scheduler/tests/models/test_user.py index f14141af..7d511b60 100644 --- a/csm_web/scheduler/tests/models/test_user.py +++ b/csm_web/scheduler/tests/models/test_user.py @@ -1,10 +1,23 @@ -import pytest +import json +import pytest +from django.urls import reverse +from scheduler.factories import ( + CoordinatorFactory, + CourseFactory, + MentorFactory, + SectionFactory, + StudentFactory, + UserFactory, +) from scheduler.models import User @pytest.mark.django_db def test_create_user(): + """ + Test that a user can be created. + """ email = "test@berkeley.edu" username = "test" user, created = User.objects.get_or_create(email=email, username=username) @@ -13,3 +26,194 @@ def test_create_user(): assert user.username == username assert User.objects.count() == 1 assert User.objects.get(email=email).username == username + + +# avoid pylint warning redefining name in outer scope +@pytest.fixture(name="setup_permissions") +def fixture_setup_permissions(): + """ + Setup users, courses, and sections for testing permissions + """ + student_user = UserFactory(username="student_user") + other_student_user = UserFactory(username="other_student_user") + mentor_user = UserFactory(username="mentor_user") + other_mentor_user = UserFactory(username="other_mentor_user") + coordinator_user = UserFactory(username="coordinator_user") + + # Create courses + course_a = CourseFactory(name="course_a") + course_b = CourseFactory(name="course_b") + + # Assign mentors to courses + mentor_a = MentorFactory(user=mentor_user, course=course_a) + mentor_b = MentorFactory(user=other_mentor_user, course=course_b) + coordinator = CoordinatorFactory(user=coordinator_user, course=course_a) + + # Create sections associated with the correct course via the mentor + section_a1 = SectionFactory(mentor=mentor_a) + section_b1 = SectionFactory(mentor=mentor_b) + + # Ensure students are enrolled in sections that match their course + student_a1 = StudentFactory(user=student_user, section=section_a1, course=course_a) + other_student_a1 = StudentFactory( + user=other_student_user, section=section_a1, course=course_a + ) + + return { + "student_user": student_user, + "other_student_user": other_student_user, + "mentor_user": mentor_user, + "other_mentor_user": other_mentor_user, + "coordinator_user": coordinator, + "course_a": course_a, + "course_b": course_b, + "section_a1": section_a1, + "section_b1": section_b1, + "student_a1": student_a1, + "other_student_a1": other_student_a1, + } + + +############### +# Student tests +############### + + +@pytest.mark.django_db +def test_student_view_own_profile(client, setup_permissions): + """ + Test that a student can view their own profile. + """ + student_user = setup_permissions["student_user"] + client.force_login(student_user) + + response = client.get(reverse("user_retrieve", kwargs={"pk": student_user.pk})) + assert response.status_code == 200 + assert response.data["email"] == student_user.email + + +@pytest.mark.django_db +def test_student_view_other_student_in_same_section(client, setup_permissions): + """ + Test that a student can view another student in the same section. + """ + student = setup_permissions["student_user"] + other_student = setup_permissions["other_student_user"] + client.force_login(student) + response = client.get(reverse("user_retrieve", kwargs={"pk": other_student.pk})) + assert response.status_code == 200 + assert response.data["email"] == other_student.email + + +@pytest.mark.django_db +def test_student_view_mentors(client, setup_permissions): + """ + Test that a student can view a mentor's profile. + """ + student = setup_permissions["student_user"] + mentor = setup_permissions["mentor_user"] + client.force_login(student) + response = client.get(reverse("user_retrieve", kwargs={"pk": mentor.pk})) + assert response.status_code == 200 + assert response.data["email"] == mentor.email + + +@pytest.mark.django_db +def test_student_edit_own_profile(client, setup_permissions): + """ + Test that a student can edit their own profile. + """ + student = setup_permissions["student_user"] + client.force_login(student) + edit_url = reverse("user_update", kwargs={"pk": student.pk}) + response = client.put( + edit_url, + data=json.dumps({"first_name": "NewName"}), + content_type="application/json", + ) + assert response.status_code == 200 + student.refresh_from_db() + assert student.first_name == "NewName" + + +############## +# Mentor tests +############## +@pytest.mark.django_db +def test_mentor_view_own_profile(client, setup_permissions): + """ + Test that a mentor can view their own profile. + """ + mentor_user = setup_permissions["mentor_user"] + client.force_login(mentor_user) + + response = client.get(reverse("user_retrieve", kwargs={"pk": mentor_user.pk})) + assert response.status_code == 200 + assert response.data["email"] == mentor_user.email + + +@pytest.mark.django_db +def test_mentor_view_students_in_course(client, setup_permissions): + """ + Test that a mentor can view student profiles in the course they teach. + """ + mentor_user = setup_permissions["mentor_user"] + student_user = setup_permissions["student_user"] + client.force_login(mentor_user) + + response = client.get(reverse("user_retrieve", kwargs={"pk": student_user.pk})) + assert response.status_code == 200 + assert response.data["email"] == student_user.email + + +@pytest.mark.django_db +def test_mentor_cannot_edit_other_profiles(client, setup_permissions): + """ + Test that a mentor cannot edit another student's or mentor's profile. + """ + mentor_user = setup_permissions["mentor_user"] + other_student_user = setup_permissions["other_student_user"] + client.force_login(mentor_user) + response = client.put( + reverse("user_update", kwargs={"pk": other_student_user.pk}), + data=json.dumps({"first_name": "new_username"}), + content_type="application/json", + ) + assert response.status_code == 403 + + +################### +# Coordinator tests +################### + + +@pytest.mark.django_db +def test_coordinator_view_all_profiles_in_course(client, setup_permissions): + """ + Test that a coordinator can view all profiles in the course they coordinate. + """ + coordinator_user = setup_permissions["coordinator_user"] + student_user = setup_permissions["student_user"] + client.force_login(coordinator_user) + + response = client.get(reverse("user_retrieve", kwargs={"pk": student_user.pk})) + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_coordinator_edit_all_profiles_in_course(client, setup_permissions): + """ + Test that a coordinator can edit all profiles in the course they coordinate. + """ + coordinator_user = setup_permissions["coordinator_user"] + student_user = setup_permissions["student_user"] + client.force_login(coordinator_user) + + response = client.put( + reverse("user_update", kwargs={"pk": student_user.pk}), + data=json.dumps({"first_name": "new_student_name"}), + content_type="application/json", + ) + assert response.status_code == 200 + student_user.refresh_from_db() + assert student_user.first_name == "new_student_name" diff --git a/csm_web/scheduler/urls.py b/csm_web/scheduler/urls.py index 1df15e71..370f4e48 100644 --- a/csm_web/scheduler/urls.py +++ b/csm_web/scheduler/urls.py @@ -15,7 +15,7 @@ urlpatterns = router.urls urlpatterns += [ - path("user_info/", views.user_info, name="user_info"), + path("user/", views.user_info, name="user"), path("user//", views.user_retrieve, name="user_retrieve"), path("user//update/", views.user_update, name="user_update"), path("matcher/active/", views.matcher.active), diff --git a/csm_web/scheduler/views/user.py b/csm_web/scheduler/views/user.py index c041603f..6e4d352f 100644 --- a/csm_web/scheduler/views/user.py +++ b/csm_web/scheduler/views/user.py @@ -37,6 +37,10 @@ def has_permission(request_user, target_user): if request_user == target_user: return True + # if the target user is a mentor, return True + if Mentor.objects.filter(user=target_user).exists(): + return True + # if requestor is a student, get all the sections they are in # if the target user is a student in any of those sections, return True if Student.objects.filter(user=request_user).exists(): @@ -51,14 +55,14 @@ def has_permission(request_user, target_user): return True # if the target user is a mentor in any of the courses the student is in, return True - if Mentor.objects.filter(user=target_user).exists(): - target_user_courses = Mentor.objects.filter(user=target_user).values_list( - "course", flat=True - ) - if Student.objects.filter( - user=request_user, course__in=target_user_courses - ).exists(): - return True + # if Mentor.objects.filter(user=target_user).exists(): + # target_user_courses = Mentor.objects.filter(user=target_user).values_list( + # "course", flat=True + # ) + # if Student.objects.filter( + # user=request_user, course__in=target_user_courses + # ).exists(): + # return True # if requestor is a mentor, get all the courses they mentor # if the target user is a student or mentor in any of those courses, return True @@ -69,8 +73,8 @@ def has_permission(request_user, target_user): if Student.objects.filter(user=target_user, course__in=mentor_courses).exists(): return True - if Mentor.objects.filter(user=target_user, course__in=mentor_courses).exists(): - return True + # if Mentor.objects.filter(user=target_user, course__in=mentor_courses).exists(): + # return True # if requestor is a coordinator, get all the courses they coordinate # if the target user is a student or mentor in any of those courses, return True @@ -82,10 +86,10 @@ def has_permission(request_user, target_user): user=target_user, course__in=coordinator_courses ).exists(): return True - if Mentor.objects.filter( - user=target_user, course__in=coordinator_courses - ).exists(): - return True + # if Mentor.objects.filter( + # user=target_user, course__in=coordinator_courses + # ).exists(): + # return True if Coordinator.objects.filter( user=target_user, course__in=coordinator_courses ).exists(): @@ -122,17 +126,17 @@ def user_update(request, pk): except User.DoesNotExist: return Response({"detail": "Not found."}, status=status.HTTP_404_NOT_FOUND) - if not request.user == user: - raise PermissionDenied("You do not have permission to edit this profile") - - coordinator_courses = Coordinator.objects.filter(user=request.user).values_list( - "course", flat=True - ) - - if not Coordinator.objects.filter( - user=request.user, course_in=coordinator_courses - ).exists(): - raise PermissionDenied("You do not have permission to edit this profile") + if request.user == user: + pass + else: + # Check if the user is a coordinator and has permission + coordinator_courses = Coordinator.objects.filter(user=request.user).values_list( + "course", flat=True + ) + if not Coordinator.objects.filter( + user=request.user, course__in=coordinator_courses + ).exists(): + raise PermissionDenied("You do not have permission to edit this profile") serializer = UserSerializer(user, data=request.data, partial=True) if serializer.is_valid(): diff --git a/cypress/e2e/course/restricted-courses.cy.ts b/cypress/e2e/course/restricted-courses.cy.ts index b4dad30e..6ad2bf7b 100644 --- a/cypress/e2e/course/restricted-courses.cy.ts +++ b/cypress/e2e/course/restricted-courses.cy.ts @@ -72,7 +72,7 @@ describe("whitelisted courses", () => { cy.login(); cy.intercept("/api/courses").as("get-courses"); - cy.intercept("/api/userinfo").as("get-userinfo"); + cy.intercept("/api/user").as("get-user"); cy.visit("/"); cy.wait("@get-courses"); @@ -82,7 +82,7 @@ describe("whitelisted courses", () => { // view courses cy.contains(".primary-btn", /add course/i).click(); - cy.wait("@get-userinfo"); + cy.wait("@get-user"); cy.contains(".page-title", /which course/i).should("be.visible"); // should have two buttons at the top @@ -126,7 +126,7 @@ describe("whitelisted courses", () => { cy.login(); cy.intercept("/api/courses").as("get-courses"); - cy.intercept("/api/userinfo").as("get-userinfo"); + cy.intercept("/api/user").as("get-user"); cy.visit("/"); cy.wait("@get-courses"); @@ -136,7 +136,7 @@ describe("whitelisted courses", () => { // view courses cy.contains(".primary-btn", /add course/i).click(); - cy.wait("@get-userinfo"); + cy.wait("@get-user"); cy.contains(".page-title", /which course/i).should("be.visible"); // should have two buttons at the top From 24044bd555d99fc7109e944eb12be5aea6900305 Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Tue, 6 Aug 2024 21:34:43 -0700 Subject: [PATCH 4/7] fixed little bug in test --- csm_web/scheduler/tests/models/test_user.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/csm_web/scheduler/tests/models/test_user.py b/csm_web/scheduler/tests/models/test_user.py index 7d511b60..4f7a8ce0 100644 --- a/csm_web/scheduler/tests/models/test_user.py +++ b/csm_web/scheduler/tests/models/test_user.py @@ -3,7 +3,6 @@ import pytest from django.urls import reverse from scheduler.factories import ( - CoordinatorFactory, CourseFactory, MentorFactory, SectionFactory, @@ -47,7 +46,6 @@ def fixture_setup_permissions(): # Assign mentors to courses mentor_a = MentorFactory(user=mentor_user, course=course_a) mentor_b = MentorFactory(user=other_mentor_user, course=course_b) - coordinator = CoordinatorFactory(user=coordinator_user, course=course_a) # Create sections associated with the correct course via the mentor section_a1 = SectionFactory(mentor=mentor_a) @@ -64,7 +62,7 @@ def fixture_setup_permissions(): "other_student_user": other_student_user, "mentor_user": mentor_user, "other_mentor_user": other_mentor_user, - "coordinator_user": coordinator, + "coordinator_user": coordinator_user, "course_a": course_a, "course_b": course_b, "section_a1": section_a1, From 5085cc87c0b789ae67685791bd2761eb6d653ca2 Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Thu, 8 Aug 2024 13:59:50 -0700 Subject: [PATCH 5/7] dynamic user routing works now I think --- csm_web/frontend/src/components/App.tsx | 12 +- .../frontend/src/components/CourseMenu.tsx | 2 +- .../frontend/src/components/UserProfile.tsx | 70 ++++++++++++ .../src/components/section/Section.tsx | 1 - csm_web/frontend/src/utils/queries/base.tsx | 48 +------- csm_web/frontend/src/utils/queries/user.tsx | 103 ++++++++++++++++++ csm_web/frontend/src/utils/types.tsx | 3 + csm_web/scheduler/tests/models/test_user.py | 5 + 8 files changed, 193 insertions(+), 51 deletions(-) create mode 100644 csm_web/frontend/src/components/UserProfile.tsx create mode 100644 csm_web/frontend/src/utils/queries/user.tsx diff --git a/csm_web/frontend/src/components/App.tsx b/csm_web/frontend/src/components/App.tsx index 51a2b012..66ca218c 100644 --- a/csm_web/frontend/src/components/App.tsx +++ b/csm_web/frontend/src/components/App.tsx @@ -8,6 +8,7 @@ import { emptyRoles, Roles } from "../utils/user"; import CourseMenu from "./CourseMenu"; import Home from "./Home"; import Policies from "./Policies"; +import UserProfile from "./UserProfile"; import { DataExport } from "./data_export/DataExport"; import { EnrollmentMatcher } from "./enrollment_automation/EnrollmentMatcher"; import { Resources } from "./resource_aggregation/Resources"; @@ -42,8 +43,12 @@ const App = () => { } /> } /> { - // TODO: add route for profiles (/user/:id/* element = {UserProfile}) + // TODO: add route for profiles (/profile/:id/* element = {UserProfile}) + // TODO: add route for your own profile /profile/* + // reference Section } + } /> + } /> } /> @@ -82,7 +87,7 @@ function Header(): React.ReactElement { }; /** - * Helper function to determine class name for the home NavLInk component; + * Helper function to determine class name for the home NavLnk component; * is always active unless we're in another tab. */ const homeNavlinkClass = () => { @@ -143,6 +148,9 @@ function Header(): React.ReactElement {

Policies

 + +

Profile

+ diff --git a/csm_web/frontend/src/components/CourseMenu.tsx b/csm_web/frontend/src/components/CourseMenu.tsx index a4d67e90..0179f412 100644 --- a/csm_web/frontend/src/components/CourseMenu.tsx +++ b/csm_web/frontend/src/components/CourseMenu.tsx @@ -3,8 +3,8 @@ import React, { useEffect, useState } from "react"; import { Link, Route, Routes } from "react-router-dom"; import { DEFAULT_LONG_LOCALE_OPTIONS, DEFAULT_TIMEZONE } from "../utils/datetime"; -import { useUserInfo } from "../utils/queries/base"; import { useCourses } from "../utils/queries/courses"; +import { useUserInfo } from "../utils/queries/user"; import { Course as CourseType, UserInfo } from "../utils/types"; import LoadingSpinner from "./LoadingSpinner"; import Course from "./course/Course"; diff --git a/csm_web/frontend/src/components/UserProfile.tsx b/csm_web/frontend/src/components/UserProfile.tsx new file mode 100644 index 00000000..b7d28b6b --- /dev/null +++ b/csm_web/frontend/src/components/UserProfile.tsx @@ -0,0 +1,70 @@ +import React from "react"; +import { useParams } from "react-router-dom"; +import { useUser } from "../utils/queries/user"; +import LoadingSpinner from "./LoadingSpinner"; +import "../css/base/form.scss"; // Import the base.scss file for styling +import "../css/base/table.scss"; + +const UserProfile: React.FC = () => { + const { id } = useParams(); // Type the id parameter + const { data, error, isLoading } = useUser(Number(id)); + + // Handle loading and error states + if (isLoading) { + return ; + } + if (error) { + return
Error: {error.message}
; + } + + return ( +
+

User Profile

+
+
+ +

+ {data?.firstName} +

+
+
+ +

+ {data?.lastName} +

+
+
+ +

+ {data?.email} +

+
+
+ +

+ {data?.bio || "N/A"} +

+
+
+ +

+ {data?.pronouns || "N/A"} +

+
+
+ +

+ {data?.pronunciation || "N/A"} +

+
+
+ +
+
+
+ ); +}; + +export default UserProfile; diff --git a/csm_web/frontend/src/components/section/Section.tsx b/csm_web/frontend/src/components/section/Section.tsx index 769d7919..4d725134 100644 --- a/csm_web/frontend/src/components/section/Section.tsx +++ b/csm_web/frontend/src/components/section/Section.tsx @@ -13,7 +13,6 @@ import "../../css/section.scss"; export default function Section(): React.ReactElement | null { const { id } = useParams(); - const { data: section, isSuccess: sectionLoaded, isError: sectionLoadError } = useSection(Number(id)); if (!sectionLoaded) { diff --git a/csm_web/frontend/src/utils/queries/base.tsx b/csm_web/frontend/src/utils/queries/base.tsx index 6acf0518..5f9dfa32 100644 --- a/csm_web/frontend/src/utils/queries/base.tsx +++ b/csm_web/frontend/src/utils/queries/base.tsx @@ -10,7 +10,7 @@ import { useQuery, UseQueryResult } from "@tanstack/react-query"; import { fetchNormalized } from "../api"; -import { Profile, RawUserInfo } from "../types"; +import { Profile } from "../types"; import { handleError, handlePermissionsError, handleRetry, ServerError } from "./helpers"; /** @@ -34,49 +34,3 @@ export const useProfiles = (): UseQueryResult => { handleError(queryResult); return queryResult; }; - -// TODO: move to new queries/users.tsx -/** - * Hook to get the user's info. - */ -export const useUserInfo = (): UseQueryResult => { - const queryResult = useQuery( - ["user"], - async () => { - const response = await fetchNormalized("/user"); - if (response.ok) { - return await response.json(); - } else { - handlePermissionsError(response.status); - throw new ServerError("Failed to fetch user info"); - } - }, - { retry: handleRetry } - ); - - handleError(queryResult); - return queryResult; -}; - -// TODO: move to new queries/users.tsx -/** - * Hook to get a list of all user emails. - */ -export const useUserEmails = (): UseQueryResult => { - const queryResult = useQuery( - ["users"], - async () => { - const response = await fetchNormalized("/users"); - if (response.ok) { - return await response.json(); - } else { - handlePermissionsError(response.status); - throw new ServerError("Failed to fetch user info"); - } - }, - { retry: handleRetry } - ); - - handleError(queryResult); - return queryResult; -}; diff --git a/csm_web/frontend/src/utils/queries/user.tsx b/csm_web/frontend/src/utils/queries/user.tsx new file mode 100644 index 00000000..b439302e --- /dev/null +++ b/csm_web/frontend/src/utils/queries/user.tsx @@ -0,0 +1,103 @@ +import { useQuery, UseQueryResult } from "@tanstack/react-query"; +import { fetchNormalized } from "../api"; +import { RawUserInfo } from "../types"; +import { handleError, handlePermissionsError, handleRetry, ServerError } from "./helpers"; + +/** + * Hook to get a list of all user emails. + */ +export const useUserEmails = (): UseQueryResult => { + const queryResult = useQuery( + ["users"], + async () => { + const response = await fetchNormalized("/users"); + if (response.ok) { + return await response.json(); + } else { + handlePermissionsError(response.status); + throw new ServerError("Failed to fetch user info"); + } + }, + { retry: handleRetry } + ); + + handleError(queryResult); + return queryResult; +}; + +/** + * Hook to get the current user's info. + */ +// TODO: merge with useUserDetails +export const useUserInfo = (): UseQueryResult => { + const queryResult = useQuery( + ["user"], + async () => { + const response = await fetchNormalized("/user"); + if (response.ok) { + return await response.json(); + } else { + handlePermissionsError(response.status); + throw new ServerError("Failed to fetch user info"); + } + }, + { retry: handleRetry } + ); + + handleError(queryResult); + return queryResult; +}; + +/** + * Hook to get the requested user's info + */ +// TODO: handle if there is no userId +export const useUserDetails = (userId?: number): UseQueryResult => { + const queryResult = useQuery( + ["userDetails", userId], + async () => { + const response = await fetchNormalized(`/user/${userId}`); + if (response.ok) { + return await response.json(); + } else { + handlePermissionsError(response.status); + throw new ServerError("Failed to fetch user details"); + } + }, + { + retry: handleRetry, + enabled: !!userId // only run query if userId is available + } + ); + + handleError(queryResult); + return queryResult; +}; + +/** + * Hook to get user info. If userId is provided, fetches details for that user; otherwise, fetches current user's info. + */ +export const useUser = (userId?: number): UseQueryResult => { + const queryKey = userId ? ["userDetails", userId] : ["user"]; + + const queryResult = useQuery( + queryKey, + async () => { + const endpoint = userId ? `/user/${userId}` : "/user"; + const response = await fetchNormalized(endpoint); + if (response.ok) { + return await response.json(); + } else { + handlePermissionsError(response.status); + throw new ServerError(userId ? "Failed to fetch user details" : "Failed to fetch user info"); + } + }, + { + retry: handleRetry, + enabled: userId !== undefined // Only run the query if userId is provided or if fetching current user's info + } + ); + + handleError(queryResult); + return queryResult; +}; diff --git a/csm_web/frontend/src/utils/types.tsx b/csm_web/frontend/src/utils/types.tsx index 78e9f215..3c189893 100644 --- a/csm_web/frontend/src/utils/types.tsx +++ b/csm_web/frontend/src/utils/types.tsx @@ -36,6 +36,9 @@ export interface UserInfo { lastName: string; email: string; priorityEnrollment?: DateTime; + bio: string; + pronouns: string; + pronunciation: string; } /** diff --git a/csm_web/scheduler/tests/models/test_user.py b/csm_web/scheduler/tests/models/test_user.py index 4f7a8ce0..5533988a 100644 --- a/csm_web/scheduler/tests/models/test_user.py +++ b/csm_web/scheduler/tests/models/test_user.py @@ -3,6 +3,7 @@ import pytest from django.urls import reverse from scheduler.factories import ( + CoordinatorFactory, CourseFactory, MentorFactory, SectionFactory, @@ -46,6 +47,7 @@ def fixture_setup_permissions(): # Assign mentors to courses mentor_a = MentorFactory(user=mentor_user, course=course_a) mentor_b = MentorFactory(user=other_mentor_user, course=course_b) + coordinator_a = CoordinatorFactory(user=coordinator_user, course=course_a) # Create sections associated with the correct course via the mentor section_a1 = SectionFactory(mentor=mentor_a) @@ -63,6 +65,7 @@ def fixture_setup_permissions(): "mentor_user": mentor_user, "other_mentor_user": other_mentor_user, "coordinator_user": coordinator_user, + "coordinator_a": coordinator_a, "course_a": course_a, "course_b": course_b, "section_a1": section_a1, @@ -137,6 +140,8 @@ def test_student_edit_own_profile(client, setup_permissions): ############## # Mentor tests ############## + + @pytest.mark.django_db def test_mentor_view_own_profile(client, setup_permissions): """ From e8e16ad80100982887dc7536c934070230b6ef62 Mon Sep 17 00:00:00 2001 From: ElaineShu0312 Date: Tue, 20 Aug 2024 18:56:24 -0700 Subject: [PATCH 6/7] finished profile form --- .../frontend/src/components/CourseMenu.tsx | 2 +- .../frontend/src/components/UserProfile.tsx | 250 ++++++++++++++---- .../components/course/CreateSectionModal.tsx | 2 +- .../section/CoordinatorAddStudentModal.tsx | 3 +- csm_web/frontend/src/css/base/form.scss | 2 +- .../frontend/src/utils/queries/profiles.tsx | 85 ++++++ csm_web/frontend/src/utils/queries/user.tsx | 103 -------- csm_web/frontend/src/utils/types.tsx | 1 + csm_web/scheduler/views/user.py | 53 ++-- 9 files changed, 314 insertions(+), 187 deletions(-) create mode 100644 csm_web/frontend/src/utils/queries/profiles.tsx delete mode 100644 csm_web/frontend/src/utils/queries/user.tsx diff --git a/csm_web/frontend/src/components/CourseMenu.tsx b/csm_web/frontend/src/components/CourseMenu.tsx index 0179f412..3796a469 100644 --- a/csm_web/frontend/src/components/CourseMenu.tsx +++ b/csm_web/frontend/src/components/CourseMenu.tsx @@ -4,7 +4,7 @@ import { Link, Route, Routes } from "react-router-dom"; import { DEFAULT_LONG_LOCALE_OPTIONS, DEFAULT_TIMEZONE } from "../utils/datetime"; import { useCourses } from "../utils/queries/courses"; -import { useUserInfo } from "../utils/queries/user"; +import { useUserInfo } from "../utils/queries/profiles"; import { Course as CourseType, UserInfo } from "../utils/types"; import LoadingSpinner from "./LoadingSpinner"; import Course from "./course/Course"; diff --git a/csm_web/frontend/src/components/UserProfile.tsx b/csm_web/frontend/src/components/UserProfile.tsx index b7d28b6b..933abd6e 100644 --- a/csm_web/frontend/src/components/UserProfile.tsx +++ b/csm_web/frontend/src/components/UserProfile.tsx @@ -1,68 +1,226 @@ -import React from "react"; +import React, { useState, useEffect } from "react"; import { useParams } from "react-router-dom"; -import { useUser } from "../utils/queries/user"; +import { PermissionError } from "../utils/queries/helpers"; +import { useUserInfo, useUserInfoUpdateMutation } from "../utils/queries/profiles"; import LoadingSpinner from "./LoadingSpinner"; -import "../css/base/form.scss"; // Import the base.scss file for styling + +import "../css/base/form.scss"; import "../css/base/table.scss"; const UserProfile: React.FC = () => { - const { id } = useParams(); // Type the id parameter - const { data, error, isLoading } = useUser(Number(id)); + const { id } = useParams(); + let userId = Number(id); + const { data: currUserData, isError: isCurrUserError, isLoading: currUserIsLoading } = useUserInfo(); + const { data: requestedData, error: requestedError, isLoading: requestedIsLoading } = useUserInfo(userId); + const updateMutation = useUserInfoUpdateMutation(userId); + const [isEditing, setIsEditing] = useState(false); + + const [formData, setFormData] = useState({ + firstName: "", + lastName: "", + bio: "", + pronouns: "", + pronunciation: "" + }); + + const [showSaveSpinner, setShowSaveSpinner] = useState(false); + const [validationText, setValidationText] = useState(""); + + // Populate form data with fetched user data + useEffect(() => { + if (requestedData) { + setFormData({ + firstName: requestedData.firstName || "", + lastName: requestedData.lastName || "", + bio: requestedData.bio || "", + pronouns: requestedData.pronouns || "", + pronunciation: requestedData.pronunciation || "" + }); + } + }, [requestedData]); - // Handle loading and error states - if (isLoading) { + if (requestedIsLoading || currUserIsLoading) { return ; } - if (error) { - return
Error: {error.message}
; + + if (requestedError || isCurrUserError) { + if (requestedError instanceof PermissionError) { + return

Permission Denied

; + } else { + return

Failed to fetch user data

; + } + } + + if (id === undefined && requestedData) { + userId = requestedData.id; } + // Handle input changes + const handleInputChange = (e: React.ChangeEvent) => { + const { name, value } = e.target; + setFormData(prev => ({ + ...prev, + [name]: value + })); + }; + + // Validate current form data + const validateFormData = (): boolean => { + if (!formData.firstName || !formData.lastName) { + setValidationText("First and last names must be specified."); + return false; + } + + setValidationText(""); + return true; + }; + + // Handle form submission + const handleFormSubmit = () => { + if (!validateFormData()) { + return; + } + + setShowSaveSpinner(true); + + updateMutation.mutate( + { + id: userId, + firstName: formData.firstName, + lastName: formData.lastName, + bio: formData.bio, + pronouns: formData.pronouns, + pronunciation: formData.pronunciation + }, + { + onSuccess: () => { + setIsEditing(false); // Exit edit mode after successful save + console.log("Profile updated successfully"); + setShowSaveSpinner(false); + }, + onError: () => { + setValidationText("Error occurred on save."); + setShowSaveSpinner(false); + } + } + ); + }; + + const isCurrUser = currUserData?.id === requestedData?.id || requestedData.isEditable; + + // Toggle edit mode + const handleEditToggle = () => { + setIsEditing(true); + }; + return ( -
-

User Profile

-
-
- -

- {data?.firstName} -

+
+

User Profile

+
+
+ + {isEditing ? ( + + ) : ( +

{formData.firstName}

+ )}
-
- -

- {data?.lastName} -

+
+ + {isEditing ? ( + + ) : ( +

{formData.lastName}

+ )}
-
- -

- {data?.email} -

+
+ + {isEditing ? ( + + ) : ( +

{formData.pronunciation}

+ )}
-
- -

- {data?.bio || "N/A"} -

+
+ + {isEditing ? ( + + ) : ( +

{formData.pronouns}

+ )}
-
- -

- {data?.pronouns || "N/A"} -

+
+ +

{requestedData?.email}

-
- -

- {data?.pronunciation || "N/A"} -

+
+ + {isEditing ? ( +