From cbbaffceb274bd6b06cafaea72f5d634cbf8def8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9=20=D0=A7=D0=B5?= <39742182+Dmi4er4@users.noreply.github.com> Date: Tue, 2 Jul 2024 20:06:15 +0300 Subject: [PATCH] fix tests and remove n+1 (#856) --- apps/admission/selectors.py | 2 +- apps/admission/tests/test_views.py | 9 +- apps/courses/selectors.py | 2 +- apps/learning/icalendar.py | 15 ++- apps/learning/tests/test_icalendar.py | 33 +++++- apps/learning/views/icalendar.py | 4 +- lms/tests.py | 148 ++++++++++++++------------ lms/views.py | 2 +- 8 files changed, 130 insertions(+), 85 deletions(-) diff --git a/apps/admission/selectors.py b/apps/admission/selectors.py index 886aa0c21c..a107160bf2 100644 --- a/apps/admission/selectors.py +++ b/apps/admission/selectors.py @@ -144,4 +144,4 @@ def residence_city_campaigns_queryset( def get_ongoing_interviews(user): return (interview for interview in user.interview_set.select_related('slot__stream') - if interview.slot.datetime_local >= timezone.now()) + if not hasattr(interview, 'slot') or interview.slot.datetime_local >= timezone.now()) diff --git a/apps/admission/tests/test_views.py b/apps/admission/tests/test_views.py index 17ef502c19..22a504300f 100644 --- a/apps/admission/tests/test_views.py +++ b/apps/admission/tests/test_views.py @@ -176,7 +176,14 @@ def test_view_interview_list_csv(client, curator, settings): response = client.get(url) status_log_csv = response.content.decode("utf-8") data = [s for s in csv.reader(io.StringIO(status_log_csv))] - headers = ["date", "time (Europe/Moscow)", "applicant_name", "interviewer_name"] + headers = ["Date", + "Time Europe/Moscow", + "Section", + "Applicant", + "Interviewer", + "Status", + "Format" + ] today = today_local_spb.strftime("%d.%m.%Y") tomorrow = (today_local_spb + datetime.timedelta(days=1)).strftime("%d.%m.%Y") assert len(data) == 3 diff --git a/apps/courses/selectors.py b/apps/courses/selectors.py index e76c872f4d..a66ed7ddde 100644 --- a/apps/courses/selectors.py +++ b/apps/courses/selectors.py @@ -71,7 +71,7 @@ def course_teachers_prefetch_queryset(*, role_priority: bool = True, return (queryset .only('id', 'course_id', 'teacher_id', 'roles', 'teacher__first_name', 'teacher__last_name', 'teacher__patronymic', - 'teacher__gender', 'teacher__photo', 'teacher__cropbox_data') + 'teacher__gender', 'teacher__photo', 'teacher__cropbox_data', 'teacher__username' ) .order_by(*order_by, 'teacher__last_name', 'teacher__first_name')) diff --git a/apps/learning/icalendar.py b/apps/learning/icalendar.py index f788c9c17c..f511bc252b 100644 --- a/apps/learning/icalendar.py +++ b/apps/learning/icalendar.py @@ -1,5 +1,5 @@ from abc import ABC, abstractmethod -from datetime import datetime +from datetime import datetime, timedelta from typing import Callable, Iterable, List, Literal, NamedTuple import pytz @@ -213,16 +213,21 @@ def _model_to_dict(self, instance: Event): 'categories': vInline('CSC,EVENT') } -class InteviewICalendarEvent(ICalendarEvent): +class InterviewICalendarEvent(ICalendarEvent): def get_calendar_event_id(self, instance: Interview, user): return f"interviews-{user.pk}-{instance.pk}-admission@{self.domain}" def _model_to_dict(self, instance: Interview): absolute_url = self.url_builder(instance.get_absolute_url()) description = str(instance) - summary = f"Собеседование - {instance.get_section_display()} ({instance.slot.stream.get_format_display()})" - starts_at = instance.slot.datetime_local - ends_at = instance.slot.datetime_end_local + try: + summary = f"Собеседование - {instance.get_section_display()} ({instance.slot.stream.get_format_display()})" + starts_at = instance.slot.datetime_local + ends_at = instance.slot.datetime_end_local + except Interview.slot.RelatedObjectDoesNotExist: + summary = f"Собеседование - {instance.get_section_display()} (Неизвестно)" + starts_at = instance.date_local() + ends_at = starts_at + timedelta(minutes=30) return { 'url': vUri(absolute_url), 'summary': vText(summary), diff --git a/apps/learning/tests/test_icalendar.py b/apps/learning/tests/test_icalendar.py index 3b9510e825..e803616924 100644 --- a/apps/learning/tests/test_icalendar.py +++ b/apps/learning/tests/test_icalendar.py @@ -5,12 +5,14 @@ from django.contrib.sites.models import Site +from admission.constants import InterviewSections +from admission.tests.factories import InterviewFactory from core.urls import reverse from courses.tests.factories import AssignmentFactory, CourseClassFactory, CourseFactory from learning.settings import Branches from learning.tests.factories import EnrollmentFactory, EventFactory from users.constants import Roles -from users.tests.factories import StudentFactory +from users.tests.factories import StudentFactory, UserFactory # TODO: убедиться, что для заданий/занятий учитывается таймзона пользователя из URL календаря, а не залогиненного # TODO: для событий - пока залогиненного @@ -18,12 +20,17 @@ @pytest.mark.django_db def test_smoke(client, curator, settings): - """Any user can view icalendar since these urls are not secret""" + """User can view only own icalendar since these urls are secret""" student = StudentFactory() + other_student = StudentFactory() + client.login(student) response = client.get(student.get_classes_icalendar_url()) assert response.status_code == 200 response = client.get(student.get_assignments_icalendar_url()) assert response.status_code == 200 + response = client.get(other_student.get_assignments_icalendar_url()) + assert response.status_code == 302 + assert response.url.startswith('/login') @pytest.mark.django_db @@ -83,7 +90,27 @@ def test_assignments(client, settings, mocker): cal = Calendar.from_ical(resp.content) assert {f"{a.title} ({a.course.meta_course.name})" for a in chain(as_teaching, as_learning)} == { - evt['SUMMARY'] for evt in cal.subcomponents if isinstance(evt, Event)} + evt['SUMMARY'] for evt in cal.subcomponents if isinstance(evt, Event)} + + +@pytest.mark.django_db +def test_interviews(client, settings, mocker): + user = UserFactory(groups=[Roles.INTERVIEWER]) + client.login(user) + fname = 'interviews.ics' + # Empty calendar + url = reverse('user_ical_interviews', args=[user.pk], + subdomain=settings.LMS_SUBDOMAIN) + resp = client.get(url) + assert "text/calendar; charset=UTF-8" == resp['content-type'] + assert fname in resp['content-disposition'] + cal = Calendar.from_ical(resp.content) + site = Site.objects.get(pk=settings.SITE_ID) + assert f"Собеседования {site.name}" == cal['X-WR-CALNAME'] + InterviewFactory.create_batch(2, interviewers=[user], section=InterviewSections.MATH) + resp = client.get(url) + cal = Calendar.from_ical(resp.content) + assert len([evt for evt in cal.subcomponents if isinstance(evt, Event)]) == 2 @pytest.mark.django_db diff --git a/apps/learning/views/icalendar.py b/apps/learning/views/icalendar.py index 8a5466002e..f469f2d926 100644 --- a/apps/learning/views/icalendar.py +++ b/apps/learning/views/icalendar.py @@ -12,7 +12,7 @@ from learning.icalendar import ( StudentAssignmentICalendarEvent, StudentClassICalendarEvent, StudyEventICalendarEvent, TeacherAssignmentICalendarEvent, - TeacherClassICalendarEvent, generate_icalendar, InteviewICalendarEvent + TeacherClassICalendarEvent, generate_icalendar, InterviewICalendarEvent ) from learning.models import StudentAssignment from learning.selectors import ( @@ -141,6 +141,6 @@ def get_calendar_meta(user, site, url_builder, tz) -> ICalendarMeta: file_name="interviews.ics" ) def get_calendar_events(self, user, site, url_builder, tz): - event_factory = InteviewICalendarEvent(tz, url_builder, site) + event_factory = InterviewICalendarEvent(tz, url_builder, site) for interview in get_ongoing_interviews(user): yield event_factory.create(interview, user) diff --git a/lms/tests.py b/lms/tests.py index 2f5ecec2ca..a872f23d43 100644 --- a/lms/tests.py +++ b/lms/tests.py @@ -1,11 +1,12 @@ import datetime -from pprint import pprint +from unittest import mock import factory import pytest from django.conf import settings from django.contrib.sites.models import Site +from django.utils import timezone from django.utils.encoding import smart_bytes from django.utils.timezone import now @@ -172,38 +173,39 @@ def test_view_course_offerings(client): def test_view_course_offerings_invited_restriction(client): """Invited students should only see courses for which they were enrolled or invited""" - url = reverse('course_list', subdomain=settings.LMS_SUBDOMAIN) - future = now() + datetime.timedelta(days=3) - autumn_term = SemesterFactory.create_current(enrollment_period__ends_on=future.date()) - site = SiteFactory(id=settings.SITE_ID) - course_invitation = CourseInvitationFactory(course__semester=autumn_term) - student_profile = StudentProfileFactory(type=StudentTypes.INVITED) - student = student_profile.user - complete_student_profile(student, site, course_invitation.invitation) - - autumn_courses = CourseFactory.create_batch(3, semester=autumn_term) - enrolled_curr, unenrolled_curr, can_enroll_curr = autumn_courses - EnrollmentFactory(student=student, - student_profile=student_profile, - course=enrolled_curr) - EnrollmentFactory(student=student, - student_profile=student_profile, - course=unenrolled_curr, - is_deleted=True) - - client.login(student) - response = client.get(url) - terms_courses = list(response.context_data['courses'].values()) - founded_courses = sum(map(len, terms_courses)) - assert founded_courses == 1 - assert terms_courses[0][0]['name'] == enrolled_curr.meta_course.name + with mock.patch('django.utils.timezone.now', return_value=timezone.make_aware(datetime.datetime(2022, 10, 31))): + url = reverse('course_list', subdomain=settings.LMS_SUBDOMAIN) + future = now() + datetime.timedelta(days=3) + autumn_term = SemesterFactory.create_current(enrollment_period__ends_on=future.date()) + site = SiteFactory(id=settings.SITE_ID) + course_invitation = CourseInvitationFactory(course__semester=autumn_term) + student_profile = StudentProfileFactory(type=StudentTypes.INVITED, created = timezone.now()) + student = student_profile.user + complete_student_profile(student, site, course_invitation.invitation) + + autumn_courses = CourseFactory.create_batch(3, semester=autumn_term) + enrolled_curr, unenrolled_curr, can_enroll_curr = autumn_courses + EnrollmentFactory(student=student, + student_profile=student_profile, + course=enrolled_curr) + EnrollmentFactory(student=student, + student_profile=student_profile, + course=unenrolled_curr, + is_deleted=True) + + client.login(student) + response = client.get(url) + terms_courses = list(response.context_data['courses'].values()) + founded_courses = sum(map(len, terms_courses)) + assert founded_courses == 1 + assert terms_courses[0][0]['name'] == enrolled_curr.meta_course.name - response = client.get(course_invitation.invitation.get_absolute_url()) - assert response.status_code == 200 - response = client.get(url) - terms_courses = list(response.context_data['courses'].values()) - founded_courses = sum(map(len, terms_courses)) - assert founded_courses == 2 + response = client.get(course_invitation.invitation.get_absolute_url()) + assert response.status_code == 200 + response = client.get(url) + terms_courses = list(response.context_data['courses'].values()) + founded_courses = sum(map(len, terms_courses)) + assert founded_courses == 2 @pytest.mark.django_db @@ -272,48 +274,52 @@ def test_view_course_offerings_old_invited(client): @pytest.mark.django_db def test_view_course_offerings_regular_in_academic(client): - url = reverse('course_list', subdomain=settings.LMS_SUBDOMAIN) - future = now() + datetime.timedelta(days=3) - current_term = SemesterFactory.create_current(enrollment_period__ends_on=future.date()) - - regular_profile = StudentProfileFactory() - student = regular_profile.user - branch = regular_profile.branch - - course_enrolled, random_course = CourseFactory.create_batch(2, main_branch=branch) - enrollment = EnrollmentFactory(course=course_enrolled, - student=student, - student_profile=regular_profile, - grade=GradeTypes.UNSATISFACTORY) + with mock.patch('django.utils.timezone.now', return_value=timezone.make_aware(datetime.datetime(2022, 10, 31))): + url = reverse('course_list', subdomain=settings.LMS_SUBDOMAIN) + future = now() + datetime.timedelta(days=3) + current_term = SemesterFactory.create_current(enrollment_period__ends_on=future.date()) + + regular_profile = StudentProfileFactory(created=timezone.now()) + student = regular_profile.user + branch = regular_profile.branch + + course_enrolled, random_course = CourseFactory.create_batch(2, main_branch=branch) + enrollment = EnrollmentFactory(course=course_enrolled, + student=student, + student_profile=regular_profile, + grade=GradeTypes.UNSATISFACTORY) + + curator = CuratorFactory() + update_student_status(student_profile=regular_profile, + new_status=StudentStatuses.EXPELLED, + editor=curator) + + client.login(student) + response = client.get(url) + terms_courses = list(response.context_data['courses'].values()) + founded_courses = sum(map(len, terms_courses)) + # Expelled student still have access to courses + assert founded_courses == 2 - curator = CuratorFactory() - update_student_status(student_profile=regular_profile, - new_status=StudentStatuses.EXPELLED, - editor=curator) + site = SiteFactory(id=settings.SITE_ID) + course_invitation = CourseInvitationFactory(course__semester=current_term, course__main_branch=branch, + invitation__branch=branch) + complete_student_profile(student, site, course_invitation.invitation) + profile = student.get_student_profile() + profile.created = timezone.now() + profile.save() - client.login(student) - response = client.get(url) - terms_courses = list(response.context_data['courses'].values()) - founded_courses = sum(map(len, terms_courses)) - # Expelled student still have access to courses - assert founded_courses == 2 - - site = SiteFactory(id=settings.SITE_ID) - course_invitation = CourseInvitationFactory(course__semester=current_term, course__main_branch=branch, - invitation__branch=branch) - complete_student_profile(student, site, course_invitation.invitation) - - response = client.get(url) - terms_courses = list(response.context_data['courses'].values()) - founded_courses = sum(map(len, terms_courses)) - # Show only the course the student was enrolled in - assert founded_courses == 1 + response = client.get(url) + terms_courses = list(response.context_data['courses'].values()) + founded_courses = sum(map(len, terms_courses)) + # Show only the course the student was enrolled in + assert founded_courses == 1 - client.get(course_invitation.invitation.get_absolute_url()) - response = client.get(url) - terms_courses = list(response.context_data['courses'].values()) - founded_courses = sum(map(len, terms_courses)) - assert founded_courses == 2 + client.get(course_invitation.invitation.get_absolute_url()) + response = client.get(url) + terms_courses = list(response.context_data['courses'].values()) + founded_courses = sum(map(len, terms_courses)) + assert founded_courses == 2 @pytest.mark.django_db diff --git a/lms/views.py b/lms/views.py index 149a50bac4..26caae5a12 100644 --- a/lms/views.py +++ b/lms/views.py @@ -90,7 +90,7 @@ def get_queryset(self): "public_attachments_count", "meta_course__name", "meta_course__slug", "semester__year", "semester__index", "semester__type", - "main_branch__code", "main_branch__site_id") + "main_branch__code", "main_branch__site_id", "duration") .prefetch_related(course_teachers) .order_by('-semester__year', '-semester__index', 'meta_course__name'))