diff --git a/backend/reviews/adapters.py b/backend/reviews/adapters.py new file mode 100644 index 0000000000..155d8dd48b --- /dev/null +++ b/backend/reviews/adapters.py @@ -0,0 +1,754 @@ +""" +Review adapters for different review session types. + +This module implements an adapter pattern to separate the handling of different +review types (Proposals and Grants) in the review system. Each adapter encapsulates +the specific logic for its review type, making the code more maintainable and testable. + +Usage: + adapter = get_review_adapter(review_session) + next_item = adapter.get_next_to_review_item_id(review_session, user) +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Protocol + +from django.contrib.postgres.expressions import ArraySubquery +from django.db.models import ( + Avg, + Count, + Exists, + F, + FloatField, + OuterRef, + Prefetch, + Q, + StdDev, + Subquery, + Sum, +) +from django.db.models.expressions import ExpressionWrapper +from django.db.models.functions import Cast +from django.http import HttpRequest +from django.urls import reverse + +from custom_admin.audit import ( + create_addition_admin_log_entry, + create_change_admin_log_entry, + create_deletion_admin_log_entry, +) +from grants.models import Grant, GrantReimbursement, GrantReimbursementCategory +from participants.models import Participant +from submissions.models import Submission, SubmissionTag + +if TYPE_CHECKING: + from django.contrib.admin import AdminSite + from django.db.models import QuerySet + + from reviews.models import ReviewSession, UserReview + from users.models import User + + +class ReviewAdapter(Protocol): + """ + Protocol defining the interface for review type adapters. + + Each review type (Proposals, Grants) implements this protocol to provide + its specific behavior for the review system. + """ + + @property + def recap_template(self) -> str: + """Return the template name for the recap view.""" + ... + + @property + def review_template(self) -> str: + """Return the template name for the individual review view.""" + ... + + def get_recap_items_queryset( + self, + review_session: ReviewSession, + ) -> QuerySet: + """ + Get the queryset of items to display in the recap view. + + The queryset should be annotated with scoring information and + ordered appropriately for the recap display. + """ + ... + + def get_recap_context( + self, + request: HttpRequest, + review_session: ReviewSession, + items: QuerySet, + admin_site: AdminSite, + ) -> dict[str, Any]: + """ + Get additional context for the recap template. + + Returns a dictionary with template context specific to this review type. + """ + ... + + def process_recap_post( + self, + request: HttpRequest, + review_session: ReviewSession, + ) -> None: + """ + Process POST data from the recap form. + + Handles saving decisions and any other recap form submissions. + """ + ... + + def get_review_context( + self, + request: HttpRequest, + review_session: ReviewSession, + review_item_id: int, + user_review: UserReview | None, + admin_site: AdminSite, + ) -> dict[str, Any]: + """ + Get context for the individual item review template. + + Returns a dictionary with all context needed to render the review view. + """ + ... + + def get_next_to_review_item_id( + self, + review_session: ReviewSession, + user: User, + skip_item: int | None = None, + exclude: list[int] | None = None, + seen: list[int] | None = None, + ) -> int | None: + """ + Get the ID of the next item to review. + + Returns the ID of an unreviewed item, prioritizing items with fewer votes. + Returns None if no items are available to review. + """ + ... + + def get_user_review_filter(self, review_item_id: int) -> dict[str, Any]: + """ + Get filter kwargs for finding a user's review of an item. + + Used when looking up existing reviews for a specific item. + """ + ... + + def get_user_review_create_values(self, review_item_id: int) -> dict[str, Any]: + """ + Get values dict for creating/updating a user review. + + Used when saving a new review or updating an existing one. + """ + ... + + +class ProposalsReviewAdapter: + """ + Adapter for handling Proposals (Submissions) reviews. + + Handles all the specific logic for reviewing conference talk/workshop + submissions, including tag filtering and speaker information display. + """ + + @property + def recap_template(self) -> str: + return "proposals-recap.html" + + @property + def review_template(self) -> str: + return "proposal-review.html" + + def get_recap_items_queryset( + self, + review_session: ReviewSession, + ) -> QuerySet[Submission]: + from reviews.models import UserReview + + review_session_id = review_session.id + + return ( + Submission.objects.for_conference(review_session.conference_id) + .non_cancelled() + .annotate( + score=Subquery( + UserReview.objects.select_related("score") + .filter( + review_session_id=review_session_id, + proposal_id=OuterRef("id"), + ) + .values("proposal_id") + .annotate(score=Avg("score__numeric_value")) + .values("score") + ) + ) + .order_by(F("score").desc(nulls_last=True)) + .prefetch_related( + Prefetch( + "userreview_set", + queryset=UserReview.objects.prefetch_related( + "user", "score" + ).filter(review_session_id=review_session_id), + ), + "duration", + "audience_level", + "languages", + "speaker", + "tags", + "type", + "rankings", + "rankings__tag", + ) + ) + + def get_recap_context( + self, + request: HttpRequest, + review_session: ReviewSession, + items: QuerySet, + admin_site: AdminSite, + ) -> dict[str, Any]: + conference = review_session.conference + speakers_ids = items.values_list("speaker_id", flat=True) + + grants = { + str(grant.user_id): grant + for grant in Grant.objects.filter( + conference=conference, user_id__in=speakers_ids + ) + } + + return dict( + admin_site.each_context(request), + items=items, + grants=grants, + review_session_id=review_session.id, + audience_levels=conference.audience_levels.all(), + review_session_repr=str(review_session), + all_statuses=[choice for choice in Submission.STATUS], + title="Recap", + ) + + def process_recap_post( + self, + request: HttpRequest, + review_session: ReviewSession, + ) -> None: + conference = review_session.conference + data = request.POST + + decisions = { + int(key.split("-")[1]): value + for key, value in data.items() + if key.startswith("decision-") + } + + proposals = list(conference.submissions.filter(id__in=decisions.keys())) + + for proposal in proposals: + decision = decisions[proposal.id] + proposal.pending_status = decision + + Submission.objects.bulk_update( + proposals, + fields=["pending_status"], + ) + + def get_review_context( + self, + request: HttpRequest, + review_session: ReviewSession, + review_item_id: int, + user_review: UserReview | None, + admin_site: AdminSite, + ) -> dict[str, Any]: + from reviews.models import AvailableScoreOption, UserReview + + proposal = ( + Submission.objects.for_conference(review_session.conference_id) + .prefetch_related( + "rankings", + "rankings__tag", + Prefetch( + "userreview_set", + queryset=UserReview.objects.prefetch_related( + "user", "score" + ).filter(review_session_id=review_session.id), + ), + ) + .get(id=review_item_id) + ) + + languages = list(proposal.languages.all()) + speaker = proposal.speaker + grant = ( + Grant.objects.of_user(proposal.speaker_id) + .for_conference(proposal.conference_id) + .first() + ) + grant_link = ( + reverse("admin:grants_grant_change", args=(grant.id,)) if grant else "" + ) + + existing_comment = request.GET.get("comment", "") + tags_already_excluded = request.GET.get("exclude", "").split(",") + + used_tags = ( + Submission.objects.filter( + conference_id=proposal.conference_id, + ) + .values_list("tags__id", flat=True) + .distinct() + ) + + tags_to_filter = SubmissionTag.objects.filter(id__in=used_tags).order_by("name") + + return dict( + admin_site.each_context(request), + proposal=proposal, + languages=proposal.languages.all(), + available_scores=AvailableScoreOption.objects.filter( + review_session_id=review_session.id + ).order_by("-numeric_value"), + proposal_id=review_item_id, + review_session_id=review_session.id, + user_review=user_review, + has_italian_language=any( + language for language in languages if language.code == "it" + ), + has_english_language=any( + language for language in languages if language.code == "en" + ), + speaker=speaker, + grant=grant, + grant_link=grant_link, + participant=Participant.objects.filter( + user_id=proposal.speaker_id, + conference=proposal.conference, + ).first(), + tags_to_filter=tags_to_filter, + tags_already_excluded=tags_already_excluded, + seen=request.GET.get("seen", "").split(","), + existing_comment=existing_comment, + review_session_repr=str(review_session), + title=f"Proposal Review: {proposal.title.localize('en')}", + ) + + def get_next_to_review_item_id( + self, + review_session: ReviewSession, + user: User, + skip_item: int | None = None, + exclude: list[int] | None = None, + seen: list[int] | None = None, + ) -> int | None: + from reviews.models import UserReview + + exclude = exclude or [] + seen = seen or [] + + already_reviewed_ids = UserReview.objects.filter( + user_id=user.id, + review_session_id=review_session.id, + ).values_list("proposal_id", flat=True) + + skip_item_array = [skip_item] if skip_item else [] + seen_items_to_ignore = list(already_reviewed_ids) + skip_item_array + seen + + qs = ( + Submission.objects.non_cancelled() + .for_conference(review_session.conference_id) + .annotate( + votes_received=Count( + "userreview", + filter=Q(userreview__review_session_id=review_session.id), + ) + ) + .order_by("votes_received", "?") + ) + + if seen_items_to_ignore: + qs = qs.exclude(id__in=seen_items_to_ignore) + + if exclude: + qs = qs.exclude(tags__in=exclude) + + unvoted_item = qs.first() + return unvoted_item.id if unvoted_item else None + + def get_user_review_filter(self, review_item_id: int) -> dict[str, Any]: + return {"proposal_id": review_item_id} + + def get_user_review_create_values(self, review_item_id: int) -> dict[str, Any]: + return {"proposal_id": review_item_id} + + +class GrantsReviewAdapter: + """ + Adapter for handling Grants (Financial Aid) reviews. + + Handles all the specific logic for reviewing financial aid applications, + including reimbursement categories and internal notes management. + """ + + @property + def recap_template(self) -> str: + return "grants-recap.html" + + @property + def review_template(self) -> str: + return "grant-review.html" + + def get_recap_items_queryset( + self, + review_session: ReviewSession, + ) -> QuerySet[Grant]: + from reviews.models import UserReview + + review_session_id = review_session.id + + return ( + review_session.conference.grants.annotate( + total_score=Cast( + Sum( + "userreview__score__numeric_value", + filter=Q(userreview__review_session_id=review_session_id), + ), + output_field=FloatField(), + ), + vote_count=Cast( + Count( + "userreview", + filter=Q(userreview__review_session_id=review_session_id), + ), + output_field=FloatField(), + ), + score=ExpressionWrapper( + F("total_score") / F("vote_count"), + output_field=FloatField(), + ), + std_dev=StdDev( + "userreview__score__numeric_value", + filter=Q(userreview__review_session_id=review_session_id), + ), + has_sent_a_proposal=Exists( + Submission.objects.non_cancelled().filter( + speaker_id=OuterRef("user_id"), + conference_id=review_session.conference_id, + ) + ), + proposals_ids=ArraySubquery( + Submission.objects.non_cancelled() + .filter( + speaker_id=OuterRef("user_id"), + conference_id=review_session.conference_id, + ) + .values("id") + ), + approved_reimbursement_category_ids=ArraySubquery( + GrantReimbursement.objects.filter( + grant_id=OuterRef("pk") + ).values_list("category_id", flat=True) + ), + ) + .order_by(F("score").desc(nulls_last=True)) + .prefetch_related( + Prefetch( + "userreview_set", + queryset=UserReview.objects.prefetch_related( + "user", "score" + ).filter(review_session_id=review_session_id), + ), + "user", + ) + ) + + def get_recap_context( + self, + request: HttpRequest, + review_session: ReviewSession, + items: QuerySet, + admin_site: AdminSite, + ) -> dict[str, Any]: + proposals = { + submission.id: submission + for submission in Submission.objects.non_cancelled() + .filter( + conference_id=review_session.conference_id, + speaker_id__in=items.values_list("user_id"), + ) + .prefetch_related("rankings", "rankings__tag") + } + + return dict( + admin_site.each_context(request), + request=request, + items=items, + proposals=proposals, + review_session_id=review_session.id, + review_session_repr=str(review_session), + all_review_statuses=[ + choice + for choice in Grant.Status.choices + if choice[0] in Grant.REVIEW_SESSION_STATUSES_OPTIONS + ], + all_statuses=Grant.Status.choices, + all_reimbursement_categories=GrantReimbursementCategory.objects.for_conference( + conference=review_session.conference + ), + review_session=review_session, + title="Recap", + ) + + def process_recap_post( + self, + request: HttpRequest, + review_session: ReviewSession, + ) -> None: + data = request.POST + + reimbursement_categories = { + category.id: category + for category in GrantReimbursementCategory.objects.for_conference( + conference=review_session.conference + ) + } + + decisions = { + int(key.split("-")[1]): value + for key, value in data.items() + if key.startswith("decision-") + } + + approved_reimbursement_categories_decisions = { + int(key.split("-")[1]): [int(id_) for id_ in data.getlist(key)] + for key in data.keys() + if key.startswith("reimbursementcategory-") + } + + notes_updates = { + int(key.split("-")[1]): value + for key, value in data.items() + if key.startswith("notes-") + } + + grants = list(review_session.conference.grants.filter(id__in=decisions.keys())) + + # Track grants with pending status changes for audit logging + grants_with_pending_status_changes = {} + + for grant in grants: + decision = decisions[grant.id] + if decision not in Grant.REVIEW_SESSION_STATUSES_OPTIONS: + continue + + original_pending_status = grant.pending_status + if decision != grant.status: + grant.pending_status = decision + elif decision == grant.status: + grant.pending_status = None + + if grant.pending_status != original_pending_status: + grants_with_pending_status_changes[grant.id] = original_pending_status + + # Handle reimbursement deletions + if grant.reimbursements.exists(): + approved_reimbursement_categories = ( + approved_reimbursement_categories_decisions.get(grant.id, []) + ) + if decision != Grant.Status.approved: + # Delete all reimbursements if not approved + for reimbursement in grant.reimbursements.all(): + create_deletion_admin_log_entry( + request.user, + grant, + change_message=f"[Review Session] Reimbursement removed: {reimbursement.category.name}.", + ) + reimbursement.delete() + else: + # Only keep those in current approved categories + to_delete = grant.reimbursements.exclude( + category_id__in=approved_reimbursement_categories + ) + for reimbursement in to_delete: + create_deletion_admin_log_entry( + request.user, + grant, + change_message=f"[Review Session] Reimbursement removed: {reimbursement.category.name}.", + ) + to_delete.delete() + + # Save grants and create audit logs + for grant in grants: + grant.save(update_fields=["pending_status"]) + + if grant.id in grants_with_pending_status_changes: + original_pending_status = grants_with_pending_status_changes[grant.id] + create_change_admin_log_entry( + request.user, + grant, + change_message=f"[Review Session] Pending status changed from '{original_pending_status}' to '{grant.pending_status}'.", + ) + + # Skip reimbursement creation if not approved + if grant.pending_status != Grant.Status.approved: + continue + + approved_reimbursement_categories = ( + approved_reimbursement_categories_decisions.get(grant.id, []) + ) + + for reimbursement_category_id in approved_reimbursement_categories: + if reimbursement_category_id not in reimbursement_categories: + continue + + reimbursement, created = GrantReimbursement.objects.update_or_create( + grant=grant, + category_id=reimbursement_category_id, + defaults={ + "granted_amount": reimbursement_categories[ + reimbursement_category_id + ].max_amount + }, + ) + + if created: + create_addition_admin_log_entry( + request.user, + grant, + change_message=f"[Review Session] Reimbursement {reimbursement.category.name} added.", + ) + + # Update internal notes + if notes_updates: + grants_to_update_notes = review_session.conference.grants.filter( + id__in=notes_updates.keys() + ) + for grant in grants_to_update_notes: + new_notes = notes_updates.get(grant.id, "") + if grant.internal_notes != new_notes: + grant.internal_notes = new_notes + grant.save(update_fields=["internal_notes"]) + + def get_review_context( + self, + request: HttpRequest, + review_session: ReviewSession, + review_item_id: int, + user_review: UserReview | None, + admin_site: AdminSite, + ) -> dict[str, Any]: + from reviews.models import AvailableScoreOption + + private_comment = request.GET.get( + "private_comment", user_review.private_comment if user_review else "" + ) + comment = request.GET.get("comment", user_review.comment if user_review else "") + + grant = Grant.objects.get(id=review_item_id) + previous_grants = Grant.objects.filter( + user_id=grant.user_id, + conference__organizer_id=grant.conference.organizer_id, + ).exclude(conference_id=grant.conference_id) + + return dict( + admin_site.each_context(request), + grant=grant, + has_sent_proposal=Submission.objects.non_cancelled() + .filter( + speaker_id=grant.user_id, + conference_id=grant.conference_id, + ) + .exists(), + previous_grants=previous_grants, + available_scores=AvailableScoreOption.objects.filter( + review_session_id=review_session.id + ).order_by("-numeric_value"), + review_session_id=review_session.id, + user_review=user_review, + private_comment=private_comment, + comment=comment, + review_session_repr=str(review_session), + can_review_items=review_session.can_review_items, + seen=request.GET.get("seen", "").split(","), + title=f"Grant Review: {grant.user.display_name}", + participant=Participant.objects.filter( + user_id=grant.user_id, + conference=grant.conference, + ).first(), + ) + + def get_next_to_review_item_id( + self, + review_session: ReviewSession, + user: User, + skip_item: int | None = None, + exclude: list[int] | None = None, + seen: list[int] | None = None, + ) -> int | None: + from reviews.models import UserReview + + exclude = exclude or [] + seen = seen or [] + + already_reviewed_ids = UserReview.objects.filter( + user_id=user.id, + review_session_id=review_session.id, + ).values_list("grant_id", flat=True) + + items_to_exclude = ( + list(already_reviewed_ids) + ([skip_item] if skip_item else []) + seen + ) + + unvoted_item = ( + review_session.conference.grants.annotate( + votes_received=Count( + "userreview", + filter=Q(userreview__review_session_id=review_session.id), + ) + ) + .exclude(id__in=items_to_exclude) + .order_by("votes_received", "?") + .first() + ) + + return unvoted_item.id if unvoted_item else None + + def get_user_review_filter(self, review_item_id: int) -> dict[str, Any]: + return {"grant_id": review_item_id} + + def get_user_review_create_values(self, review_item_id: int) -> dict[str, Any]: + return {"grant_id": review_item_id} + + +def get_review_adapter(review_session: ReviewSession) -> ReviewAdapter: + """ + Factory function to get the appropriate adapter for a review session. + + Args: + review_session: The review session to get an adapter for. + + Returns: + The appropriate adapter instance based on the session type. + + Raises: + ValueError: If the session type is unknown. + """ + if review_session.is_proposals_review: + return ProposalsReviewAdapter() + + if review_session.is_grants_review: + return GrantsReviewAdapter() + + raise ValueError(f"Unknown review session type: {review_session.session_type}") diff --git a/backend/reviews/admin.py b/backend/reviews/admin.py index 219d3f1a2b..4325787bd3 100644 --- a/backend/reviews/admin.py +++ b/backend/reviews/admin.py @@ -2,40 +2,17 @@ from django import forms from django.contrib import admin, messages -from django.contrib.postgres.expressions import ArraySubquery from django.core.exceptions import PermissionDenied -from django.db.models import ( - Avg, - Count, - Exists, - F, - FloatField, - OuterRef, - Prefetch, - Q, - StdDev, - Subquery, - Sum, -) -from django.db.models.expressions import ExpressionWrapper -from django.db.models.functions import Cast from django.http.request import HttpRequest from django.shortcuts import redirect from django.template.response import TemplateResponse from django.urls import path, reverse from django.utils.safestring import mark_safe -from custom_admin.audit import ( - create_addition_admin_log_entry, - create_change_admin_log_entry, - create_deletion_admin_log_entry, -) -from grants.models import Grant, GrantReimbursement, GrantReimbursementCategory -from participants.models import Participant +from reviews.adapters import get_review_adapter from reviews.models import AvailableScoreOption, ReviewSession, UserReview -from submissions.models import Submission, SubmissionTag +from submissions.models import SubmissionTag from users.admin_mixins import ConferencePermissionMixin -from users.models import User class AvailableScoreOptionInline(admin.TabularInline): @@ -218,7 +195,10 @@ def get_urls(self): def review_start_view(self, request, review_session_id): review_session = ReviewSession.objects.get(id=review_session_id) - next_to_review = get_next_to_review_item_id(review_session, request.user) + adapter = get_review_adapter(review_session) + next_to_review = adapter.get_next_to_review_item_id( + review_session, request.user + ) if not next_to_review: messages.warning(request, "No new proposal to review.") @@ -258,145 +238,20 @@ def review_recap_view(self, request, review_session_id): ) ) - if review_session.is_proposals_review: - return self._review_proposals_recap_view(request, review_session) - - if review_session.is_grants_review: - return self._review_grants_recap_view(request, review_session) - - def _review_grants_recap_view(self, request, review_session): - review_session_id = review_session.id + adapter = get_review_adapter(review_session) if request.method == "POST": if not request.user.has_perm( "reviews.decision_reviewsession", review_session ): raise PermissionDenied() - data = request.POST - - reimbursement_categories = { - category.id: category - for category in GrantReimbursementCategory.objects.for_conference( - conference=review_session.conference - ) - } - - decisions = { - int(key.split("-")[1]): value - for [key, value] in data.items() - if key.startswith("decision-") - } - approved_reimbursement_categories_decisions = { - int(key.split("-")[1]): [int(id_) for id_ in data.getlist(key)] - for key in data.keys() - if key.startswith("reimbursementcategory-") - } - - grants = list( - review_session.conference.grants.filter(id__in=decisions.keys()).all() - ) - - grants_with_pending_status_changes = {} - for grant in grants: - decision = decisions[grant.id] - if decision not in Grant.REVIEW_SESSION_STATUSES_OPTIONS: - continue - - original_pending_status = grant.pending_status - if decision != grant.status: - grant.pending_status = decision - elif decision == grant.status: - grant.pending_status = None - - if grant.pending_status != original_pending_status: - grants_with_pending_status_changes[grant.id] = ( - original_pending_status - ) + adapter.process_recap_post(request, review_session) - # if there are grant reimbursements and the decision is not approved, delete them all - if grant.reimbursements.exists(): - approved_reimbursement_categories = ( - approved_reimbursement_categories_decisions.get(grant.id, []) - ) - # If decision is not approved, delete all; else, filter and delete missing reimbursements - if decision != Grant.Status.approved: - # Log deletions before deleting - for reimbursement in grant.reimbursements.all(): - create_deletion_admin_log_entry( - request.user, - grant, - change_message=f"[Review Session] Reimbursement removed: {reimbursement.category.name}.", - ) - reimbursement.delete() - else: - # Only keep those in current approved_reimbursement_categories - # Log deletions before deleting - to_delete = grant.reimbursements.exclude( - category_id__in=approved_reimbursement_categories - ) - for reimbursement in to_delete: - create_deletion_admin_log_entry( - request.user, - grant, - change_message=f"[Review Session] Reimbursement removed: {reimbursement.category.name}.", - ) - to_delete.delete() - - for grant in grants: - # save each to make sure we re-calculate the grants amounts - # TODO: move the amount calculation in a separate function maybe? - grant.save( - update_fields=[ - "pending_status", - ] + if review_session.is_grants_review: + messages.success( + request, "Decisions saved. Check the Grants Summary for more info." ) - if grant.id in grants_with_pending_status_changes: - original_pending_status = grants_with_pending_status_changes[ - grant.id - ] - create_change_admin_log_entry( - request.user, - grant, - change_message=f"[Review Session] Pending status changed from '{original_pending_status}' to '{grant.pending_status}'.", - ) - - # The frontend may send reimbursement categories as checked by default, - # so they're always passed to the backend. However, if the grant is not approved, - # we don't need to consider reimbursements at all and can skip all reimbursement logic. - if grant.pending_status != Grant.Status.approved: - continue - - approved_reimbursement_categories = ( - approved_reimbursement_categories_decisions.get(grant.id, []) - ) - - for reimbursement_category_id in approved_reimbursement_categories: - # Check if category exists to avoid KeyError - if reimbursement_category_id not in reimbursement_categories: - continue - reimbursement, created = ( - GrantReimbursement.objects.update_or_create( - grant=grant, - category_id=reimbursement_category_id, - defaults={ - "granted_amount": reimbursement_categories[ - reimbursement_category_id - ].max_amount - }, - ) - ) - - if created: - create_addition_admin_log_entry( - request.user, - grant, - change_message=f"[Review Session] Reimbursement {reimbursement.category.name} added.", - ) - - messages.success( - request, "Decisions saved. Check the Grants Summary for more info." - ) return redirect( reverse( @@ -407,189 +262,12 @@ def _review_grants_recap_view(self, request, review_session): ) ) - items = ( - review_session.conference.grants.annotate( - total_score=Cast( - Sum( - "userreview__score__numeric_value", - filter=Q(userreview__review_session_id=review_session_id), - ), - output_field=FloatField(), - ), - vote_count=Cast( - Count( - "userreview", - filter=Q(userreview__review_session_id=review_session_id), - ), - output_field=FloatField(), - ), - score=ExpressionWrapper( - F("total_score") / F("vote_count"), - output_field=FloatField(), - ), - std_dev=StdDev( - "userreview__score__numeric_value", - filter=Q(userreview__review_session_id=review_session_id), - ), - has_sent_a_proposal=Exists( - Submission.objects.non_cancelled().filter( - speaker_id=OuterRef("user_id"), - conference_id=review_session.conference_id, - ) - ), - proposals_ids=ArraySubquery( - Submission.objects.non_cancelled() - .filter( - speaker_id=OuterRef("user_id"), - conference_id=review_session.conference_id, - ) - .values("id") - ), - approved_reimbursement_category_ids=ArraySubquery( - GrantReimbursement.objects.filter( - grant_id=OuterRef("pk") - ).values_list("category_id", flat=True) - ), - ) - .order_by(F("score").desc(nulls_last=True)) - .prefetch_related( - Prefetch( - "userreview_set", - queryset=UserReview.objects.prefetch_related( - "user", "score" - ).filter(review_session_id=review_session_id), - ), - "user", - ) - .all() - ) - - proposals = { - submission.id: submission - for submission in Submission.objects.non_cancelled() - .filter( - conference_id=review_session.conference_id, - speaker_id__in=items.values_list("user_id"), - ) - .prefetch_related("rankings", "rankings__tag") - } - - context = dict( - self.admin_site.each_context(request), - request=request, - items=items, - proposals=proposals, - review_session_id=review_session_id, - review_session_repr=str(review_session), - all_review_statuses=[ - choice - for choice in Grant.Status.choices - if choice[0] in Grant.REVIEW_SESSION_STATUSES_OPTIONS - ], - all_statuses=Grant.Status.choices, - all_reimbursement_categories=GrantReimbursementCategory.objects.for_conference( - conference=review_session.conference - ), - review_session=review_session, - title="Recap", + items = adapter.get_recap_items_queryset(review_session).all() + context = adapter.get_recap_context( + request, review_session, items, self.admin_site ) - return TemplateResponse(request, "grants-recap.html", context) - def _review_proposals_recap_view(self, request, review_session): - review_session_id = review_session.id - conference = review_session.conference - - if request.method == "POST": - if not request.user.has_perm( - "reviews.decision_reviewsession", review_session - ): - raise PermissionDenied() - - data = request.POST - - decisions = { - int(key.split("-")[1]): value - for [key, value] in data.items() - if key.startswith("decision-") - } - - proposals = list( - conference.submissions.filter(id__in=decisions.keys()).all() - ) - - for proposal in proposals: - decision = decisions[proposal.id] - proposal.pending_status = decision - - Submission.objects.bulk_update( - proposals, - fields=["pending_status"], - ) - - return redirect( - reverse( - "admin:reviews-recap", - kwargs={ - "review_session_id": review_session_id, - }, - ) - ) - - items = ( - Submission.objects.for_conference(review_session.conference_id) - .non_cancelled() - .annotate( - score=Subquery( - UserReview.objects.select_related("score") - .filter( - review_session_id=review_session_id, - proposal_id=OuterRef("id"), - ) - .values("proposal_id") - .annotate(score=Avg("score__numeric_value")) - .values("score") - ) - ) - .order_by(F("score").desc(nulls_last=True)) - .prefetch_related( - Prefetch( - "userreview_set", - queryset=UserReview.objects.prefetch_related( - "user", "score" - ).filter(review_session_id=review_session_id), - ), - "duration", - "audience_level", - "languages", - "speaker", - "tags", - "type", - "rankings", - "rankings__tag", - ) - .all() - ) - - speakers_ids = items.values_list("speaker_id", flat=True) - - grants = { - str(grant.user_id): grant - for grant in Grant.objects.filter( - conference=conference, user_id__in=speakers_ids - ).all() - } - - context = dict( - self.admin_site.each_context(request), - items=items, - grants=grants, - review_session_id=review_session_id, - audience_levels=conference.audience_levels.all(), - review_session_repr=str(review_session), - all_statuses=[choice for choice in Submission.STATUS], - title="Recap", - ) - return TemplateResponse(request, "proposals-recap.html", context) + return TemplateResponse(request, adapter.recap_template, context) def review_view(self, request, review_session_id, review_item_id): review_session = ReviewSession.objects.get(id=review_session_id) @@ -597,314 +275,114 @@ def review_view(self, request, review_session_id, review_item_id): if not review_session.user_can_review(request.user): raise PermissionDenied() - filter_options = {} - - if review_session.is_proposals_review: - filter_options["proposal_id"] = review_item_id - elif review_session.is_grants_review: - filter_options["grant_id"] = review_item_id + adapter = get_review_adapter(review_session) if request.method == "GET": + filter_options = adapter.get_user_review_filter(review_item_id) user_review = UserReview.objects.filter( user_id=request.user.id, review_session_id=review_session_id, **filter_options, ).first() - if review_session.is_proposals_review: - response = self._render_proposal_review( - request, - review_session=review_session, - review_item_id=review_item_id, - user_review=user_review, - ) - elif review_session.is_grants_review: - response = self._render_grant_review( - request, - review_session=review_session, - review_item_id=review_item_id, - user_review=user_review, - ) + context = adapter.get_review_context( + request, + review_session, + review_item_id, + user_review, + self.admin_site, + ) + return TemplateResponse(request, adapter.review_template, context) - return response - elif request.method == "POST": - # if not review_session.can_review_items: - # messages.error(request, "You cannot vote yet/anymore.") - # return redirect( - # reverse( - # "admin:reviews_reviewsession_change", - # kwargs={ - # "object_id": review_session_id, - # }, - # ) - # ) - - form = SubmitVoteForm(request.POST) - form.is_valid() - - seen = [ - str(id_) for id_ in form.cleaned_data.get("seen", "").split(",") if id_ - ] - seen.append(str(review_item_id)) - - exclude = form.cleaned_data.get("exclude", []) - - if form.cleaned_data.get("_skip"): - # Skipping to the next item without voting - next_to_review = get_next_to_review_item_id( - review_session, - request.user, - skip_item=review_item_id, - exclude=exclude, - seen=seen, - ) - elif form.cleaned_data.get("_next"): - if not form.is_valid(): - messages.error(request, "Invalid vote") - comment = urllib.parse.quote(form.cleaned_data["comment"]) - private_comment = urllib.parse.quote( - form.cleaned_data["private_comment"] - ) + # POST request handling + form = SubmitVoteForm(request.POST) + form.is_valid() - return redirect( - reverse( - "admin:reviews-vote-view", - kwargs={ - "review_session_id": review_session_id, - "review_item_id": review_item_id, - }, - ) - + f"?exclude={','.join(exclude)}" - + f"&seen={','.join(seen)}" - + f"&comment={comment}" - + f"&private_comment={private_comment}" - ) + seen = [str(id_) for id_ in form.cleaned_data.get("seen", "").split(",") if id_] + seen.append(str(review_item_id)) - values = { - "user_id": request.user.id, - "review_session_id": review_session_id, - } - - if review_session.is_proposals_review: - values["proposal_id"] = review_item_id - elif review_session.is_grants_review: - values["grant_id"] = review_item_id - - UserReview.objects.update_or_create( - **values, - defaults={ - "score_id": form.cleaned_data["score"].id, - "comment": form.cleaned_data["comment"], - "private_comment": form.cleaned_data["private_comment"], - }, - ) - next_to_review = get_next_to_review_item_id( - review_session, request.user, exclude=exclude, seen=seen - ) + exclude = form.cleaned_data.get("exclude", []) - if not next_to_review: - messages.warning( - request, "No new items to review, showing an already seen one." - ) - next_to_review = get_next_to_review_item_id( - review_session, - request.user, - skip_item=review_item_id, - exclude=exclude, + next_to_review = None + + if form.cleaned_data.get("_skip"): + # Skipping to the next item without voting + next_to_review = adapter.get_next_to_review_item_id( + review_session, + request.user, + skip_item=review_item_id, + exclude=exclude, + seen=seen, + ) + elif form.cleaned_data.get("_next"): + if not form.is_valid(): + messages.error(request, "Invalid vote") + comment = urllib.parse.quote(form.cleaned_data["comment"]) + private_comment = urllib.parse.quote( + form.cleaned_data["private_comment"] ) - if not next_to_review: - messages.warning(request, "No new proposal to review.") - return redirect( - reverse( - "admin:reviews-recap", - kwargs={ - "review_session_id": review_session_id, - }, - ) + return redirect( + reverse( + "admin:reviews-vote-view", + kwargs={ + "review_session_id": review_session_id, + "review_item_id": review_item_id, + }, ) - - return redirect( - reverse( - "admin:reviews-vote-view", - kwargs={ - "review_session_id": review_session_id, - "review_item_id": next_to_review, - }, + + f"?exclude={','.join(exclude)}" + + f"&seen={','.join(seen)}" + + f"&comment={comment}" + + f"&private_comment={private_comment}" ) - + f"?exclude={','.join(exclude)}&seen={','.join(seen)}" - ) - def _render_grant_review( - self, request, review_session, review_item_id, user_review - ): - private_comment = request.GET.get( - "private_comment", user_review.private_comment if user_review else "" - ) - comment = request.GET.get("comment", user_review.comment if user_review else "") - - grant = Grant.objects.get(id=review_item_id) - previous_grants = Grant.objects.filter( - user_id=grant.user_id, - conference__organizer_id=grant.conference.organizer_id, - ).exclude(conference_id=grant.conference_id) - - context = dict( - self.admin_site.each_context(request), - grant=grant, - has_sent_proposal=Submission.objects.non_cancelled() - .filter( - speaker_id=grant.user_id, - conference_id=grant.conference_id, + values = { + "user_id": request.user.id, + "review_session_id": review_session_id, + **adapter.get_user_review_create_values(review_item_id), + } + + UserReview.objects.update_or_create( + **values, + defaults={ + "score_id": form.cleaned_data["score"].id, + "comment": form.cleaned_data["comment"], + "private_comment": form.cleaned_data["private_comment"], + }, ) - .exists(), - previous_grants=previous_grants, - available_scores=AvailableScoreOption.objects.filter( - review_session_id=review_session.id - ).order_by("-numeric_value"), - review_session_id=review_session.id, - user_review=user_review, - private_comment=private_comment, - comment=comment, - review_session_repr=str(review_session), - can_review_items=review_session.can_review_items, - seen=request.GET.get("seen", "").split(","), - title=f"Grant Review: {grant.user.display_name}", - participant=Participant.objects.filter( - user_id=grant.user_id, - conference=grant.conference, - ).first(), - ) - return TemplateResponse(request, "grant-review.html", context) - - def _render_proposal_review( - self, request, review_session, review_item_id, user_review - ): - proposal = ( - Submission.objects.for_conference(review_session.conference_id) - .prefetch_related( - "rankings", - "rankings__tag", - Prefetch( - "userreview_set", - queryset=UserReview.objects.prefetch_related( - "user", "score" - ).filter(review_session_id=review_session.id), - ), + next_to_review = adapter.get_next_to_review_item_id( + review_session, request.user, exclude=exclude, seen=seen ) - .get(id=review_item_id) - ) - languages = list(proposal.languages.all()) - speaker = proposal.speaker - grant = ( - Grant.objects.of_user(proposal.speaker_id) - .for_conference(proposal.conference_id) - .first() - ) - grant_link = ( - reverse("admin:grants_grant_change", args=(grant.id,)) if grant else "" - ) - - existing_comment = request.GET.get("comment", "") - tags_already_excluded = request.GET.get("exclude", "").split(",") - - used_tags = ( - Submission.objects.filter( - conference_id=proposal.conference_id, + if not next_to_review: + messages.warning( + request, "No new items to review, showing an already seen one." ) - .values_list("tags__id", flat=True) - .distinct() - ) - - tags_to_filter = ( - SubmissionTag.objects.filter(id__in=used_tags).order_by("name").all() - ) - - context = dict( - self.admin_site.each_context(request), - proposal=proposal, - languages=proposal.languages.all(), - available_scores=AvailableScoreOption.objects.filter( - review_session_id=review_session.id - ).order_by("-numeric_value"), - proposal_id=review_item_id, - review_session_id=review_session.id, - user_review=user_review, - has_italian_language=any( - language for language in languages if language.code == "it" - ), - has_english_language=any( - language for language in languages if language.code == "en" - ), - speaker=speaker, - grant=grant, - grant_link=grant_link, - participant=Participant.objects.filter( - user_id=proposal.speaker_id, - conference=proposal.conference, - ).first(), - tags_to_filter=tags_to_filter, - tags_already_excluded=tags_already_excluded, - seen=request.GET.get("seen", "").split(","), - existing_comment=existing_comment, - review_session_repr=str(review_session), - title=f"Proposal Review: {proposal.title.localize('en')}", - ) - return TemplateResponse(request, "proposal-review.html", context) - - -def get_next_to_review_item_id( - review_session: ReviewSession, - user: User, - skip_item: int | None = None, - exclude: list[int] = None, - seen: list[int] = None, -) -> int | None: - exclude = exclude or [] - seen = seen or [] - already_reviewed = UserReview.objects.filter( - user_id=user.id, - review_session_id=review_session.id, - ) - - if review_session.is_proposals_review: - already_reviewed_ids = already_reviewed.values_list("proposal_id", flat=True) - skip_item_array = [skip_item] if skip_item else [] - seen_items_to_ignore = list(already_reviewed_ids) + skip_item_array + seen - qs = ( - Submission.objects.non_cancelled() - .for_conference(review_session.conference_id) - .annotate( - votes_received=Count( - "userreview", - filter=Q(userreview__review_session_id=review_session.id), - ) + next_to_review = adapter.get_next_to_review_item_id( + review_session, + request.user, + skip_item=review_item_id, + exclude=exclude, ) - .order_by("votes_received", "?") - ) - - if seen_items_to_ignore: - qs = qs.exclude(id__in=seen_items_to_ignore) - if exclude: - qs = qs.exclude(tags__in=exclude) - - unvoted_item = qs.first() - elif review_session.is_grants_review: - already_reviewed_ids = already_reviewed.values_list("grant_id", flat=True) - unvoted_item = ( - review_session.conference.grants.annotate( - votes_received=Count( - "userreview", - filter=Q(userreview__review_session_id=review_session.id), + if not next_to_review: + messages.warning(request, "No new proposal to review.") + return redirect( + reverse( + "admin:reviews-recap", + kwargs={ + "review_session_id": review_session_id, + }, + ) ) + + return redirect( + reverse( + "admin:reviews-vote-view", + kwargs={ + "review_session_id": review_session_id, + "review_item_id": next_to_review, + }, ) - .exclude( - id__in=list(already_reviewed_ids) + [skip_item] + seen, - ) - .order_by("votes_received", "?") - .first() + + f"?exclude={','.join(exclude)}&seen={','.join(seen)}" ) - - return unvoted_item.id if unvoted_item else None diff --git a/backend/reviews/templates/grants-recap.html b/backend/reviews/templates/grants-recap.html index f710df78ca..bdf87ad6ac 100644 --- a/backend/reviews/templates/grants-recap.html +++ b/backend/reviews/templates/grants-recap.html @@ -235,6 +235,25 @@ text-decoration: underline; } + .notes-cell { + position: relative; + min-width: 220px; + } + + .notes-textarea { + position: absolute; + top: 8px; + left: 8px; + right: 8px; + bottom: 8px; + width: calc(100% - 16px); + min-height: 80px; + resize: none; + font-size: 12px; + padding: 5px; + box-sizing: border-box; + } + /* Center-align columns */ .results-table th:nth-child(1), /* number column */ .results-table th:nth-child(3), /* score column */ @@ -616,6 +635,12 @@

+ +
+ Notes +
+
+ @@ -816,6 +841,17 @@

{% else %} No permission to change. {% endif %} + + {% if perms.reviews.decision_reviewsession %} + + {% else %} + {{ item.internal_notes|default:"-" }} + {% endif %} + {% endfor %} diff --git a/backend/reviews/tests/test_admin.py b/backend/reviews/tests/test_admin.py index 1c7265aac8..d474a9fae1 100644 --- a/backend/reviews/tests/test_admin.py +++ b/backend/reviews/tests/test_admin.py @@ -11,7 +11,8 @@ GrantReimbursementCategoryFactory, GrantReimbursementFactory, ) -from reviews.admin import ReviewSessionAdmin, get_next_to_review_item_id +from reviews.adapters import get_review_adapter +from reviews.admin import ReviewSessionAdmin from reviews.models import ReviewSession from reviews.tests.factories import ( AvailableScoreOptionFactory, @@ -53,7 +54,9 @@ def test_next_item_to_review_prefers_items_with_fewer_votes(): score=score_0, ) - next_to_review = get_next_to_review_item_id(review_session, user_2) + adapter = get_review_adapter(review_session) + next_to_review = adapter.get_next_to_review_item_id(review_session, user_2) + assert next_to_review == submission_2.id @@ -88,9 +91,12 @@ def test_next_item_to_review_for_submissions_ignores_excluded_tags(iteration): submission_2.tags.add(tag_2) submission_2.tags.add(tag_3) - next_to_review = get_next_to_review_item_id( + adapter = get_review_adapter(review_session) + + next_to_review = adapter.get_next_to_review_item_id( review_session, user_1, exclude=[tag_2.id] ) + assert next_to_review == submission_1.id @@ -178,13 +184,8 @@ def test_grants_review_scores(rf, scores, avg): score=all_scores[-1], ) - request = rf.get("/") - request.user = users[5] - - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) - context_data = response.context_data - items = context_data["items"] + adapter = get_review_adapter(review_session) + items = adapter.get_recap_items_queryset(review_session).all() grant_to_check = next(item for item in items if item.id == grant_1.id) assert grant_to_check.id == grant_1.id @@ -296,9 +297,11 @@ def test_grants_review_std_dev(rf, scores, expected_std_dev): request = rf.get("/") request.user = users[5] - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) - context_data = response.context_data + adapter = get_review_adapter(review_session) + items = adapter.get_recap_items_queryset(review_session).all() + context_data = adapter.get_recap_context( + request, review_session, items, AdminSite() + ) items = context_data["items"] grant_to_check = next(item for item in items if item.id == grant.id) @@ -397,9 +400,7 @@ def test_review_start_view(rf, mocker): ) -def test_save_review_grants_updates_grant_and_creates_reimbursements(rf, mocker): - mock_messages = mocker.patch("reviews.admin.messages") - +def test_save_review_grants_updates_grant_and_creates_reimbursements(rf): user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -454,15 +455,8 @@ def test_save_review_grants_updates_grant_and_creates_reimbursements(rf, mocker) request = rf.post("/", data=post_data) request.user = user - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) - - # Should redirect after successful save - assert response.status_code == 302 - assert ( - response.url - == f"/admin/reviews/reviewsession/{review_session.id}/review/recap/" - ) + adapter = get_review_adapter(review_session) + adapter.process_recap_post(request, review_session) # Refresh grants from database grant_1.refresh_from_db() @@ -511,14 +505,8 @@ def test_save_review_grants_updates_grant_and_creates_reimbursements(rf, mocker) change_message=f"[Review Session] Reimbursement {accommodation_category.name} added.", ).exists() - mock_messages.success.assert_called_once() - - -def test_save_review_grants_update_grants_status_to_rejected_removes_reimbursements( - rf, mocker -): - mocker.patch("reviews.admin.messages") +def test_save_review_grants_update_grants_status_to_rejected_removes_reimbursements(rf): user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -575,15 +563,9 @@ def test_save_review_grants_update_grants_status_to_rejected_removes_reimburseme request = rf.post("/", data=post_data) request.user = user - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) + adapter = get_review_adapter(review_session) + adapter.process_recap_post(request, review_session) - # Should redirect after successful save - assert response.status_code == 302 - assert ( - response.url - == f"/admin/reviews/reviewsession/{review_session.id}/review/recap/" - ) grant_1.refresh_from_db() assert grant_1.pending_status == Grant.Status.rejected @@ -599,9 +581,7 @@ def test_save_review_grants_update_grants_status_to_rejected_removes_reimburseme ).exists() -def test_save_review_grants_modify_reimbursements(rf, mocker): - mocker.patch("reviews.admin.messages") - +def test_save_review_grants_modify_reimbursements(rf): user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -658,15 +638,8 @@ def test_save_review_grants_modify_reimbursements(rf, mocker): request = rf.post("/", data=post_data) request.user = user - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) - - # Should redirect after successful save - assert response.status_code == 302 - assert ( - response.url - == f"/admin/reviews/reviewsession/{review_session.id}/review/recap/" - ) + adapter = get_review_adapter(review_session) + adapter.process_recap_post(request, review_session) grant_1.refresh_from_db() @@ -696,12 +669,9 @@ def test_save_review_grants_modify_reimbursements(rf, mocker): ).exists() -def test_save_review_grants_waiting_list_does_not_create_reimbursments(rf, mocker): - mock_messages = mocker.patch("reviews.admin.messages") - +def test_save_review_grants_waiting_list_does_not_create_reimbursements(rf): user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() - # Create reimbursement categories travel_category = GrantReimbursementCategoryFactory( conference=conference, @@ -748,15 +718,8 @@ def test_save_review_grants_waiting_list_does_not_create_reimbursments(rf, mocke request = rf.post("/", data=post_data) request.user = user - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - response = admin._review_grants_recap_view(request, review_session) - - # Should redirect after successful save - assert response.status_code == 302 - assert ( - response.url - == f"/admin/reviews/reviewsession/{review_session.id}/review/recap/" - ) + adapter = get_review_adapter(review_session) + adapter.process_recap_post(request, review_session) # Refresh grants from database grant_1.refresh_from_db() @@ -773,16 +736,13 @@ def test_save_review_grants_waiting_list_does_not_create_reimbursments(rf, mocke # Verify log entries were created assert ( LogEntry.objects.filter(object_id=grant_1.id).count() == 1 - ) # 1 pending_status change, 2 reimbursement additions + ) # 1 pending_status change assert ( LogEntry.objects.filter(object_id=grant_2.id).count() == 1 - ) # 1 pending_status change, 3 reimbursement additions - mock_messages.success.assert_called_once() + ) # 1 pending_status change -def test_save_review_grants_two_times_does_not_create_duplicate_log_entries(rf, mocker): - mocker.patch("reviews.admin.messages") - +def test_save_review_grants_two_times_does_not_create_duplicate_log_entries(rf): user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -824,9 +784,9 @@ def test_save_review_grants_two_times_does_not_create_duplicate_log_entries(rf, request = rf.post("/", data=post_data) request.user = user - admin = ReviewSessionAdmin(ReviewSession, AdminSite()) - admin._review_grants_recap_view(request, review_session) # First save - admin._review_grants_recap_view(request, review_session) # Second save + adapter = get_review_adapter(review_session) + adapter.process_recap_post(request, review_session) # First save + adapter.process_recap_post(request, review_session) # Second save grant_1.refresh_from_db()