Removed question.courses field (#2741)

* Dropped always-on homework permissions featureflag
* Answer model permissions refactoring
* Dropped legacy multi-course questions
* Question breadcrumbs code cleanup
* Simpler syntax for User.add_perm(), now it uses the same perm path as User.has_perm()

Author: f213

conftest.py

@@ -14,6 +14,7 @@
     "apps.products.fixtures",
     "apps.users.fixtures",
     "apps.lms.factory",
+    "apps.homework.factory",
     "apps.dashamail.fixtures",
     "core.fixtures",
 ]

src/apps/homework/admin/question/admin.py

@@ -13,11 +13,9 @@
 class QuestionAdmin(ModelAdmin):
     list_display = [
         "name",
-        "courses_list",
+        "course",
     ]
     fields = [
-        "courses",
-        "course",
         "module",
         "lesson",
         "name",
@@ -30,11 +28,16 @@ class QuestionAdmin(ModelAdmin):
     save_as = True
     form = QuestionForm
 
-    def get_queryset(self, request: HttpRequest) -> QuerySet[Question]:
-        return super().get_queryset(request).for_admin()  # type: ignore
+    @admin.display(description=_("Course"))
+    def course(self, question: Question) -> str:
+        lesson = question.lesson_set.select_related("module", "module__course", "module__course__group").first()
+        if lesson is None:
+            return "-"
 
-    def courses_list(self, question: Question) -> str:
-        return ", ".join([course.name for course in question.courses.all()])
+        return str(lesson.module.course)
+
+    def get_queryset(self, request: HttpRequest) -> QuerySet[Question]:
+        return super().get_queryset(request).prefetch_related("lesson_set")
 
     @admin.action(description=_("Dispatch crosscheck"))
     def dispatch_crosscheck(self, request: Request, queryset: QuerySet) -> None:

src/apps/homework/admin/question/form.py

@@ -4,24 +4,20 @@
 
 from apps.homework.models import Question
 from apps.lms.models import Lesson, Module
-from apps.products.models import Course
 from core.admin.forms import ModelForm
 from core.current_user import get_current_user
 from core.tasks import write_admin_log
 
 
 class QuestionForm(ModelForm):
-    course = forms.ModelChoiceField(label=_("Course"), queryset=Course.objects.for_admin(), required=False)
     module = forms.ModelChoiceField(label=_("Module"), queryset=Module.objects.for_admin(), required=False)
     lesson = forms.ModelChoiceField(label=_("Lesson"), queryset=Lesson.objects.for_admin(), required=False)
 
     class Meta:
         model = Question
         fields = [
-            "courses",
             "name",
             "deadline",
-            "course",
             "module",
             "lesson",
             "text",

src/apps/homework/api/permissions.py

@@ -1,58 +1,21 @@
-from typing import Any
-
-from django.conf import settings
 from rest_framework import permissions
 from rest_framework.request import Request
 from rest_framework.views import APIView
 
-from apps.homework.models import Question
-
-
-def get_all_purcased_user_ids(question: Question) -> frozenset[int]:
-    user_ids: set[int] = set()
-    for course in question.courses.all():
-        user_ids.update(course.get_purchased_users().values_list("id", flat=True))
-
-    return frozenset(user_ids)
-
-
-class ShouldHavePurchasedCoursePermission(permissions.BasePermission):
-    def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool:
-        return (
-            request.user.has_perm("homework.see_all_questions")
-            or settings.DISABLE_HOMEWORK_PERMISSIONS_CHECKING
-            or (request.user.id in get_all_purcased_user_ids(obj))
-        )
-
+from apps.homework.models import Answer, Reaction
 
-class ShouldHavePurchasedQuestionCoursePermission(permissions.BasePermission):
-    def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool:
-        return (
-            request.user.has_perm("homework.see_all_questions")
-            or settings.DISABLE_HOMEWORK_PERMISSIONS_CHECKING
-            or (request.user.id in get_all_purcased_user_ids(obj.question))
-        )
 
-
-class ShouldBeAuthorOrReadOnly(permissions.BasePermission):
-    def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool:
+class AuthorOrReadonly(permissions.BasePermission):
+    def has_object_permission(self, request: Request, view: APIView, obj: Answer | Reaction) -> bool:
         if request.method in permissions.SAFE_METHODS:
             return True
 
         return obj.author == request.user
 
 
-class AnswerShouldBeEditable(permissions.BasePermission):
-    def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool:
-        if request.method in permissions.SAFE_METHODS:
-            return True
-
-        return obj.is_editable
-
-
-class MayChangeAnswerOnlyWithoutDescendants(permissions.BasePermission):
-    def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool:
+class IsEditable(permissions.BasePermission):
+    def has_object_permission(self, request: Request, view: APIView, answer: Answer) -> bool:
         if request.method in permissions.SAFE_METHODS:
             return True
 
-        return obj.get_first_level_descendants().count() == 0
+        return answer.is_editable

src/apps/homework/api/serializers/question.py

@@ -40,24 +40,19 @@ class Meta:
 
     @extend_schema_field(lazy_serializer("apps.lms.api.serializers.LMSCourseSerializer")())
     def get_course(self, question: Question) -> dict:
-        from apps.homework.breadcrumbs import get_lesson
         from apps.lms.api.serializers import LMSCourseSerializer
 
-        lesson = get_lesson(question, user=self.context["request"].user)
+        course = question.get_course(user=self.context["request"].user)
 
-        course = question.courses.first()
-        if lesson is not None:
-            course = lesson.module.course
+        if course is None:
+            course = question.get_legacy_course()
 
         return LMSCourseSerializer(course).data
 
     @extend_schema_field(lazy_serializer("apps.lms.api.serializers.BreadcrumbsSerializer")())
     def get_breadcrumbs(self, question: Question) -> dict | None:
-        from apps.homework.breadcrumbs import get_lesson
         from apps.lms.api.serializers import BreadcrumbsSerializer
 
-        lesson = get_lesson(question, user=self.context["request"].user)
-        if lesson is None:
-            return None
+        lesson = question.get_lesson(user=self.context["request"].user)
 
-        return BreadcrumbsSerializer(lesson).data
+        return BreadcrumbsSerializer(lesson).data if lesson is not None else None

src/apps/homework/api/views.py

@@ -4,23 +4,18 @@
 
 from apps.homework.api import serializers
 from apps.homework.api.filtersets import AnswerCommentFilterSet, AnswerCrossCheckFilterSet
-from apps.homework.api.permissions import ShouldHavePurchasedCoursePermission
 from apps.homework.api.serializers import CrossCheckSerializer
 from apps.homework.models import Answer, AnswerCrossCheck, AnswerImage, Question
 
 
 class QuestionView(generics.RetrieveAPIView):
     queryset = Question.objects.all()
     serializer_class = serializers.QuestionDetailSerializer
-    permission_classes = [ShouldHavePurchasedCoursePermission]
     lookup_field = "slug"
 
     def get_queryset(self) -> QuerySet[Question]:
         queryset = super().get_queryset()
 
-        if self.request.user.is_anonymous:
-            return queryset.none()
-
         return queryset.for_user(self.request.user)  # type: ignore
 
 

src/apps/homework/api/viewsets.py

@@ -4,18 +4,16 @@
 from django.utils.decorators import method_decorator
 from django.utils.functional import cached_property
 from drf_spectacular.utils import extend_schema
-from rest_framework.exceptions import MethodNotAllowed
+from rest_framework.exceptions import MethodNotAllowed, PermissionDenied
 from rest_framework.generics import get_object_or_404
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.request import Request
 from rest_framework.response import Response
 
 from apps.homework.api.filtersets import AnswerFilterSet
 from apps.homework.api.permissions import (
-    AnswerShouldBeEditable,
-    MayChangeAnswerOnlyWithoutDescendants,
-    ShouldBeAuthorOrReadOnly,
-    ShouldHavePurchasedQuestionCoursePermission,
+    AuthorOrReadonly,
+    IsEditable,
 )
 from apps.homework.api.serializers import (
     AnswerCreateSerializer,
@@ -25,12 +23,13 @@
     ReactionCreateSerializer,
     ReactionDetailedSerializer,
 )
-from apps.homework.models import Answer
+from apps.homework.models import Answer, Question
 from apps.homework.models.answer import AnswerQuerySet
 from apps.homework.models.reaction import Reaction
 from apps.homework.services import ReactionCreator
 from apps.homework.services.answer_creator import AnswerCreator
 from apps.homework.services.answer_remover import AnswerRemover
+from apps.users.models import User
 from core.api.mixins import DisablePaginationWithQueryParamMixin
 from core.viewsets import AppViewSet, CreateDeleteAppViewSet
 
@@ -60,17 +59,15 @@ class AnswerViewSet(DisablePaginationWithQueryParamMixin, AppViewSet):
 
     lookup_field = "slug"
     permission_classes = [
-        IsAuthenticated
-        & ShouldHavePurchasedQuestionCoursePermission
-        & ShouldBeAuthorOrReadOnly
-        & AnswerShouldBeEditable
-        & MayChangeAnswerOnlyWithoutDescendants,
+        IsAuthenticated & AuthorOrReadonly & IsEditable,
     ]
     filterset_class = AnswerFilterSet
 
     @extend_schema(request=AnswerCreateSerializer, responses=AnswerTreeSerializer)
     def create(self, request: Request, *args: Any, **kwargs: dict[str, Any]) -> Response:
         """Create an answer"""
+        self._check_question_permissions(user=self.user, question_slug=request.data["question"])
+
         answer = AnswerCreator(
             question_slug=request.data["question"],
             parent_slug=request.data.get("parent"),
@@ -119,9 +116,9 @@ def get_queryset(self) -> AnswerQuerySet:
         return queryset.with_children_count().order_by("created").prefetch_reactions()
 
     def limit_queryset_to_user(self, queryset: AnswerQuerySet) -> AnswerQuerySet:
-        if self.action != "retrieve" and not self.request.user.has_perm("homework.see_all_answers"):
+        if self.action != "retrieve" and not self.user.has_perm("homework.see_all_answers"):
             # Each user may access any answer knowing its slug
-            return queryset.for_user(self.request.user)  # type: ignore
+            return queryset.for_user(self.user)
 
         return queryset
 
@@ -131,14 +128,23 @@ def limit_queryset_for_list(self, queryset: AnswerQuerySet) -> AnswerQuerySet:
 
         return queryset
 
+    @staticmethod
+    def _check_question_permissions(user: User, question_slug: str) -> None:
+        if not Question.objects.for_user(user).filter(slug=question_slug).exists():
+            raise PermissionDenied()
+
+    @property
+    def user(self) -> User:
+        return self.request.user  # type: ignore
+
 
 class ReactionViewSet(CreateDeleteAppViewSet):
     queryset = Reaction.objects.for_viewset()
     serializer_class = ReactionDetailedSerializer
     serializer_action_classes = {
         "create": ReactionCreateSerializer,
     }
-    permission_classes = [IsAuthenticated & ShouldBeAuthorOrReadOnly]
+    permission_classes = [IsAuthenticated & AuthorOrReadonly]
 
     lookup_field = "slug"
 

src/apps/homework/breadcrumbs.py

@@ -0,0 +1,14 @@
+from apps.homework.models import Question
+from apps.products.models import Course
+from core.test.factory import FixtureFactory, register
+
+
+@register
+def question(self: FixtureFactory, course: Course | None = None, **kwargs: dict) -> Question:
+    question = self.mixer.blend("homework.Question", _legacy_course=None, **kwargs)
+
+    if course is not None:
+        module = self.module(course=course)
+        self.lesson(module=module, question=question)
+
+    return question

src/apps/homework/factory.py

@@ -0,0 +1,32 @@
+# Generated by Django 4.2.23 on 2025-08-18 10:28
+import django.contrib.postgres.fields
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+# type: ignore
+def populate_legacy_course_id(apps, schema_editor):  # NOQA: ARG001
+    for question in apps.get_model("homework.Question").objects.all():
+        course_ids = question.courses.values_list("id", flat=True)
+        if course_ids.exists():
+            question._legacy_courses = list(course_ids)
+            question.save(update_fields=["_legacy_courses"])
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("homework", "0023_answer_indexes"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="question",
+            name="_legacy_courses",
+            field=django.contrib.postgres.fields.ArrayField(base_field=models.PositiveIntegerField(), blank=True, null=True, size=None),
+        ),
+        migrations.RunPython(populate_legacy_course_id),
+        migrations.RemoveField(
+            model_name="question",
+            name="courses",
+        ),
+    ]