From 7c8820cfed289559e891e6012fd5515220c8cd11 Mon Sep 17 00:00:00 2001 From: Ester Beltrami Date: Sat, 30 Aug 2025 16:18:44 +0300 Subject: [PATCH 1/2] Refactor grant reimbursement categories to be flexible Remove hardcoded default grant amounts for ticket, accommodation, and travel from `Conference` in favor of using `GrantReimbursementCategory`. Update all relevant admin forms, models, and templates to reference flexible categories instead of fixed fields. - Remove legacy fields: `grants_default_ticket_amount`, `grants_default_accommodation_amount`, `grants_default_travel_from_italy_amount`, and `grants_default_travel_from_europe_amount` from `Conference` - Update `Grant` and `GrantReimbursement` logic to work exclusively with `GrantReimbursementCategory` - Refactor grant review admin and summary logic to support multiple, configurable reimbursement categories per grant - Migrate existing grants to new reimbursement category scheme - Add and update tests and migrations to cover flexible grant categories This change allows flexible reimbursement types (and amounts) to be configured per conference, supports granular grant allocation, and paves the way for internationalization and more complex business rules. --- backend/conferences/admin/conference.py | 12 - ...s_default_accommodation_amount_and_more.py | 33 ++ backend/conferences/models/conference.py | 41 --- backend/grants/admin.py | 118 ++++--- ...ove_grant_accommodation_amount_and_more.py | 167 ++++++++++ backend/grants/models.py | 199 +++++------- backend/grants/summary.py | 48 +-- ...migration_backfill_grant_reimbursements.py | 300 ++++++++++++++++++ backend/grants/tests/test_models.py | 199 +++++++----- backend/reviews/admin.py | 71 +++-- backend/reviews/templates/grants-recap.html | 18 +- backend/reviews/tests/test_admin.py | 251 ++++++++++++++- 12 files changed, 1123 insertions(+), 334 deletions(-) create mode 100644 backend/conferences/migrations/0055_remove_conference_grants_default_accommodation_amount_and_more.py create mode 100644 backend/grants/migrations/0030_remove_grant_accommodation_amount_and_more.py create mode 100644 backend/grants/tests/test_migration_backfill_grant_reimbursements.py diff --git a/backend/conferences/admin/conference.py b/backend/conferences/admin/conference.py index 3244a302d7..a833b8af9a 100644 --- a/backend/conferences/admin/conference.py +++ b/backend/conferences/admin/conference.py @@ -184,18 +184,6 @@ class ConferenceAdmin( ) }, ), - ( - "Grants", - { - "fields": ( - "grants_default_ticket_amount", - "grants_default_accommodation_amount", - "grants_default_travel_from_italy_amount", - "grants_default_travel_from_europe_amount", - "grants_default_travel_from_extra_eu_amount", - ) - }, - ), ("YouTube", {"fields": ("video_title_template", "video_description_template")}), ) inlines = [DeadlineInline, DurationInline, SponsorLevelInline, IncludedEventInline] diff --git a/backend/conferences/migrations/0055_remove_conference_grants_default_accommodation_amount_and_more.py b/backend/conferences/migrations/0055_remove_conference_grants_default_accommodation_amount_and_more.py new file mode 100644 index 0000000000..1d0465bb0f --- /dev/null +++ b/backend/conferences/migrations/0055_remove_conference_grants_default_accommodation_amount_and_more.py @@ -0,0 +1,33 @@ +# Generated by Django 5.1.4 on 2025-07-27 14:30 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('conferences', '0054_conference_frontend_revalidate_secret_and_more'), + ] + + operations = [ + migrations.RemoveField( + model_name='conference', + name='grants_default_accommodation_amount', + ), + migrations.RemoveField( + model_name='conference', + name='grants_default_ticket_amount', + ), + migrations.RemoveField( + model_name='conference', + name='grants_default_travel_from_europe_amount', + ), + migrations.RemoveField( + model_name='conference', + name='grants_default_travel_from_extra_eu_amount', + ), + migrations.RemoveField( + model_name='conference', + name='grants_default_travel_from_italy_amount', + ), + ] diff --git a/backend/conferences/models/conference.py b/backend/conferences/models/conference.py index e04db0665a..1388becd07 100644 --- a/backend/conferences/models/conference.py +++ b/backend/conferences/models/conference.py @@ -93,47 +93,6 @@ class Conference(GeoLocalizedModel, TimeFramedModel, TimeStampedModel): default="", ) - grants_default_ticket_amount = models.DecimalField( - verbose_name=_("grants default ticket amount"), - null=True, - blank=True, - max_digits=6, - decimal_places=2, - default=None, - ) - grants_default_accommodation_amount = models.DecimalField( - verbose_name=_("grants default accommodation amount"), - null=True, - blank=True, - max_digits=6, - decimal_places=2, - default=None, - ) - grants_default_travel_from_italy_amount = models.DecimalField( - verbose_name=_("grants default travel from Italy amount"), - null=True, - blank=True, - max_digits=6, - decimal_places=2, - default=None, - ) - grants_default_travel_from_europe_amount = models.DecimalField( - verbose_name=_("grants default travel from Europe amount"), - null=True, - blank=True, - max_digits=6, - decimal_places=2, - default=None, - ) - grants_default_travel_from_extra_eu_amount = models.DecimalField( - verbose_name=_("grants default travel from Extra EU amount"), - null=True, - blank=True, - max_digits=6, - decimal_places=2, - default=None, - ) - video_title_template = models.TextField( default="", blank=True, diff --git a/backend/grants/admin.py b/backend/grants/admin.py index 139e569b8b..ea5e4bbd37 100644 --- a/backend/grants/admin.py +++ b/backend/grants/admin.py @@ -1,45 +1,53 @@ import logging -from django.db import transaction -from custom_admin.audit import ( - create_addition_admin_log_entry, - create_change_admin_log_entry, -) -from conferences.models.conference_voucher import ConferenceVoucher -from pycon.constants import UTC -from custom_admin.admin import ( - confirm_pending_status, - reset_pending_status_back_to_status, - validate_single_conference_selection, -) -from import_export.resources import ModelResource from datetime import timedelta from typing import Dict, List, Optional -from countries.filters import CountryFilter + from django.contrib import admin, messages +from django.contrib.admin import SimpleListFilter +from django.db import transaction +from django.db.models import Exists, F, IntegerField, OuterRef, Sum, Value +from django.db.models.functions import Coalesce from django.db.models.query import QuerySet +from django.urls import reverse from django.utils import timezone +from django.utils.safestring import mark_safe from import_export.admin import ExportMixin from import_export.fields import Field -from users.admin_mixins import ConferencePermissionMixin +from import_export.resources import ModelResource + +from conferences.models.conference_voucher import ConferenceVoucher from countries import countries +from countries.filters import CountryFilter +from custom_admin.admin import ( + confirm_pending_status, + reset_pending_status_back_to_status, + validate_single_conference_selection, +) +from custom_admin.audit import ( + create_addition_admin_log_entry, + create_change_admin_log_entry, +) from grants.tasks import ( send_grant_reply_approved_email, + send_grant_reply_rejected_email, send_grant_reply_waiting_list_email, send_grant_reply_waiting_list_update_email, - send_grant_reply_rejected_email, ) +from participants.models import Participant +from pretix import user_has_admission_ticket +from pycon.constants import UTC from schedule.models import ScheduleItem from submissions.models import Submission -from .models import Grant, GrantConfirmPendingStatusProxy -from django.db.models import Exists, OuterRef -from pretix import user_has_admission_ticket - -from django.contrib.admin import SimpleListFilter -from participants.models import Participant -from django.urls import reverse -from django.utils.safestring import mark_safe +from users.admin_mixins import ConferencePermissionMixin from visa.models import InvitationLetterRequest +from .models import ( + Grant, + GrantConfirmPendingStatusProxy, + GrantReimbursement, + GrantReimbursementCategory, +) + logger = logging.getLogger(__name__) EXPORT_GRANTS_FIELDS = ( @@ -394,6 +402,32 @@ def queryset(self, request, queryset): return queryset +@admin.register(GrantReimbursementCategory) +class GrantReimbursementCategoryAdmin(ConferencePermissionMixin, admin.ModelAdmin): + list_display = ("__str__", "max_amount", "category", "included_by_default") + list_filter = ("conference", "category", "included_by_default") + search_fields = ("category", "name") + + +@admin.register(GrantReimbursement) +class GrantReimbursementAdmin(ConferencePermissionMixin, admin.ModelAdmin): + list_display = ( + "grant", + "category", + "granted_amount", + ) + list_filter = ("grant__conference", "category") + search_fields = ("grant__full_name", "grant__email") + autocomplete_fields = ("grant",) + + +class GrantReimbursementInline(admin.TabularInline): + model = GrantReimbursement + extra = 0 + autocomplete_fields = ["category"] + fields = ["category", "granted_amount"] + + @admin.register(Grant) class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): change_list_template = "admin/grants/grant/change_list.html" @@ -406,12 +440,8 @@ class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): "has_sent_invitation_letter_request", "emoji_gender", "conference", - "status", - "approved_type", - "ticket_amount", - "travel_amount", - "accommodation_amount", - "total_amount", + "current_or_pending_status", + "total_amount_display", "country_type", "user_has_ticket", "has_voucher", @@ -425,7 +455,6 @@ class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): "pending_status", "country_type", "occupation", - "approved_type", "needs_funds_for_travel", "need_visa", "need_accommodation", @@ -451,6 +480,7 @@ class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): "delete_selected", ] autocomplete_fields = ("user",) + inlines = [GrantReimbursementInline] fieldsets = ( ( @@ -459,12 +489,7 @@ class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): "fields": ( "status", "pending_status", - "approved_type", "country_type", - "ticket_amount", - "travel_amount", - "accommodation_amount", - "total_amount", "applicant_reply_sent_at", "applicant_reply_deadline", "internal_notes", @@ -528,6 +553,11 @@ def user_display_name(self, obj): return obj.user.display_name return obj.email + @admin.display(description="Status") + def current_or_pending_status(self, obj): + return obj.current_or_pending_status + + @admin.display( description="C", ) @@ -591,11 +621,22 @@ def has_sent_invitation_letter_request(self, obj: Grant) -> bool: if obj.has_invitation_letter_request: return "📧" return "" + @admin.display(description="Total") + def total_amount_display(self, obj): + return f"{obj.total_allocated:.2f}" + + @admin.display(description="Approved Reimbursements") + def approved_amounts_display(self, obj): + return ", ".join( + f"{r.category.name}: {r.granted_amount}" for r in obj.reimbursements.all() + ) def get_queryset(self, request): qs = ( super() .get_queryset(request) + .select_related("user") + .prefetch_related("reimbursements__category") .annotate( is_proposed_speaker=Exists( Submission.objects.non_cancelled().filter( @@ -622,6 +663,11 @@ def get_queryset(self, request): requester_id=OuterRef("user_id"), ) ), + total_allocated=Coalesce( + Sum("reimbursements__granted_amount"), + Value(0), + output_field=IntegerField(), + ), ) ) diff --git a/backend/grants/migrations/0030_remove_grant_accommodation_amount_and_more.py b/backend/grants/migrations/0030_remove_grant_accommodation_amount_and_more.py new file mode 100644 index 0000000000..6c481005ae --- /dev/null +++ b/backend/grants/migrations/0030_remove_grant_accommodation_amount_and_more.py @@ -0,0 +1,167 @@ +# Generated by Django 5.1.4 on 2025-11-01 15:07 + +import django.db.models.deletion +from django.db import migrations, models + + +def ensure_categories_exist(apps, schema_editor): + """Ensure reimbursement categories exist for all conferences.""" + Conference = apps.get_model("conferences", "Conference") + GrantReimbursementCategory = apps.get_model("grants", "GrantReimbursementCategory") + + for conference in Conference.objects.all(): + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="ticket", + defaults={ + "name": "Ticket", + "description": "Conference ticket", + "max_amount": conference.grants_default_ticket_amount + or Decimal("0.00"), + "included_by_default": True, + }, + ) + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="travel", + defaults={ + "name": "Travel", + "description": "Travel support", + "max_amount": conference.grants_default_travel_from_extra_eu_amount + or Decimal("400.00"), + "included_by_default": False, + }, + ) + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="accommodation", + defaults={ + "name": "Accommodation", + "description": "Accommodation support", + "max_amount": conference.grants_default_accommodation_amount + or Decimal("300.00"), + "included_by_default": True, + }, + ) + + +def migrate_grants(apps, schema_editor): + """Migrate existing grants to use the new reimbursement system.""" + Grant = apps.get_model("grants", "Grant") + GrantReimbursement = apps.get_model("grants", "GrantReimbursement") + GrantReimbursementCategory = apps.get_model("grants", "GrantReimbursementCategory") + + grants = Grant.objects.filter(approved_type__isnull=False).exclude(approved_type="") + + for grant in grants: + categories = { + c.category: c + for c in GrantReimbursementCategory.objects.filter( + conference_id=grant.conference_id + ) + } + + def add_reimbursement(category_key, amount): + if category_key in categories and amount: + GrantReimbursement.objects.get_or_create( + grant=grant, + category=categories[category_key], + defaults={"granted_amount": amount}, + ) + + # Always add ticket reimbursement + add_reimbursement("ticket", grant.ticket_amount) + + # Add travel reimbursement if approved + if grant.approved_type in ("ticket_travel", "ticket_travel_accommodation"): + add_reimbursement("travel", grant.travel_amount) + + # Add accommodation reimbursement if approved + if grant.approved_type in ( + "ticket_accommodation", + "ticket_travel_accommodation", + ): + add_reimbursement("accommodation", grant.accommodation_amount) + + +def reverse_migration(apps, schema_editor): + """Reverse the migration by deleting all reimbursements.""" + GrantReimbursement = apps.get_model("grants", "GrantReimbursement") + GrantReimbursement.objects.all().delete() + + + +class Migration(migrations.Migration): + + dependencies = [ + ('conferences', '0055_remove_conference_grants_default_accommodation_amount_and_more'), + ('grants', '0029_alter_grant_pending_status'), + ] + + operations = [ + # Create reimbursement tables + migrations.CreateModel( + name='GrantReimbursementCategory', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=100)), + ('description', models.TextField(blank=True, null=True)), + ('max_amount', models.DecimalField(decimal_places=0, help_text='Maximum amount for this category', max_digits=6)), + ('category', models.CharField(choices=[('travel', 'Travel'), ('ticket', 'Ticket'), ('accommodation', 'Accommodation'), ('other', 'Other')], max_length=20)), + ('included_by_default', models.BooleanField(default=False, help_text='Automatically include this category in grants by default')), + ('conference', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='reimbursement_categories', to='conferences.conference')), + ], + options={ + 'verbose_name': 'Grant Reimbursement Category', + 'verbose_name_plural': 'Grant Reimbursement Categories', + 'ordering': ['conference', 'category'], + 'unique_together': {('conference', 'category')}, + }, + ), + migrations.CreateModel( + name='GrantReimbursement', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('granted_amount', models.DecimalField(decimal_places=0, help_text='Actual amount granted for this category', max_digits=6, verbose_name='granted amount')), + ('grant', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='reimbursements', to='grants.grant', verbose_name='grant')), + ('category', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='grants.grantreimbursementcategory', verbose_name='reimbursement category')), + ], + options={ + 'verbose_name': 'Grant Reimbursement', + 'verbose_name_plural': 'Grant Reimbursements', + 'ordering': ['grant', 'category'], + 'unique_together': {('grant', 'category')}, + }, + ), + # Backfill existing grants + migrations.RunPython( + code=migrate_grants, + reverse_code=reverse_migration, + ), + # Finally, remove old fields + migrations.RemoveField( + model_name='grant', + name='accommodation_amount', + ), + migrations.RemoveField( + model_name='grant', + name='approved_type', + ), + migrations.RemoveField( + model_name='grant', + name='ticket_amount', + ), + migrations.RemoveField( + model_name='grant', + name='total_amount', + ), + migrations.RemoveField( + model_name='grant', + name='travel_amount', + ), + migrations.AddField( + model_name='grant', + name='reimbursement_categories', + field=models.ManyToManyField(related_name='grants', through='grants.GrantReimbursement', to='grants.grantreimbursementcategory'), + ), + ] diff --git a/backend/grants/models.py b/backend/grants/models.py index d57384d550..564f0c2d3d 100644 --- a/backend/grants/models.py +++ b/backend/grants/models.py @@ -1,9 +1,9 @@ -from conferences.querysets import ConferenceQuerySetMixin from django.db import models from django.urls import reverse from django.utils.translation import gettext_lazy as _ from model_utils.models import TimeStampedModel +from conferences.querysets import ConferenceQuerySetMixin from countries import countries from helpers.constants import GENDERS from users.models import User @@ -14,6 +14,45 @@ def of_user(self, user): return self.filter(user=user) +class GrantReimbursementCategory(models.Model): + """ + Define types of reimbursements available for a grant (e.g., Travel, Ticket, Accommodation). + """ + + class Category(models.TextChoices): + TRAVEL = "travel", _("Travel") + TICKET = "ticket", _("Ticket") + ACCOMMODATION = "accommodation", _("Accommodation") + OTHER = "other", _("Other") + + conference = models.ForeignKey( + "conferences.Conference", + on_delete=models.CASCADE, + related_name="reimbursement_categories", + ) + name = models.CharField(max_length=100) + description = models.TextField(blank=True, null=True) + max_amount = models.DecimalField( + max_digits=6, decimal_places=0, help_text=_("Maximum amount for this category") + ) + category = models.CharField(max_length=20, choices=Category.choices) + included_by_default = models.BooleanField( + default=False, + help_text="Automatically include this category in grants by default", + ) + + objects = GrantQuerySet().as_manager() + + def __str__(self): + return f"{self.name} ({self.conference.name})" + + class Meta: + verbose_name = _("Grant Reimbursement Category") + verbose_name_plural = _("Grant Reimbursement Categories") + unique_together = [("conference", "category")] + ordering = ["conference", "category"] + + class Grant(TimeStampedModel): # TextChoices class Status(models.TextChoices): @@ -63,15 +102,6 @@ class GrantType(models.TextChoices): unemployed = "unemployed", _("Unemployed") speaker = "speaker", _("Speaker") - class ApprovedType(models.TextChoices): - ticket_only = "ticket_only", _("Ticket Only") - ticket_travel = "ticket_travel", _("Ticket + Travel") - ticket_accommodation = "ticket_accommodation", _("Ticket + Accommodation") - ticket_travel_accommodation = ( - "ticket_travel_accommodation", - _("Ticket + Travel + Accommodation"), - ) - conference = models.ForeignKey( "conferences.Conference", on_delete=models.CASCADE, @@ -152,43 +182,6 @@ class ApprovedType(models.TextChoices): null=True, blank=True, ) - approved_type = models.CharField( - verbose_name=_("approved type"), - choices=ApprovedType.choices, - max_length=30, - blank=True, - null=True, - ) - - # Financial amounts - ticket_amount = models.DecimalField( - verbose_name=_("ticket amount"), - null=True, - max_digits=6, - decimal_places=2, - default=0, - ) - accommodation_amount = models.DecimalField( - verbose_name=_("accommodation amount"), - null=True, - max_digits=6, - decimal_places=2, - default=0, - ) - travel_amount = models.DecimalField( - verbose_name=_("travel amount"), - null=True, - max_digits=6, - decimal_places=2, - default=0, - ) - total_amount = models.DecimalField( - verbose_name=_("total amount"), - null=True, - max_digits=6, - decimal_places=2, - default=0, - ) country_type = models.CharField( _("Country type"), @@ -213,13 +206,16 @@ class ApprovedType(models.TextChoices): blank=True, ) + reimbursement_categories = models.ManyToManyField( + GrantReimbursementCategory, through="GrantReimbursement", related_name="grants" + ) + objects = GrantQuerySet().as_manager() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._original_status = self.status self._original_pending_status = self.pending_status - self._original_approved_type = self.approved_type self._original_country_type = self.country_type def __str__(self): @@ -227,74 +223,18 @@ def __str__(self): def save(self, *args, **kwargs): self._update_country_type() - self._calculate_grant_amounts() update_fields = kwargs.get("update_fields", None) if update_fields: - update_fields.append("total_amount") - update_fields.append("ticket_amount") - update_fields.append("accommodation_amount") - update_fields.append("travel_amount") update_fields.append("country_type") update_fields.append("pending_status") super().save(*args, **kwargs) - self._original_approved_type = self.approved_type self._original_country_type = self.country_type self._original_pending_status = self.pending_status self._original_status = self.status - def _calculate_grant_amounts(self): - if self.current_or_pending_status != Grant.Status.approved: - return - - if ( - self._original_pending_status == self.pending_status - and self._original_approved_type == self.approved_type - and self._original_country_type == self.country_type - ): - return - - conference = self.conference - self.ticket_amount = conference.grants_default_ticket_amount or 0 - self.accommodation_amount = 0 - self.travel_amount = 0 - - default_accommodation_amount = ( - conference.grants_default_accommodation_amount or 0 - ) - default_travel_from_italy_amount = ( - conference.grants_default_travel_from_italy_amount or 0 - ) - default_travel_from_europe_amount = ( - conference.grants_default_travel_from_europe_amount or 0 - ) - default_travel_from_extra_eu_amount = ( - conference.grants_default_travel_from_extra_eu_amount or 0 - ) - - if self.approved_type in ( - Grant.ApprovedType.ticket_accommodation, - Grant.ApprovedType.ticket_travel_accommodation, - ): - self.accommodation_amount = default_accommodation_amount - - if self.approved_type in ( - Grant.ApprovedType.ticket_travel_accommodation, - Grant.ApprovedType.ticket_travel, - ): - if self.country_type == Grant.CountryType.italy: - self.travel_amount = default_travel_from_italy_amount - elif self.country_type == Grant.CountryType.europe: - self.travel_amount = default_travel_from_europe_amount - elif self.country_type == Grant.CountryType.extra_eu: - self.travel_amount = default_travel_from_extra_eu_amount - - self.total_amount = ( - self.ticket_amount + self.accommodation_amount + self.travel_amount - ) - def _update_country_type(self): if not self.departure_country: return @@ -318,21 +258,56 @@ def get_admin_url(self): ) def has_approved_travel(self): - return ( - self.approved_type == Grant.ApprovedType.ticket_travel_accommodation - or self.approved_type == Grant.ApprovedType.ticket_travel - ) + return self.reimbursements.filter( + category__category=GrantReimbursementCategory.Category.TRAVEL + ).exists() def has_approved_accommodation(self): - return ( - self.approved_type == Grant.ApprovedType.ticket_accommodation - or self.approved_type == Grant.ApprovedType.ticket_travel_accommodation - ) + return self.reimbursements.filter( + category__category=GrantReimbursementCategory.Category.ACCOMMODATION + ).exists() + + @property + def total_allocated_amount(self): + return sum(r.granted_amount for r in self.reimbursements.all()) + + def has_approved(self, type_): + return self.reimbursements.filter(category__category=type_).exists() @property def current_or_pending_status(self): return self.pending_status or self.status +class GrantReimbursement(models.Model): + """Links a Grant to its reimbursement categories and stores the actual amount granted.""" + + grant = models.ForeignKey( + Grant, + on_delete=models.CASCADE, + related_name="reimbursements", + verbose_name=_("grant"), + ) + category = models.ForeignKey( + GrantReimbursementCategory, + on_delete=models.CASCADE, + verbose_name=_("reimbursement category"), + ) + granted_amount = models.DecimalField( + _("granted amount"), + max_digits=6, + decimal_places=0, + help_text=_("Actual amount granted for this category"), + ) + + def __str__(self): + return f"{self.grant.full_name} - {self.category.name} - {self.granted_amount}" + + class Meta: + verbose_name = _("Grant Reimbursement") + verbose_name_plural = _("Grant Reimbursements") + unique_together = [("grant", "category")] + ordering = ["grant", "category"] + class GrantConfirmPendingStatusProxy(Grant): class Meta: diff --git a/backend/grants/summary.py b/backend/grants/summary.py index b78a1ec0ea..da1c861a59 100644 --- a/backend/grants/summary.py +++ b/backend/grants/summary.py @@ -42,7 +42,7 @@ def calculate(self, conference_id): filtered_grants, statuses ) gender_stats = self._aggregate_data_by_gender(filtered_grants, statuses) - financial_summary, total_amount = self._aggregate_financial_data_by_status( + financial_summary, total_amount = self._aggregate_financial_data_by_status_new( filtered_grants, statuses ) grant_type_summary = self._aggregate_data_by_grant_type( @@ -51,16 +51,9 @@ def calculate(self, conference_id): speaker_status_summary = self._aggregate_data_by_speaker_status( filtered_grants, statuses ) - approved_type_summary = self._aggregate_data_by_approved_type( - filtered_grants, statuses - ) requested_needs_summary = self._aggregate_data_by_requested_needs_summary( filtered_grants, statuses ) - approved_types = { - approved_type.value: approved_type.label - for approved_type in Grant.ApprovedType - } country_types = { country_type.value: country_type.label for country_type in Grant.CountryType } @@ -68,6 +61,10 @@ def calculate(self, conference_id): filtered_grants, statuses ) + reimbursement_category_summary = self._aggregate_data_by_reimbursement_category( + filtered_grants, statuses + ) + return dict( conference_id=conference_id, conference_repr=str(conference), @@ -83,8 +80,7 @@ def calculate(self, conference_id): preselected_statuses=["approved", "confirmed"], grant_type_summary=grant_type_summary, speaker_status_summary=speaker_status_summary, - approved_type_summary=approved_type_summary, - approved_types=approved_types, + reimbursement_category_summary=reimbursement_category_summary, requested_needs_summary=requested_needs_summary, country_type_summary=country_type_summary, country_types=country_types, @@ -160,21 +156,35 @@ def _aggregate_financial_data_by_status(self, filtered_grants, statuses): """ Aggregates financial data (total amounts) by grant status. """ - financial_data = filtered_grants.values("pending_status").annotate( - total_amount_sum=Sum("total_amount") - ) financial_summary = {status[0]: 0 for status in statuses} overall_total = 0 - for data in financial_data: - pending_status = data["pending_status"] - total_amount = data["total_amount_sum"] or 0 - financial_summary[pending_status] += total_amount - if pending_status in self.BUDGET_STATUSES: - overall_total += total_amount + for status in statuses: + grants_for_status = filtered_grants.filter(pending_status=status[0]) + reimbursements = GrantReimbursement.objects.filter( + grant__in=grants_for_status + ) + total = reimbursements.aggregate(total=Sum("granted_amount"))["total"] or 0 + financial_summary[status[0]] = total + if status[0] in self.BUDGET_STATUSES: + overall_total += total return financial_summary, overall_total + def _aggregate_data_by_reimbursement_category(self, filtered_grants, statuses): + """ + Aggregates grant data by reimbursement category and status. + """ + from grants.models import GrantReimbursement + + category_summary = defaultdict(lambda: {status[0]: 0 for status in statuses}) + reimbursements = GrantReimbursement.objects.filter(grant__in=filtered_grants) + for r in reimbursements: + category = r.category.category + status = r.grant.pending_status + category_summary[category][status] += 1 + return dict(category_summary) + def _aggregate_data_by_grant_type(self, filtered_grants, statuses): """ Aggregates grant data by grant_type and status. diff --git a/backend/grants/tests/test_migration_backfill_grant_reimbursements.py b/backend/grants/tests/test_migration_backfill_grant_reimbursements.py new file mode 100644 index 0000000000..097de1e4ca --- /dev/null +++ b/backend/grants/tests/test_migration_backfill_grant_reimbursements.py @@ -0,0 +1,300 @@ +import pytest +from decimal import Decimal +from grants.models import GrantReimbursement, GrantReimbursementCategory +from grants.tests.factories import GrantFactory, GrantReimbursementCategoryFactory +from conferences.tests.factories import ConferenceFactory + +pytestmark = pytest.mark.django_db + + +@pytest.fixture(autouse=True) +def conference_with_categories(): + """Create a conference with standard reimbursement categories.""" + conference = ConferenceFactory() + + GrantReimbursementCategoryFactory( + conference=conference, + category="ticket", + name="Ticket", + description="Conference ticket", + max_amount=Decimal("100.00"), + included_by_default=True, + ) + + GrantReimbursementCategoryFactory( + conference=conference, + category="travel", + name="Travel", + description="Travel support", + max_amount=Decimal("500.00"), + included_by_default=False, + ) + + GrantReimbursementCategoryFactory( + conference=conference, + category="accommodation", + name="Accommodation", + description="Accommodation support", + max_amount=Decimal("200.00"), + included_by_default=True, + ) + + return conference + + +def _create_reimbursements_for_grant(grant): + """Simulate the migration logic for creating reimbursements from grant amounts.""" + categories = { + c.category: c + for c in GrantReimbursementCategory.objects.filter(conference=grant.conference) + } + + if "ticket" in categories and grant.ticket_amount: + GrantReimbursement.objects.get_or_create( + grant=grant, + category=categories["ticket"], + defaults={"granted_amount": grant.ticket_amount}, + ) + + if ( + grant.approved_type in ("ticket_travel", "ticket_travel_accommodation") + and "travel" in categories + and grant.travel_amount + ): + GrantReimbursement.objects.get_or_create( + grant=grant, + category=categories["travel"], + defaults={"granted_amount": grant.travel_amount}, + ) + + if ( + grant.approved_type in ("ticket_accommodation", "ticket_travel_accommodation") + and "accommodation" in categories + and grant.accommodation_amount + ): + GrantReimbursement.objects.get_or_create( + grant=grant, + category=categories["accommodation"], + defaults={"granted_amount": grant.accommodation_amount}, + ) + + +def _ensure_categories_exist_for_conference(conference): + """Create grant reimbursement categories if they don't exist.""" + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="ticket", + defaults={ + "name": "Ticket", + "description": "Conference ticket", + "max_amount": Decimal("150.00"), + "included_by_default": True, + }, + ) + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="travel", + defaults={ + "name": "Travel", + "description": "Travel support", + "max_amount": Decimal("400.00"), + "included_by_default": False, + }, + ) + GrantReimbursementCategory.objects.get_or_create( + conference=conference, + category="accommodation", + defaults={ + "name": "Accommodation", + "description": "Accommodation support", + "max_amount": Decimal("300.00"), + "included_by_default": True, + }, + ) + + +def test_creates_ticket_reimbursement_for_ticket_only_grant(conference_with_categories): + grant = GrantFactory( + conference=conference_with_categories, + approved_type="ticket_only", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("0.00"), + accommodation_amount=Decimal("0.00"), + ) + + _create_reimbursements_for_grant(grant) + + reimbursements = GrantReimbursement.objects.filter(grant=grant) + assert reimbursements.count() == 1 + + ticket_reimbursement = reimbursements.get(category__category="ticket") + assert ticket_reimbursement.granted_amount == Decimal("100.00") + + +def test_creates_ticket_and_travel_reimbursement_for_ticket_travel_grant( + conference_with_categories, +): + grant = GrantFactory( + conference=conference_with_categories, + approved_type="ticket_travel", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("400.00"), + accommodation_amount=Decimal("0.00"), + ) + + _create_reimbursements_for_grant(grant) + + reimbursements = GrantReimbursement.objects.filter(grant=grant) + assert reimbursements.count() == 2 + + ticket_reimbursement = reimbursements.get(category__category="ticket") + travel_reimbursement = reimbursements.get(category__category="travel") + + assert ticket_reimbursement.granted_amount == Decimal("100.00") + assert travel_reimbursement.granted_amount == Decimal("400.00") + + +def test_creates_ticket_and_accommodation_reimbursement_for_ticket_accommodation_grant( + conference_with_categories, +): + grant = GrantFactory( + conference=conference_with_categories, + approved_type="ticket_accommodation", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("0.00"), + accommodation_amount=Decimal("200.00"), + ) + + _create_reimbursements_for_grant(grant) + + reimbursements = GrantReimbursement.objects.filter(grant=grant) + assert reimbursements.count() == 2 + + ticket_reimbursement = reimbursements.get(category__category="ticket") + accommodation_reimbursement = reimbursements.get(category__category="accommodation") + + assert ticket_reimbursement.granted_amount == Decimal("100.00") + assert accommodation_reimbursement.granted_amount == Decimal("200.00") + + +def test_creates_all_reimbursements_for_full_grant(conference_with_categories): + grant = GrantFactory( + conference=conference_with_categories, + approved_type="ticket_travel_accommodation", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("400.00"), + accommodation_amount=Decimal("200.00"), + ) + + _create_reimbursements_for_grant(grant) + + reimbursements = GrantReimbursement.objects.filter(grant=grant) + assert reimbursements.count() == 3 + + ticket_reimbursement = reimbursements.get(category__category="ticket") + travel_reimbursement = reimbursements.get(category__category="travel") + accommodation_reimbursement = reimbursements.get(category__category="accommodation") + + assert ticket_reimbursement.granted_amount == Decimal("100.00") + assert travel_reimbursement.granted_amount == Decimal("400.00") + assert accommodation_reimbursement.granted_amount == Decimal("200.00") + + +def test_skips_grants_without_approved_type(conference_with_categories): + grant = GrantFactory( + conference=conference_with_categories, + approved_type=None, + ticket_amount=Decimal("0.00"), + travel_amount=Decimal("0.00"), + accommodation_amount=Decimal("0.00"), + ) + + if grant.approved_type is not None and grant.approved_type != "": + _create_reimbursements_for_grant(grant) + + reimbursements = GrantReimbursement.objects.filter(grant=grant) + assert reimbursements.count() == 0 + + +def test_preserves_total_amounts_after_migration(conference_with_categories): + grants = [ + GrantFactory( + conference=conference_with_categories, + approved_type="ticket_only", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("0.00"), + accommodation_amount=Decimal("0.00"), + ), + GrantFactory( + conference=conference_with_categories, + approved_type="ticket_travel", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("400.00"), + accommodation_amount=Decimal("0.00"), + ), + GrantFactory( + conference=conference_with_categories, + approved_type="ticket_travel_accommodation", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("400.00"), + accommodation_amount=Decimal("200.00"), + ), + ] + + for grant in grants: + _create_reimbursements_for_grant(grant) + + original_total = ( + grant.ticket_amount + grant.travel_amount + grant.accommodation_amount + ) + reimbursements_total = sum( + r.granted_amount for r in GrantReimbursement.objects.filter(grant=grant) + ) + assert original_total == reimbursements_total + + +def test_does_not_create_duplicates_when_run_multiple_times(conference_with_categories): + grant = GrantFactory( + conference=conference_with_categories, + approved_type="ticket_travel_accommodation", + ticket_amount=Decimal("100.00"), + travel_amount=Decimal("400.00"), + accommodation_amount=Decimal("200.00"), + ) + + _create_reimbursements_for_grant(grant) + initial_count = GrantReimbursement.objects.filter(grant=grant).count() + assert initial_count == 3 + + _create_reimbursements_for_grant(grant) + final_count = GrantReimbursement.objects.filter(grant=grant).count() + assert final_count == 3 + + +def test_creates_categories_with_conference_defaults(): + conference = ConferenceFactory( + grants_default_ticket_amount=Decimal("150.00"), + grants_default_accommodation_amount=Decimal("250.00"), + grants_default_travel_from_extra_eu_amount=Decimal("550.00"), + ) + + _ensure_categories_exist_for_conference(conference) + + categories = GrantReimbursementCategory.objects.filter(conference=conference) + assert categories.count() == 3 + + ticket_cat = categories.get(category="ticket") + travel_cat = categories.get(category="travel") + accommodation_cat = categories.get(category="accommodation") + + assert ticket_cat.name == "Ticket" + assert ticket_cat.max_amount == Decimal("150.00") + assert ticket_cat.included_by_default is True + + assert travel_cat.name == "Travel" + assert travel_cat.max_amount == Decimal("550.00") + assert travel_cat.included_by_default is False + + assert accommodation_cat.name == "Accommodation" + assert accommodation_cat.max_amount == Decimal("250.00") + assert accommodation_cat.included_by_default is True diff --git a/backend/grants/tests/test_models.py b/backend/grants/tests/test_models.py index 5bb922cb4f..b85d9710d7 100644 --- a/backend/grants/tests/test_models.py +++ b/backend/grants/tests/test_models.py @@ -1,7 +1,9 @@ -from grants.models import Grant -from grants.tests.factories import GrantFactory +from decimal import Decimal + import pytest +from grants.models import Grant, GrantReimbursement, GrantReimbursementCategory +from grants.tests.factories import GrantFactory pytestmark = pytest.mark.django_db @@ -10,35 +12,35 @@ "data", [ { - "approved_type": Grant.ApprovedType.ticket_travel, + "categories": ["ticket", "travel"], "departure_country": "IT", "expected_ticket_amount": 100, "expected_accommodation_amount": 0, "expected_travel_amount": 300, }, { - "approved_type": Grant.ApprovedType.ticket_only, + "categories": ["ticket"], "departure_country": "IT", "expected_ticket_amount": 100, "expected_accommodation_amount": 0, "expected_travel_amount": 0, }, { - "approved_type": Grant.ApprovedType.ticket_accommodation, + "categories": ["ticket", "accommodation"], "departure_country": "FR", "expected_ticket_amount": 100, "expected_accommodation_amount": 200, "expected_travel_amount": 0, }, { - "approved_type": Grant.ApprovedType.ticket_travel, + "categories": ["ticket", "travel"], "departure_country": "FR", "expected_ticket_amount": 100, "expected_accommodation_amount": 0, "expected_travel_amount": 400, }, { - "approved_type": Grant.ApprovedType.ticket_travel_accommodation, + "categories": ["ticket", "travel", "accommodation"], "departure_country": "AU", "expected_ticket_amount": 100, "expected_accommodation_amount": 200, @@ -47,98 +49,136 @@ ], ) def test_calculate_grant_amounts(data): - approved_type = data["approved_type"] + categories = data["categories"] departure_country = data["departure_country"] expected_ticket_amount = data["expected_ticket_amount"] expected_accommodation_amount = data["expected_accommodation_amount"] expected_travel_amount = data["expected_travel_amount"] grant = GrantFactory( - pending_status=Grant.Status.pending, - approved_type=approved_type, + pending_status=Grant.Status.approved, departure_country=departure_country, - conference__grants_default_ticket_amount=100, - conference__grants_default_accommodation_amount=200, - conference__grants_default_travel_from_italy_amount=300, - conference__grants_default_travel_from_europe_amount=400, - conference__grants_default_travel_from_extra_eu_amount=500, + ) + conference = grant.conference + + ticket_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TICKET, + name="Ticket", + max_amount=Decimal("100"), + included_by_default=True, + ) + travel_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TRAVEL, + name="Travel", + max_amount=Decimal("500"), + included_by_default=False, + ) + accommodation_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.ACCOMMODATION, + name="Accommodation", + max_amount=Decimal("200"), + included_by_default=False, ) - grant.pending_status = Grant.Status.approved - grant.save() + # Create reimbursements based on categories + if "ticket" in categories: + GrantReimbursement.objects.update_or_create( + grant=grant, + category=ticket_category, + defaults={"granted_amount": Decimal(expected_ticket_amount)}, + ) + if "travel" in categories: + GrantReimbursement.objects.update_or_create( + grant=grant, + category=travel_category, + defaults={"granted_amount": Decimal(expected_travel_amount)}, + ) + if "accommodation" in categories: + GrantReimbursement.objects.update_or_create( + grant=grant, + category=accommodation_category, + defaults={"granted_amount": Decimal(expected_accommodation_amount)}, + ) grant.refresh_from_db() - assert grant.ticket_amount == expected_ticket_amount - assert grant.accommodation_amount == expected_accommodation_amount - assert grant.travel_amount == expected_travel_amount - assert ( - grant.total_amount - == expected_ticket_amount - + expected_accommodation_amount - + expected_travel_amount + # Verify individual reimbursement amounts + if "ticket" in categories: + ticket_reimbursement = GrantReimbursement.objects.get( + grant=grant, category=ticket_category + ) + assert ticket_reimbursement.granted_amount == Decimal(expected_ticket_amount) + else: + assert not GrantReimbursement.objects.filter( + grant=grant, category=ticket_category + ).exists() + + if "travel" in categories: + travel_reimbursement = GrantReimbursement.objects.get( + grant=grant, category=travel_category + ) + assert travel_reimbursement.granted_amount == Decimal(expected_travel_amount) + else: + assert not GrantReimbursement.objects.filter( + grant=grant, category=travel_category + ).exists() + + if "accommodation" in categories: + accommodation_reimbursement = GrantReimbursement.objects.get( + grant=grant, category=accommodation_category + ) + assert accommodation_reimbursement.granted_amount == Decimal( + expected_accommodation_amount + ) + else: + assert not GrantReimbursement.objects.filter( + grant=grant, category=accommodation_category + ).exists() + + # Verify total_allocated_amount sums correctly + expected_total = ( + expected_ticket_amount + expected_accommodation_amount + expected_travel_amount ) + assert grant.total_allocated_amount == Decimal(expected_total) -def test_resets_amounts_on_approved_type_change(): - grant = GrantFactory( - pending_status=Grant.Status.pending, - approved_type=Grant.ApprovedType.ticket_only, - departure_country="IT", - conference__grants_default_ticket_amount=100, - conference__grants_default_accommodation_amount=200, - conference__grants_default_travel_from_italy_amount=300, - conference__grants_default_travel_from_europe_amount=400, - conference__grants_default_travel_from_extra_eu_amount=500, +def test_has_approved_travel(): + grant = GrantFactory() + travel_category = GrantReimbursementCategory.objects.create( + conference=grant.conference, + category=GrantReimbursementCategory.Category.TRAVEL, + name="Travel", + max_amount=Decimal("500"), + included_by_default=False, ) - - grant.pending_status = Grant.Status.approved - grant.save() - - assert grant.ticket_amount == 100 - assert grant.accommodation_amount == 0 - assert grant.travel_amount == 0 - assert grant.total_amount == 100 - - grant.approved_type = Grant.ApprovedType.ticket_travel_accommodation - grant.save() - - assert grant.ticket_amount == 100 - assert grant.accommodation_amount == 200 - assert grant.travel_amount == 300 - assert grant.total_amount == 600 - - -def test_can_manually_change_amounts(): - grant = GrantFactory( - pending_status=Grant.Status.pending, - approved_type=Grant.ApprovedType.ticket_only, - departure_country="IT", - conference__grants_default_ticket_amount=100, - conference__grants_default_accommodation_amount=200, - conference__grants_default_travel_from_italy_amount=300, - conference__grants_default_travel_from_europe_amount=400, - conference__grants_default_travel_from_extra_eu_amount=500, + GrantReimbursement.objects.create( + grant=grant, + category=travel_category, + granted_amount=Decimal("500"), ) - grant.pending_status = Grant.Status.approved - grant.save(update_fields=["pending_status"]) + assert grant.has_approved_travel() - assert grant.ticket_amount == 100 - assert grant.accommodation_amount == 0 - assert grant.travel_amount == 0 - assert grant.total_amount == 100 - grant.ticket_amount = 20 - grant.accommodation_amount = 50 - grant.travel_amount = 0 - grant.total_amount = 70 - grant.save() +def test_has_approved_accommodation(): + grant = GrantFactory() + accommodation_category = GrantReimbursementCategory.objects.create( + conference=grant.conference, + category=GrantReimbursementCategory.Category.ACCOMMODATION, + name="Accommodation", + max_amount=Decimal("200"), + included_by_default=False, + ) + GrantReimbursement.objects.create( + grant=grant, + category=accommodation_category, + granted_amount=Decimal("200"), + ) - assert grant.ticket_amount == 20 - assert grant.accommodation_amount == 50 - assert grant.travel_amount == 0 - assert grant.total_amount == 70 + assert grant.has_approved_accommodation() @pytest.mark.parametrize( @@ -193,7 +233,7 @@ def test_doesnt_sync_pending_status_if_different_values(): assert grant.pending_status == Grant.Status.refused assert grant.status == Grant.Status.waiting_for_confirmation - +@pytest.mark.skip(reason="We don't automatically create on save anymore") def test_pending_status_none_means_no_pending_change(): grant = GrantFactory( pending_status=None, @@ -210,6 +250,7 @@ def test_pending_status_none_means_no_pending_change(): assert grant.ticket_amount is not None +@pytest.mark.skip(reason="We don't automatically create on save anymore") def test_pending_status_set_overrides_current_status(): grant = GrantFactory( pending_status=Grant.Status.approved, diff --git a/backend/reviews/admin.py b/backend/reviews/admin.py index 4ec1e5af6d..1f29b66998 100644 --- a/backend/reviews/admin.py +++ b/backend/reviews/admin.py @@ -1,25 +1,34 @@ -from django.contrib.postgres.expressions import ArraySubquery -from django.db.models.expressions import ExpressionWrapper -from django.db.models import FloatField -from django.db.models.functions import Cast -from users.admin_mixins import ConferencePermissionMixin -from django.core.exceptions import PermissionDenied -from django.db.models import Q, Exists import urllib.parse from django import forms from django.contrib import admin, messages -from django.db.models import Count, F, OuterRef, Prefetch, Subquery, Sum, Avg +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, + 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 grants.models import Grant +from grants.models import Grant, GrantReimbursement, GrantReimbursementCategory from participants.models import Participant from reviews.models import AvailableScoreOption, ReviewSession, UserReview from submissions.models import Submission, SubmissionTag +from users.admin_mixins import ConferencePermissionMixin from users.models import User @@ -259,16 +268,20 @@ def _review_grants_recap_view(self, request, 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_type_decisions = { - int(key.split("-")[1]): value - for [key, value] in data.items() - if key.startswith("approvedtype-") + 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( @@ -280,21 +293,34 @@ def _review_grants_recap_view(self, request, review_session): if decision not in Grant.REVIEW_SESSION_STATUSES_OPTIONS: continue - approved_type = approved_type_decisions.get(grant.id, "") - if decision != grant.status: grant.pending_status = decision elif decision == grant.status: grant.pending_status = None - grant.approved_type = ( - approved_type if decision == Grant.Status.approved else None - ) + # 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: + grant.reimbursements.all().delete() + else: + # Only keep those in current approved_reimbursement_categories + grant.reimbursements.exclude( + category_id__in=approved_reimbursement_categories + ).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", "approved_type"]) + grant.save(update_fields=["pending_status",]) + approved_reimbursement_categories = approved_reimbursement_categories_decisions.get(grant.id, "") + for reimbursement_category_id in approved_reimbursement_categories: + GrantReimbursement.objects.update_or_create( + grant=grant, + category_id=reimbursement_category_id, + defaults={"granted_amount": reimbursement_categories[reimbursement_category_id].max_amount}, + ) messages.success( request, "Decisions saved. Check the Grants Summary for more info." @@ -343,6 +369,11 @@ def _review_grants_recap_view(self, request, review_session): ) .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( @@ -380,7 +411,7 @@ def _review_grants_recap_view(self, request, review_session): if choice[0] in Grant.REVIEW_SESSION_STATUSES_OPTIONS ], all_statuses=Grant.Status.choices, - all_approved_types=[choice for choice in Grant.ApprovedType.choices], + all_reimbursement_categories=GrantReimbursementCategory.objects.for_conference(conference=review_session.conference), review_session=review_session, title="Recap", ) diff --git a/backend/reviews/templates/grants-recap.html b/backend/reviews/templates/grants-recap.html index d41ddfb107..9177c9d416 100644 --- a/backend/reviews/templates/grants-recap.html +++ b/backend/reviews/templates/grants-recap.html @@ -643,21 +643,19 @@

data-item-id="{{ item.id }}" class="approved-type-choices {% if item.current_or_pending_status != 'approved' %}hidden{% endif %}" > - {% for approved_type in all_approved_types %} + {% for reimbursement_category in all_reimbursement_categories %}
    • {% endfor %} -
  • - -
    • {% else %} No permission to change. {% endif %} diff --git a/backend/reviews/tests/test_admin.py b/backend/reviews/tests/test_admin.py index 061746d87a..7fe0ddd856 100644 --- a/backend/reviews/tests/test_admin.py +++ b/backend/reviews/tests/test_admin.py @@ -1,7 +1,13 @@ -from conferences.tests.factories import ConferenceFactory +from decimal import Decimal + +import pytest from django.contrib.admin import AdminSite + +from conferences.tests.factories import ConferenceFactory +from grants.models import Grant, GrantReimbursement, GrantReimbursementCategory from grants.tests.factories import GrantFactory -import pytest +from reviews.admin import ReviewSessionAdmin, get_next_to_review_item_id +from reviews.models import ReviewSession from reviews.tests.factories import ( AvailableScoreOptionFactory, ReviewSessionFactory, @@ -10,9 +16,6 @@ from submissions.tests.factories import SubmissionFactory, SubmissionTagFactory from users.tests.factories import UserFactory -from reviews.admin import ReviewSessionAdmin, get_next_to_review_item_id -from reviews.models import ReviewSession - pytestmark = pytest.mark.django_db @@ -269,3 +272,241 @@ def test_review_start_view(rf, mocker): response.url == f"/admin/reviews/reviewsession/{review_session.id}/review/{submission_1.id}/" ) + + +def test_save_review_grants_updates_grant_and_creates_reimbursements(rf, mocker): + mock_messages = mocker.patch("reviews.admin.messages") + + user = UserFactory(is_staff=True, is_superuser=True) + conference = ConferenceFactory() + + # Create reimbursement categories + travel_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TRAVEL, + name="Travel", + max_amount=Decimal("500"), + included_by_default=False, + ) + ticket_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TICKET, + name="Ticket", + max_amount=Decimal("100"), + included_by_default=True, + ) + accommodation_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.ACCOMMODATION, + name="Accommodation", + max_amount=Decimal("200"), + included_by_default=False, + ) + + # Create review session for grants + review_session = ReviewSessionFactory( + conference=conference, + session_type=ReviewSession.SessionType.GRANTS, + status=ReviewSession.Status.COMPLETED, + ) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=0) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=1) + + # Create grants with initial status + grant_1 = GrantFactory(conference=conference, status=Grant.Status.pending) + grant_2 = GrantFactory(conference=conference, status=Grant.Status.pending) + + # Build POST data + # Note: The current admin code uses data.items() which only keeps the last value + # when multiple checkboxes have the same name. For multiple categories, the code + # would need to use request.POST.getlist(). Testing with one category per grant. + post_data = { + f"decision-{grant_1.id}": Grant.Status.approved, + f"reimbursementcategory-{grant_1.id}": [str(ticket_category.id), str(travel_category.id)], + f"decision-{grant_2.id}": Grant.Status.approved, + f"reimbursementcategory-{grant_2.id}": [str(ticket_category.id), str(travel_category.id),str(accommodation_category.id),] + } + + 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/" + ) + + # Refresh grants from database + grant_1.refresh_from_db() + grant_2.refresh_from_db() + + # Verify grants were updated with pending_status + assert grant_1.pending_status == Grant.Status.approved + assert grant_2.pending_status == Grant.Status.approved + + # Verify GrantReimbursement objects were created + assert grant_1.reimbursements.count() == 2 + assert { reimbursement.category for reimbursement in grant_1.reimbursements.all() } == { ticket_category, travel_category } + + assert grant_2.reimbursements.count() == 3 + assert { reimbursement.category for reimbursement in grant_2.reimbursements.all() } == { ticket_category, travel_category, accommodation_category } + + mock_messages.success.assert_called_once() + + +def test_save_review_grants_update_grants_status_to_rejected_removes_reimbursements(rf, mocker): + mock_messages = mocker.patch("reviews.admin.messages") + + user = UserFactory(is_staff=True, is_superuser=True) + conference = ConferenceFactory() + + # Create reimbursement categories + travel_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TRAVEL, + name="Travel", + max_amount=Decimal("500"), + included_by_default=False, + ) + ticket_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TICKET, + name="Ticket", + max_amount=Decimal("100"), + included_by_default=True, + ) + accommodation_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.ACCOMMODATION, + name="Accommodation", + max_amount=Decimal("200"), + included_by_default=False, + ) + + # Create review session for grants + review_session = ReviewSessionFactory( + conference=conference, + session_type=ReviewSession.SessionType.GRANTS, + status=ReviewSession.Status.COMPLETED, + ) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=0) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=1) + + # Create grants with initial status + grant_1 = GrantFactory(conference=conference, status=Grant.Status.approved) + GrantReimbursement.objects.create( + grant=grant_1, + category=travel_category, + granted_amount=Decimal("500"), + ) + GrantReimbursement.objects.create( + grant=grant_1, + category=ticket_category, + granted_amount=Decimal("100"), + ) + GrantReimbursement.objects.create( + grant=grant_1, + category=accommodation_category, + granted_amount=Decimal("200"), + ) + + # Build POST data + post_data = { + f"decision-{grant_1.id}": Grant.Status.rejected, + f"reimbursementcategory-{grant_1.id}": [], + } + + 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/" + ) + grant_1.refresh_from_db() + + assert grant_1.pending_status == Grant.Status.rejected + + assert grant_1.reimbursements.count() == 0 + +def test_save_review_grants_modify_reimbursements(rf, mocker): + mock_messages = mocker.patch("reviews.admin.messages") + + user = UserFactory(is_staff=True, is_superuser=True) + conference = ConferenceFactory() + + # Create reimbursement categories + travel_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TRAVEL, + name="Travel", + max_amount=Decimal("500"), + included_by_default=False, + ) + ticket_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.TICKET, + name="Ticket", + max_amount=Decimal("100"), + included_by_default=True, + ) + accommodation_category = GrantReimbursementCategory.objects.create( + conference=conference, + category=GrantReimbursementCategory.Category.ACCOMMODATION, + name="Accommodation", + max_amount=Decimal("200"), + included_by_default=False, + ) + + # Create review session for grants + review_session = ReviewSessionFactory( + conference=conference, + session_type=ReviewSession.SessionType.GRANTS, + status=ReviewSession.Status.COMPLETED, + ) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=0) + AvailableScoreOptionFactory(review_session=review_session, numeric_value=1) + + # Create grants with initial status + grant_1 = GrantFactory(conference=conference, status=Grant.Status.approved) + GrantReimbursement.objects.create( + grant=grant_1, + category=travel_category, + granted_amount=Decimal("500"), + ) + GrantReimbursement.objects.create( + grant=grant_1, + category=ticket_category, + granted_amount=Decimal("100"), + ) + GrantReimbursement.objects.create( + grant=grant_1, + category=accommodation_category, + granted_amount=Decimal("200"), + ) + + # Removing the travel and accommodation reimbursements + post_data = { + f"decision-{grant_1.id}": Grant.Status.approved, + f"reimbursementcategory-{grant_1.id}": [str(ticket_category.id)], + } + + request = rf.post("/", data=post_data) + request.user = user + + admin = ReviewSessionAdmin(ReviewSession, AdminSite()) + response = admin._review_grants_recap_view(request, review_session) + + grant_1.refresh_from_db() + + assert grant_1.reimbursements.count() == 1 + assert { reimbursement.category for reimbursement in grant_1.reimbursements.all() } == { ticket_category } From 34ae2827d5e04a3167dcc5c86641a10e8f9f0a72 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 11:58:20 +0000 Subject: [PATCH 2/2] Initial plan