Listener classes rework (#954)

* SOI links issue

* add duration icon to MetaCourseDetailView

* Do not notify listeners

* rename filters

* almost there

* Done, untested

* обновление доки (капец она существует)

* do not notify on surveys

* fix dump to yandex disk year of curriculum

* add for_teacher filter

* tests

* update ci

* test fix

* update tests

.github/workflows/tests.yml

@@ -11,7 +11,7 @@ on:
       - 'docs/**'
     branches:
       - master
-    types: [review_requested, ready_for_review]
+    types: [opened, synchronize, reopened]
 
 concurrency:
   group: ${{ github.ref_name }}

apps/api/management/commands/dump_to_yandex_disk.py

@@ -6,7 +6,8 @@
 from django.db.models import Max
 from django.utils.translation import gettext_lazy as _
 
-from courses.models import Assignment
+from courses.constants import SemesterTypes
+from courses.models import Assignment, Semester
 from learning.models import StudentAssignment
 from users.models import StudentProfile, StudentTypes
 from api.models import ExternalServiceToken
@@ -26,9 +27,10 @@ def handle(self, *args, **options):
                                  _('Student Group'), _('Teacher'), _('Assignment'), _('Assignment status'), _('Assignment Grade'), 
                                  _('Maximum score'), _('Assignment count'), _('Maximum student score'), _('Grade'), _('Grade re-credited')])
 
-            current_year = datetime.datetime.now().year
+            current_semester = Semester.get_current()
+            current_curriculum_year = current_semester.year if current_semester.type == SemesterTypes.AUTUMN else current_semester.year - 1
             student_profiles = (StudentProfile.objects.filter(type__in=[StudentTypes.REGULAR, StudentTypes.PARTNER],
-                                                             year_of_curriculum__in=[current_year - 1, current_year])
+                                                             year_of_curriculum__in=[current_curriculum_year - 1, current_curriculum_year])
                                                         .select_related('user')
                                                         .prefetch_related('enrollment_set__course__assignment_set__studentassignment_set'))
             max_assignment_grades: dict[Assignment, int] = dict()

apps/courses/selectors.py

@@ -123,5 +123,5 @@ def course_personal_assignments(*, course: Course, filters: Optional[Dict[str, A
                               'assignee')
             .order_by())
 
-def course_active_personal_assignments(*, course: Course, filters: Optional[Dict[str, Any]] = None) -> StudentAssignmentQuerySet:
-    return course_personal_assignments(course=course, filters=filters).active()
+def course_personal_assignments_for_teachers(*, course: Course, filters: Optional[Dict[str, Any]] = None) -> StudentAssignmentQuerySet:
+    return course_personal_assignments(course=course, filters=filters).for_teachers()

apps/courses/views/course_class.py

@@ -179,7 +179,6 @@ def form_valid(self, form):
                             ))
                     for obj in objects:
                         for attachment_copy in attachment_copies:
-                            attachment_copy
                             CourseClassAttachment.objects.create(
                                 course_class=obj,
                                 material=attachment_copy

apps/courses/views/meta_course.py

@@ -3,7 +3,7 @@
 
 from auth.mixins import PermissionRequiredMixin
 from courses.forms import MetaCourseForm
-from courses.models import Course, MetaCourse
+from courses.models import Course, CourseDurations, MetaCourse
 from courses.permissions import EditMetaCourse
 
 __all__ = ('MetaCourseDetailView', 'MetaCourseUpdateView')
@@ -23,6 +23,7 @@ def get_context_data(self, **kwargs):
         context = {
             'meta_course': self.object,
             'courses': courses,
+            'CourseDurations': CourseDurations
         }
         return context
 

apps/learning/api/urls.py

@@ -11,7 +11,7 @@
             path('courses/<int:course_id>/assignments/', v.CourseAssignmentList.as_view(), name='course_assignments'),
             path('courses/<int:course_id>/enrollments/', v.CourseStudentsList.as_view(), name='course_enrollments'),
             path('courses/<int:course_id>/personal-assignments/', v.PersonalAssignmentList.as_view(), name='personal_assignments'),
-            path('courses/<int:course_id>/personal-assignments/active/', v.ActivePersonalAssignmentList.as_view(), name='active_personal_assignments'),
+            path('courses/<int:course_id>/personal-assignments/active/', v.TeacherPersonalAssignmentList.as_view(), name='active_personal_assignments'),
             path('courses/<int:course_id>/assignments/<int:assignment_id>/students/<int:student_id>/', v.StudentAssignmentUpdate.as_view(), name='my_course_student_assignment_update'),
             path('courses/<int:course_id>/assignments/<int:assignment_id>/students/<int:student_id>/assignee', v.StudentAssignmentAssigneeUpdate.as_view(), name='my_course_student_assignment_assignee_update'),
         ]))

apps/learning/api/views.py

@@ -21,7 +21,7 @@
 from core.http import AuthenticatedAPIRequest
 from courses.models import Assignment, Course
 from courses.permissions import CreateAssignment
-from courses.selectors import course_active_personal_assignments, course_personal_assignments, get_course_teachers
+from courses.selectors import course_personal_assignments_for_teachers, course_personal_assignments, get_course_teachers
 from learning.api.serializers import (
     BaseEnrollmentSerializer, BaseStudentAssignmentSerializer,
     CourseAssignmentSerializer, CourseNewsNotificationSerializer, MyCourseSerializer,
@@ -161,8 +161,9 @@ def get(self, request: AuthenticatedAPIRequest, **kwargs: Any):
         data = self.OutputSerializer(personal_assignments, many=True).data
         return Response(data)
 
-class ActivePersonalAssignmentList(PersonalAssignmentList):
-    personal_assignments_function = staticmethod(course_active_personal_assignments)
+class TeacherPersonalAssignmentList(PersonalAssignmentList):
+    # Just like regular, but includes also graded assignments of students who have left the course
+    personal_assignments_function = staticmethod(course_personal_assignments_for_teachers)
 
 
 class StudentAssignmentUpdate(UpdateAPIView):

apps/learning/gradebook/data.py

@@ -172,7 +172,7 @@ def gradebook_data(course: Course, student_group: Optional[int] = None) -> Grade
     enrolled_students = OrderedDict()
     course_enrollments = (Enrollment.active
                           .filter(course=course)
-                          .exclude(type=EnrollmentTypes.LECTIONS_ONLY))
+                          .can_submit_assignments())
     if student_group is not None:
         course_enrollments = course_enrollments.filter(student_group=student_group)
     enrollments = (course_enrollments

apps/learning/gradebook/tests.py

@@ -329,12 +329,14 @@ def test_empty_gradebook_view(client):
     assert len(students) == len(response.context_data['form'].fields)
     assert smart_bytes(_("Learners") + f": {len(students)}") in response.content
     assert smart_bytes(_("Total students") + f": {len(students)}") in response.content
+    assert smart_bytes(_("Recredited") + ": 0") in response.content
     assert smart_bytes(_("Listeners") + ": 0") in response.content
-    EnrollmentFactory.create(course=co1, type=EnrollmentTypes.LECTIONS_ONLY)
+    EnrollmentFactory.can_not_submit_assignments(course=co1)
     response = client.get(co1.get_gradebook_url())
     assert smart_bytes(_("Learners") + f": {len(students)}") in response.content
-    assert smart_bytes(_("Total students") + f": {len(students) + 1}") in response.content
+    assert smart_bytes(_("Total students") + f": {len(students) + 3}") in response.content
     assert smart_bytes(_("Listeners") + ": 1") in response.content
+    assert smart_bytes(_("Recredited") + ": 2") in response.content
     for co in [co1, co2]:
         url = co.get_gradebook_url()
         assert smart_bytes(url) in response.content

apps/learning/gradebook/views.py

@@ -49,7 +49,7 @@
     "ImportAssignmentScoresByYandexLoginView"
 ]
 
-from learning.settings import AssignmentScoreUpdateSource, EnrollmentGradeUpdateSource, EnrollmentTypes
+from learning.settings import AssignmentScoreUpdateSource, EnrollmentGradeUpdateSource, EnrollmentTypes, GradeTypes
 from users.models import StudentTypes, User
 
 
@@ -202,12 +202,15 @@ def get_context_data(self, form: BaseGradebookForm,
                    .select_related('semester', 'meta_course', 'main_branch'))
         context['course_offering_list'] = courses
         context['user_type'] = self.user_type
-        enrollments_summary = Enrollment.active.filter(course=self.course).aggregate(
-            total_listeners=Count('id', filter=Q(type=EnrollmentTypes.LECTIONS_ONLY)),
-            total_learners=Count('id', filter=Q(type=EnrollmentTypes.REGULAR))
+
+        is_recredited_filter = Q(grade=GradeTypes.RE_CREDIT) | Q(is_grade_recredited=True)
+        context.update(
+            Enrollment.active.filter(course=self.course).aggregate(
+                total_listeners=Count('id', filter=Q(type=EnrollmentTypes.LECTIONS_ONLY) & ~is_recredited_filter),
+                total_learners=Count('id', filter=Q(type=EnrollmentTypes.REGULAR) & ~is_recredited_filter),
+                total_recredited=Count('id', filter=is_recredited_filter)
+            )
         )
-        context['total_listeners'] = enrollments_summary['total_listeners']
-        context['total_learners'] = enrollments_summary['total_learners']
 
         return context
 

apps/learning/managers.py

@@ -1,12 +1,13 @@
 from django.apps import apps
 from django.conf import settings
 from django.db import models
-from django.db.models import query
+from django.db.models import Q, query
 from django.utils import timezone
 
 from core.db.models import LiveManager
 from django.db.models import OuterRef, Exists
 from core.utils import is_club_site
+from courses.constants import AssignmentStatus
 from learning.settings import EnrollmentTypes, GradeTypes
 
 
@@ -28,19 +29,23 @@ def with_future_deadline(self):
         """
         return self.filter(assignment__deadline_at__gt=timezone.now())
 
-    def active(self):
+    def can_be_submitted(self):
         # Have to do so to avoid cercular import:
         # learning.managers (this) -> learning.models(with Enrollment) -> learning.managers (with AssignmentCommentPublishedManager)
         Enrollment = apps.get_model('learning', 'Enrollment')
-        enrollment_subquery = Enrollment.active.filter(
+        enrollment_subquery = Enrollment.active.can_submit_assignments().filter(
                             student=OuterRef('student'),
-                            course=OuterRef('assignment__course'),
-                            grade__ne=GradeTypes.RE_CREDIT,
-                            is_grade_recredited=False,
-                            type__ne=EnrollmentTypes.LECTIONS_ONLY
+                            course=OuterRef('assignment__course')
                             )
+        return self.filter(Exists(enrollment_subquery))
 
-        return self.annotate(active=Exists(enrollment_subquery)).filter(active=True)
+    def for_teachers(self):
+        Enrollment = apps.get_model('learning', 'Enrollment')
+        enrollment_subquery = Enrollment.active.can_submit_assignments().filter(
+                            student=OuterRef('student'),
+                            course=OuterRef('assignment__course')
+                            )
+        return self.filter(Q(Exists(enrollment_subquery)) | Q(status__in=[AssignmentStatus.NEED_FIXES, AssignmentStatus.COMPLETED]))
 
 
 class _StudentAssignmentDefaultManager(LiveManager):
@@ -79,7 +84,13 @@ def get_queryset(self):
 
 
 class EnrollmentQuerySet(models.QuerySet):
-    pass
+
+    def can_submit_assignments(self):
+        return self.filter(
+                    grade__ne=GradeTypes.RE_CREDIT,
+                    is_grade_recredited=False,
+                    type__ne=EnrollmentTypes.LECTIONS_ONLY
+                    )
 
 
 EnrollmentDefaultManager = _EnrollmentDefaultManager.from_queryset(

apps/learning/models.py

@@ -815,6 +815,10 @@ def weighted_final_score(self) -> Optional[Decimal]:
 
     def get_execution_time_display(self):
         return humanize_duration(self.execution_time)
+
+    @property
+    def can_be_submitted(self):
+        return self.__class__.objects.can_be_submitted().filter(pk=self.pk).exists()
 
 
 class AssignmentScoreAuditLog(TimestampedModel):
@@ -922,6 +926,10 @@ def save(self, **kwargs):
         if has_been_published:
             handle_submission_assignee_and_notifications.delay(
                 assignment_submission_id=self.pk)
+
+    def clean(self):
+        if self.pk and not self.student_assignment.can_be_submitted:
+            raise ValidationError(_("Can not add comment to unactive assignment"))
 
     def created_local(self, tz=None):
         if not tz:
@@ -962,6 +970,7 @@ def get_execution_time_display(self) -> str:
         return s
 
 
+
 def assignment_submission_attachment_upload_to(self: "SubmissionAttachment",
                                                filename) -> str:
     sa = self.submission.student_assignment

apps/learning/permissions.py

@@ -390,6 +390,11 @@ def rule(user, assignment: Assignment):
 @add_perm
 class CreateAssignmentComment(Permission):
     name = "learning.create_assignment_comment"
+
+    @staticmethod
+    @rules.predicate
+    def rule(user, student_assignment: StudentAssignment):
+        return student_assignment and student_assignment.can_be_submitted
 
 
 @add_perm
@@ -402,7 +407,7 @@ def rule(user, student_assignment: StudentAssignment):
         student_profile = user.get_student_profile()
         if student_profile.status in StudentStatuses.inactive_statuses:
             return False
-        return student_assignment.student_id == user.id
+        return CreateAssignmentComment.rule(user, student_assignment) and student_assignment.student_id == user.id
 
 
 @add_perm
@@ -413,7 +418,7 @@ class CreateAssignmentCommentAsTeacher(Permission):
     @rules.predicate
     def rule(user, student_assignment: StudentAssignment):
         course = student_assignment.assignment.course
-        return course.is_actual_teacher(user.pk)
+        return CreateAssignmentComment.rule(user, student_assignment) and course.is_actual_teacher(user.pk)
 
 
 @add_perm

apps/learning/services/assignment_service.py

@@ -72,10 +72,7 @@ def bulk_create_student_assignments(cls, assignment: Assignment,
         """
         filters = [
             Q(course_id=assignment.course_id),
-            ~Q(student_profile__status__in=StudentStatuses.inactive_statuses),
-            ~Q(grade=GradeTypes.RE_CREDIT),
-            Q(is_grade_recredited=False),
-            ~Q(type=EnrollmentTypes.LECTIONS_ONLY)     
+            ~Q(student_profile__status__in=StudentStatuses.inactive_statuses)
         ]
         restrict_to = list(sg.pk for sg in assignment.restricted_to.all())
         # Filter out enrollments not in the targeted course groups
@@ -118,7 +115,7 @@ def bulk_create_student_assignments(cls, assignment: Assignment,
         # TODO: move to the separated method
         # Generate notifications
         to_notify = [sid for sid in students if sid not in already_exist]
-        created = (StudentAssignment.objects
+        created = (StudentAssignment.objects.can_be_submitted()
                    .filter(assignment=assignment, student_id__in=to_notify)
                    .values_list('pk', 'student_id', named=True))
         objs = (notify_student_new_assignment(sa, commit=False) for sa

apps/learning/signals.py

@@ -63,7 +63,7 @@ def create_notifications_about_course_news(sender, instance: CourseNews,
         return
     co_id = instance.course_id
     notifications = []
-    active_enrollments = Enrollment.active.filter(course_id=co_id)
+    active_enrollments = Enrollment.active.can_submit_assignments().filter(course_id=co_id)
     for e in active_enrollments.iterator():
         notifications.append(
             CourseNewsNotification(user_id=e.student_id,
@@ -85,7 +85,7 @@ def create_deadline_change_notification(sender, instance, created,
         active_enrollments = Enrollment.active.filter(course=instance.course)
         for e in active_enrollments:
             try:
-                sa = (StudentAssignment.objects
+                sa = (StudentAssignment.objects.can_be_submitted()
                       .only('pk')
                       .get(student_id=e.student_id,
                            assignment=instance))
@@ -94,7 +94,7 @@ def create_deadline_change_notification(sender, instance, created,
                                         is_about_deadline=True)
                  .save())
             except StudentAssignment.DoesNotExist:
-                # It can occur for student with inactive status
+                # It can occur for student with inactive status or if assignment can not be submitted
                 continue
 
 

apps/learning/study/tests/test_misc.py

@@ -92,6 +92,27 @@ def test_view_student_assignment_as_regular_student(client):
     client.login(student)
     response = client.get(new_student_assignment.get_student_url())
     assert response.status_code == 200
+    assert "comment_form" in response.context_data
+    assert "solution_form" in response.context_data
+
+@pytest.mark.django_db
+def test_view_student_assignment_can_not_submit(client):
+    teacher = TeacherFactory()
+    current_semester = SemesterFactory.create_current()
+    course = CourseFactory(teachers=[teacher], semester=current_semester)
+    EnrollmentFactory.can_not_submit_assignments(course=course)
+    AssignmentFactory(course=course)
+    for student_assignment in StudentAssignment.objects.all():
+        client.force_login(student_assignment.student)
+        response = client.get(student_assignment.get_student_url())
+        assert response.status_code == 200
+        assert "comment_form" not in response.context_data
+        assert "solution_form" not in response.context_data
+        client.login(teacher)
+        response = client.get(student_assignment.get_teacher_url())
+        assert response.status_code == 200
+        assert "comment_form" not in response.context_data
+        assert "solution_form" not in response.context_data
 
 
 @pytest.mark.django_db
@@ -281,12 +302,7 @@ def test_view_assignment_list(client):
     assert len(response.context_data['assignment_list_archive']) == 0
     # Enroll in the course
     EnrollmentFactory(student=student, course=course)
-    unactive_kwargs = [
-        {"grade": GradeTypes.RE_CREDIT},
-        {"is_grade_recredited": True},
-        {"type": EnrollmentTypes.LECTIONS_ONLY}
-    ]
-    unactive_enrollments = [EnrollmentFactory(course=course, **kwargs) for kwargs in unactive_kwargs]
+    EnrollmentFactory.can_not_submit_assignments(course=course)
     response = client.get(url)
     assert len(response.context_data['assignment_list_open']) == 2
     assert len(response.context_data['assignment_list_archive']) == 0