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

Feat/search bar/backend #472

Open
wants to merge 6 commits into
base: feat/coord-interface/main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions csm_web/scheduler/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,7 @@
path("matcher/<int:pk>/mentors/", views.matcher.mentors),
path("matcher/<int:pk>/configure/", views.matcher.configure),
path("matcher/<int:pk>/create/", views.matcher.create),
path(
"search/get-sections/", views.get_sections_of_user, name="get_sections_of_user"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, the path itself shouldn't need to include actions like "get"; the HTTP method that the request has should indicate what the user wants. The more conventional path would be something like search/sections/.

),
]
3 changes: 2 additions & 1 deletion csm_web/scheduler/views/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from .course import CourseViewSet
from . import matcher
from .course import CourseViewSet
from .profile import ProfileViewSet
from .resource import ResourceViewSet
from .search import get_sections_of_user
from .section import SectionViewSet
from .spacetime import SpacetimeViewSet
from .student import StudentViewSet
Expand Down
123 changes: 123 additions & 0 deletions csm_web/scheduler/views/search.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
from django.db.models import Count, Q
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.models import Section, Student
from scheduler.serializers import SectionSerializer, StudentSerializer


@api_view(["GET"])
def get_sections_of_user(request):
"""
Gets all sections that match the queried user.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend adding a little more detail in this docstring describing exactly what the view expects from the request (ex. all of the possible query parameters, possible values for each parameter, how it handles each parameter, etc.)

"""

is_coordinator = bool(
request.user.coordinator_set.filter(user=request.user).count()
)
if not is_coordinator:
raise PermissionDenied(
"You are not authorized to search through sections of this course."
)

query = request.query_params.get("query", "")
student_absences = request.query_params.get("student_absences", None)

if not query and student_absences is None:
return Response(
{"error": "Please provide a query"}, status=status.HTTP_400_BAD_REQUEST
)

sections = Section.objects.all()

# Fetch courses associated with the user's coordinator role
courses = request.user.coordinator_set.values_list("course", flat=True)

# Filter sections based on the courses associated with the user's coordinator role
sections = sections.filter(mentor__course__in=courses)

if query:
sections = sections.filter(
# pylint: disable-next=unsupported-binary-operation
Q(students__user__first_name__icontains=query)
| Q(students__user__last_name__icontains=query)
| Q(students__user__email__icontains=query)
| Q(mentor__user__first_name__icontains=query)
| Q(mentor__user__last_name__icontains=query)
| Q(mentor__user__email__icontains=query)
).distinct()

students = Student.objects.filter(
# pylint: disable-next=unsupported-binary-operation
Q(user__first_name__icontains=query)
| Q(user__last_name__icontains=query)
| Q(user__email__icontains=query),
section__in=sections,
).distinct()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This students object seems to be overwritten in the if statements below when working with student_absences. If it's possible to provide both the query and the student_absences parameters, then they should work together properly. (And this should be explained in the docstring as well.)


if student_absences is not None:
try:
# Check if the query is a range or single number
if "-" in student_absences:
start, end = student_absences.split("-")
students = (
Student.objects.annotate(
num_absences=Count(
"attendance", filter=Q(attendance__presence="UN")
)
)
.filter(
num_absences__gte=start,
num_absences__lte=end,
section__in=sections,
)
.distinct()
)
else:
num_absences = int(student_absences)
students = (
Student.objects.annotate(
num_absences=Count(
"attendance", filter=Q(attendance__presence="UN")
)
)
.filter(num_absences=num_absences, section__in=sections)
.distinct()
)
sections = sections.filter(students__in=students).distinct()
except ValueError:
return Response(
{
"error": (
"Invalid value for student_absences. Please provide an integer."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you also accept ranges, so more than just integers should be expected (and the error message should reflect that).

An alternative to providing this union of possible types is to specify a new (mutually exclusive) query parameter for a range of student absences. Either works though, so it's up to you whether you want to stick with this or split it up.

)
},
status=status.HTTP_400_BAD_REQUEST,
)

section_serializer = SectionSerializer(
sections, many=True, context={"request": request}
)

student_data = []
for student in students:
# attendance_data = student.attendance.filter(presence="UN")
student_serializer = StudentSerializer(student)
student_data.append(
{
"id": student_serializer.data["id"],
"name": student_serializer.data["name"],
"email": student_serializer.data["email"],
"section": student_serializer.data["section"],
"mentor": student.section.mentor.user.get_full_name(),
"num_absences": student.num_absences,
}
)

combined_data = {
"sections": section_serializer.data,
"students": student_data,
}

return Response(combined_data, status=status.HTTP_200_OK)
Loading