diff --git a/backend/api/grants/mutations.py b/backend/api/grants/mutations.py index 171f3ea8a2..4ff0d1cccb 100644 --- a/backend/api/grants/mutations.py +++ b/backend/api/grants/mutations.py @@ -10,6 +10,10 @@ from api.permissions import IsAuthenticated from api.types import BaseErrorType from conferences.models.conference import Conference +from custom_admin.audit import ( + create_addition_admin_log_entry, + create_change_admin_log_entry, +) from grants.models import Grant as GrantModel from grants.tasks import get_name, notify_new_grant_reply_slack from notifications.models import EmailTemplate, EmailTemplateIdentifier @@ -279,11 +283,14 @@ def send_grant(self, info: Info, input: SendGrantInput) -> SendGrantResult: }, ) + create_addition_admin_log_entry(request.user, instance, "Grant created.") + # hack because we return django models instance.__strawberry_definition__ = Grant.__strawberry_definition__ return instance @strawberry.mutation(permission_classes=[IsAuthenticated]) + @transaction.atomic def update_grant(self, info: Info, input: UpdateGrantInput) -> UpdateGrantResult: request = info.context.request @@ -299,8 +306,11 @@ def update_grant(self, info: Info, input: UpdateGrantInput) -> UpdateGrantResult for attr, value in asdict(input).items(): setattr(instance, attr, value) + instance.save() + create_change_admin_log_entry(request.user, instance, "Grant updated.") + Participant.objects.update_or_create( user_id=request.user.id, conference=instance.conference, @@ -335,6 +345,10 @@ def send_grant_reply( grant.status = input.status.to_grant_status() grant.save() + create_change_admin_log_entry( + request.user, grant, f"Grantee has replied with status {grant.status}." + ) + admin_url = request.build_absolute_uri(grant.get_admin_url()) notify_new_grant_reply_slack.delay(grant_id=grant.id, admin_url=admin_url) diff --git a/backend/api/grants/tests/test_send_grant.py b/backend/api/grants/tests/test_send_grant.py index 9b75f639c3..b09bed14f4 100644 --- a/backend/api/grants/tests/test_send_grant.py +++ b/backend/api/grants/tests/test_send_grant.py @@ -1,4 +1,5 @@ import pytest +from django.contrib.admin.models import LogEntry from conferences.tests.factories import ConferenceFactory from grants.models import Grant @@ -119,6 +120,14 @@ def test_send_grant( user=user, conference=conference, privacy_policy="grant" ).exists() + # Verify a log entry is created for the grant creation + assert LogEntry.objects.count() == 1 + log_entry = LogEntry.objects.first() + assert log_entry.user_id == user.id + assert log_entry.user == user + assert log_entry.object_id == str(grant.id) + assert log_entry.change_message == "Grant created." + # Verify that the correct email template was used and email was sent emails_sent = sent_emails() assert emails_sent.count() == 1 diff --git a/backend/api/grants/tests/test_send_grant_reply.py b/backend/api/grants/tests/test_send_grant_reply.py index 22e2fede44..44c637b9b6 100644 --- a/backend/api/grants/tests/test_send_grant_reply.py +++ b/backend/api/grants/tests/test_send_grant_reply.py @@ -1,9 +1,10 @@ from unittest.mock import ANY -from grants.tests.factories import GrantFactory import pytest +from django.contrib.admin.models import LogEntry from grants.models import Grant +from grants.tests.factories import GrantFactory from users.tests.factories import UserFactory pytestmark = pytest.mark.django_db @@ -85,6 +86,13 @@ def test_status_is_updated_when_reply_is_confirmed(graphql_client, user): grant.refresh_from_db() assert grant.status == Grant.Status.confirmed + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=user, + object_id=grant.id, + change_message="Grantee has replied with status confirmed.", + ).exists() + def test_status_is_updated_when_reply_is_refused(graphql_client, user): graphql_client.force_login(user) @@ -97,6 +105,13 @@ def test_status_is_updated_when_reply_is_refused(graphql_client, user): grant.refresh_from_db() assert grant.status == Grant.Status.refused + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=user, + object_id=grant.id, + change_message="Grantee has replied with status refused.", + ).exists() + def test_call_notify_new_grant_reply(graphql_client, user, mocker): graphql_client.force_login(user) diff --git a/backend/api/grants/tests/test_update_grant.py b/backend/api/grants/tests/test_update_grant.py index c96fe6807f..cc8b2efacb 100644 --- a/backend/api/grants/tests/test_update_grant.py +++ b/backend/api/grants/tests/test_update_grant.py @@ -1,4 +1,5 @@ from users.tests.factories import UserFactory +from django.contrib.admin.models import LogEntry from conferences.tests.factories import ConferenceFactory from grants.tests.factories import GrantFactory import pytest @@ -129,6 +130,13 @@ def test_update_grant(graphql_client, user): assert participant.facebook_url == "http://facebook.com/pythonpizza" assert participant.linkedin_url == "http://linkedin.com/company/pythonpizza" + assert LogEntry.objects.count() == 1 + log_entry = LogEntry.objects.first() + assert log_entry.user_id == user.id + assert log_entry.user == user + assert log_entry.object_id == str(grant.id) + assert log_entry.change_message == "Grant updated." + def test_cannot_update_a_grant_if_user_is_not_owner( graphql_client, diff --git a/backend/custom_admin/admin.py b/backend/custom_admin/admin.py index a1fe84fb7c..a832dc40ad 100644 --- a/backend/custom_admin/admin.py +++ b/backend/custom_admin/admin.py @@ -6,6 +6,8 @@ from django.contrib import admin from django.urls import path +from custom_admin.audit import create_change_admin_log_entry + SITE_NAME = "PyCon Italia" admin.site.site_header = SITE_NAME @@ -58,15 +60,50 @@ def wrapper(modeladmin, request, queryset): @admin.action(description="Confirm pending status change") @validate_single_conference_selection def confirm_pending_status(modeladmin, request, queryset): + """ + Efficiently bulk-update status with pending_status, and accurately log the change per object. + """ + # Use values_list to fetch ids and old statuses before updating. + changed_objs_info = list(queryset.values_list("pk", "status", "pending_status")) + + # Perform the bulk update. queryset.update( status=F("pending_status"), pending_status=None, ) + model = queryset.model + for pk, old_status, pending_status in changed_objs_info: + obj = model.objects.get(pk=pk) + create_change_admin_log_entry( + request.user, + obj, + change_message=( + f"[Bulk Admin Action] Status changed from '{old_status}' to '{pending_status}'." + ), + ) + @admin.action(description="Reset pending status to status") @validate_single_conference_selection def reset_pending_status_back_to_status(modeladmin, request, queryset): + """ + Efficiently bulk-reset pending_status to None, and accurately log the change per object. + """ + changed_objs_info = list(queryset.values_list("pk", "pending_status")) + queryset.update( pending_status=None, ) + + model = queryset.model + for pk, old_pending_status in changed_objs_info: + if old_pending_status is not None: + obj = model.objects.get(pk=pk) + create_change_admin_log_entry( + request.user, + obj, + change_message=( + f"[Bulk Admin Action] pending_status reset from '{old_pending_status}' to None." + ), + ) diff --git a/backend/grants/admin.py b/backend/grants/admin.py index 4a73c6fb0f..8e9f732941 100644 --- a/backend/grants/admin.py +++ b/backend/grants/admin.py @@ -218,6 +218,11 @@ def send_reply_emails(modeladmin, request, queryset): grant.save() send_grant_reply_approved_email.delay(grant_id=grant.id, is_reminder=False) + create_change_admin_log_entry( + request.user, + grant, + change_message="Sent Approved reply email to applicant.", + ) messages.info(request, f"Sent Approved reply email to {grant.name}") if ( @@ -225,10 +230,20 @@ def send_reply_emails(modeladmin, request, queryset): or grant.status == Grant.Status.waiting_list_maybe ): send_grant_reply_waiting_list_email.delay(grant_id=grant.id) + create_change_admin_log_entry( + request.user, + grant, + change_message="Sent Waiting List reply email to applicant.", + ) messages.info(request, f"Sent Waiting List reply email to {grant.name}") if grant.status == Grant.Status.rejected: send_grant_reply_rejected_email.delay(grant_id=grant.id) + create_change_admin_log_entry( + request.user, + grant, + change_message="Sent Rejected reply email to applicant.", + ) messages.info(request, f"Sent Rejected reply email to {grant.name}") @@ -252,6 +267,11 @@ def send_grant_reminder_to_waiting_for_confirmation(modeladmin, request, queryse send_grant_reply_approved_email.delay(grant_id=grant.id, is_reminder=True) + create_change_admin_log_entry( + request.user, + grant, + change_message="Sent Approved reminder email to applicant.", + ) messages.info(request, f"Grant reminder sent to {grant.name}") @@ -267,6 +287,11 @@ def send_reply_email_waiting_list_update(modeladmin, request, queryset): for grant in queryset: send_grant_reply_waiting_list_update_email.delay(grant_id=grant.id) + create_change_admin_log_entry( + request.user, + grant, + change_message="Sent Waiting List update reply email to applicant.", + ) messages.info(request, f"Sent Waiting List update reply email to {grant.name}") @@ -300,7 +325,7 @@ def create_grant_vouchers(modeladmin, request, queryset): create_addition_admin_log_entry( request.user, grant, - change_message="Created voucher for this grant", + change_message="Created voucher for this grant.", ) vouchers_to_create.append( @@ -321,12 +346,12 @@ def create_grant_vouchers(modeladmin, request, queryset): create_change_admin_log_entry( request.user, existing_voucher, - change_message="Upgraded Co-Speaker voucher to Grant voucher", + change_message="Upgraded Co-Speaker voucher to Grant voucher.", ) create_change_admin_log_entry( request.user, grant, - change_message="Updated existing Co-Speaker voucher to grant", + change_message="Updated existing Co-Speaker voucher to grant.", ) existing_voucher.voucher_type = ConferenceVoucher.VoucherType.GRANT vouchers_to_update.append(existing_voucher) @@ -348,9 +373,16 @@ def mark_rejected_and_send_email(modeladmin, request, queryset): ) for grant in queryset: + old_status = grant.status grant.status = Grant.Status.rejected grant.save() + create_change_admin_log_entry( + request.user, + grant, + change_message=f"Status changed from '{old_status}' to 'rejected' and rejection email sent.", + ) + send_grant_reply_rejected_email.delay(grant_id=grant.id) messages.info(request, f"Sent Rejected reply email to {grant.name}") @@ -405,6 +437,14 @@ class GrantReimbursementAdmin(ConferencePermissionMixin, admin.ModelAdmin): search_fields = ("grant__full_name", "grant__email") autocomplete_fields = ("grant",) + def delete_model(self, request, obj): + create_change_admin_log_entry( + request.user, + obj.grant, + change_message=f"Reimbursement removed: {obj.category.name}.", + ) + super().delete_model(request, obj) + class GrantReimbursementInline(admin.TabularInline): model = GrantReimbursement @@ -412,6 +452,14 @@ class GrantReimbursementInline(admin.TabularInline): autocomplete_fields = ["category"] fields = ["category", "granted_amount"] + def delete_model(self, request, obj): + create_change_admin_log_entry( + request.user, + obj.grant, + change_message=f"Reimbursement removed: {obj.category.name}.", + ) + super().delete_model(request, obj) + @admin.register(Grant) class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): @@ -516,6 +564,31 @@ class GrantAdmin(ExportMixin, ConferencePermissionMixin, admin.ModelAdmin): ), ) + def save_model(self, request, obj, form, change): + """ + Override to log admin actions when status is changed. + """ + if change: + if obj.status != obj._original_status: + create_change_admin_log_entry( + request.user, + obj, + change_message=f"Status changed from '{obj._original_status}' to '{obj.status}'.", + ) + if obj.pending_status != obj._original_pending_status: + create_change_admin_log_entry( + request.user, + obj, + change_message=f"Pending status changed from '{obj._original_pending_status}' to '{obj.pending_status}'.", + ) + else: + create_addition_admin_log_entry( + request.user, + obj, + change_message="Grant created.", + ) + super().save_model(request, obj, form, change) + def change_view(self, request, object_id, form_url="", extra_context=None): extra_context = extra_context or {} grant = self.model.objects.get(id=object_id) diff --git a/backend/grants/tests/test_admin.py b/backend/grants/tests/test_admin.py index 14a15183d8..e5662e3c27 100644 --- a/backend/grants/tests/test_admin.py +++ b/backend/grants/tests/test_admin.py @@ -3,22 +3,28 @@ from unittest.mock import call import pytest +from django.contrib.admin.models import LogEntry +from django.contrib.admin.sites import AdminSite from django.utils import timezone from conferences.models.conference_voucher import ConferenceVoucher from conferences.tests.factories import ConferenceFactory, ConferenceVoucherFactory from grants.admin import ( confirm_pending_status, + GrantAdmin, + GrantReimbursementAdmin, create_grant_vouchers, mark_rejected_and_send_email, reset_pending_status_back_to_status, + send_grant_reminder_to_waiting_for_confirmation, + send_reply_email_waiting_list_update, send_reply_emails, ) -from grants.models import Grant from grants.tests.factories import ( GrantFactory, GrantReimbursementFactory, ) +from grants.models import Grant, GrantReimbursement pytestmark = pytest.mark.django_db @@ -61,6 +67,7 @@ def test_send_reply_emails_with_grants_from_multiple_conferences_fails( mock_send_approved_email.assert_not_called() mock_send_waiting_list_email.assert_not_called() mock_send_rejected_email.assert_not_called() + assert not LogEntry.objects.exists() def test_send_reply_emails_approved_grant_missing_reimbursements( @@ -81,6 +88,7 @@ def test_send_reply_emails_approved_grant_missing_reimbursements( f"Grant for {grant.name} is missing reimbursement categories!", ) mock_send_approved_email.assert_not_called() + assert not LogEntry.objects.exists() def test_send_reply_emails_approved_missing_amount(rf, mocker, admin_user): @@ -106,6 +114,7 @@ def test_send_reply_emails_approved_missing_amount(rf, mocker, admin_user): f"Grant for {grant.name} is missing 'Total Amount'!", ) mock_send_approved_email.assert_not_called() + assert not LogEntry.objects.exists() def test_send_reply_emails_approved_set_deadline_in_fourteen_days( @@ -134,20 +143,31 @@ def test_send_reply_emails_approved_set_deadline_in_fourteen_days( send_reply_emails(None, request=request, queryset=Grant.objects.all()) + # Verify admin action was called correctly mock_messages.info.assert_called_once_with( request, f"Sent Approved reply email to {grant.name}", ) - mock_send_approved_email.assert_called_once_with( - grant_id=grant.id, is_reminder=False - ) + # Verify deadline was set correctly grant.refresh_from_db() assert ( f"{grant.applicant_reply_deadline:%Y-%m-%d}" == f"{(timezone.now().date() + timedelta(days=14)):%Y-%m-%d}" ) + # Verify task was queued correctly + mock_send_approved_email.assert_called_once_with( + grant_id=grant.id, is_reminder=False + ) + + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Approved reply email to applicant.", + ).exists() + def test_send_reply_emails_waiting_list(rf, mocker, admin_user): mock_messages = mocker.patch("grants.admin.messages") @@ -166,6 +186,11 @@ def test_send_reply_emails_waiting_list(rf, mocker, admin_user): request, f"Sent Waiting List reply email to {grant.name}" ) mock_send_waiting_list_email.assert_called_once_with(grant_id=grant.id) + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Waiting List reply email to applicant.", + ).exists() def test_send_reply_emails_waiting_list_maybe(rf, mocker, admin_user): @@ -185,6 +210,11 @@ def test_send_reply_emails_waiting_list_maybe(rf, mocker, admin_user): request, f"Sent Waiting List reply email to {grant.name}" ) mock_send_waiting_list_email.assert_called_once_with(grant_id=grant.id) + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Waiting List reply email to applicant.", + ).exists() def test_send_reply_emails_rejected(rf, mocker, admin_user): @@ -204,6 +234,73 @@ def test_send_reply_emails_rejected(rf, mocker, admin_user): request, f"Sent Rejected reply email to {grant.name}" ) mock_send_rejected_email.assert_called_once_with(grant_id=grant.id) + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Rejected reply email to applicant.", + ).exists() + + +def test_send_grant_reminder_to_waiting_for_confirmation(rf, mocker, admin_user): + mock_messages = mocker.patch("grants.admin.messages") + grant = GrantFactory(status=Grant.Status.waiting_for_confirmation) + request = rf.get("/") + request.user = admin_user + mock_send_approved_reminder_email = mocker.patch( + "grants.admin.send_grant_reply_approved_email.delay" + ) + + send_grant_reminder_to_waiting_for_confirmation( + None, request=request, queryset=Grant.objects.all() + ) + + # Verify admin action was called correctly + mock_messages.info.assert_called_once_with( + request, + f"Grant reminder sent to {grant.name}", + ) + + # Verify task was queued correctly + mock_send_approved_reminder_email.assert_called_once_with( + grant_id=grant.id, is_reminder=True + ) + + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Approved reminder email to applicant.", + ).exists() + + +def test_send_reply_email_waiting_list_update(rf, mocker, admin_user): + mock_messages = mocker.patch("grants.admin.messages") + grant = GrantFactory(status=Grant.Status.waiting_list) + request = rf.get("/") + request.user = admin_user + mock_send_waiting_list_update_email = mocker.patch( + "grants.admin.send_grant_reply_waiting_list_update_email.delay" + ) + + send_reply_email_waiting_list_update( + None, request=request, queryset=Grant.objects.all() + ) + + # Verify admin action was called correctly + mock_messages.info.assert_called_once_with( + request, + f"Sent Waiting List update reply email to {grant.name}", + ) + + # Verify task was queued correctly + mock_send_waiting_list_update_email.assert_called_once_with(grant_id=grant.id) + + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Sent Waiting List update reply email to applicant.", + ).exists() def test_create_grant_vouchers(rf, mocker, admin_user): @@ -241,6 +338,16 @@ def test_create_grant_vouchers(rf, mocker, admin_user): request, "Vouchers created!", ) + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant_1.id, + change_message="Created voucher for this grant.", + ).exists() + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant_2.id, + change_message="Created voucher for this grant.", + ).exists() @pytest.mark.parametrize( @@ -451,6 +558,8 @@ def test_mark_rejected_and_send_email(rf, mocker, admin_user): grant2.refresh_from_db() assert grant1.status == Grant.Status.rejected assert grant2.status == Grant.Status.rejected + + # Verify admin action was called correctly mock_messages.info.assert_has_calls( [ call(request, f"Sent Rejected reply email to {grant1.name}"), @@ -458,12 +567,26 @@ def test_mark_rejected_and_send_email(rf, mocker, admin_user): ], any_order=True, ) + + # Verify task was queued correctly mock_send_rejected_email.assert_has_calls( [call(grant_id=grant1.id), call(grant_id=grant2.id)], any_order=True ) + # Verify audit log entries were created correctly + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant1.id, + change_message="Status changed from 'waiting_list' to 'rejected' and rejection email sent.", + ).exists() + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant2.id, + change_message="Status changed from 'waiting_list_maybe' to 'rejected' and rejection email sent.", + ).exists() + -def test_confirm_pending_status_action(rf): +def test_confirm_pending_status_action(rf, admin_user): grant_1 = GrantFactory( status=Grant.Status.pending, pending_status=Grant.Status.confirmed, @@ -488,6 +611,7 @@ def test_confirm_pending_status_action(rf): ) request = rf.get("/") + request.user = admin_user confirm_pending_status( None, request, Grant.objects.filter(id__in=[grant_1.id, grant_2.id, grant_3.id]) ) @@ -505,11 +629,28 @@ def test_confirm_pending_status_action(rf): assert grant_2.pending_status is None assert grant_3.pending_status is None + # Verify audit log entries were created correctly + assert LogEntry.objects.filter( + object_id=grant_1.id, + change_message="[Bulk Admin Action] Status changed from 'pending' to 'confirmed'.", + ).exists() + assert LogEntry.objects.filter( + object_id=grant_2.id, + change_message="[Bulk Admin Action] Status changed from 'rejected' to 'waiting_list'.", + ).exists() + assert LogEntry.objects.filter( + object_id=grant_3.id, + change_message="[Bulk Admin Action] Status changed from 'waiting_list' to 'waiting_list_maybe'.", + ).exists() + # Left out from the action assert grant_4.status == Grant.Status.waiting_list_maybe + assert not LogEntry.objects.filter( + object_id=grant_4.id, + ).exists() -def test_reset_pending_status_back_to_status_action(rf): +def test_reset_pending_status_back_to_status_action(rf, admin_user): grant_1 = GrantFactory( status=Grant.Status.pending, pending_status=Grant.Status.confirmed, @@ -534,6 +675,7 @@ def test_reset_pending_status_back_to_status_action(rf): ) request = rf.get("/") + request.user = admin_user reset_pending_status_back_to_status( None, request, Grant.objects.filter(id__in=[grant_1.id, grant_2.id, grant_3.id]) ) @@ -552,6 +694,98 @@ def test_reset_pending_status_back_to_status_action(rf): assert grant_3.status == Grant.Status.waiting_list assert grant_3.pending_status is None + # Verify audit log entries were created correctly + assert LogEntry.objects.filter( + object_id=grant_1.id, + change_message="[Bulk Admin Action] pending_status reset from 'confirmed' to None.", + ).exists() + assert LogEntry.objects.filter( + object_id=grant_2.id, + change_message="[Bulk Admin Action] pending_status reset from 'waiting_list' to None.", + ).exists() + assert LogEntry.objects.filter( + object_id=grant_3.id, + change_message="[Bulk Admin Action] pending_status reset from 'waiting_list_maybe' to None.", + ).exists() + # Left out from the action assert grant_4.status == Grant.Status.waiting_list_maybe assert grant_4.pending_status == Grant.Status.confirmed + assert not LogEntry.objects.filter( + object_id=grant_4.id, + ).exists() + + +def test_delete_reimbursement_from_admin_logs_audit_log_entry(rf, admin_user): + grant = GrantFactory() + reimbursement = GrantReimbursementFactory( + grant=grant, + category__conference=grant.conference, + category__ticket=True, + granted_amount=Decimal("100"), + ) + + request = rf.get("/") + request.user = admin_user + + admin = GrantReimbursementAdmin(GrantReimbursement, AdminSite()) + admin.delete_model(request, reimbursement) + + # Verify reimbursement was deleted + assert not GrantReimbursement.objects.filter(id=reimbursement.id).exists() + + # Verify audit log entry was created correctly + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message=f"Reimbursement removed: {reimbursement.category.name}.", + ).exists() + + +def test_save_grant_in_admin_logs_audit_log_entry(rf, admin_user): + grant = GrantFactory() + request = rf.get("/") + request.user = admin_user + + admin = GrantAdmin(Grant, AdminSite()) + admin.save_model(request, grant, None, False) + + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Grant created.", + ).exists() + + +def test_save_grant_in_admin_logs_audit_log_entry_for_status_change(rf, admin_user): + grant = GrantFactory(status=Grant.Status.pending) + request = rf.get("/") + request.user = admin_user + + admin = GrantAdmin(Grant, AdminSite()) + grant.status = Grant.Status.confirmed + admin.save_model(request, grant, None, True) + + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Status changed from 'pending' to 'confirmed'.", + ).exists() + + +def test_save_grant_in_admin_logs_audit_log_entry_for_pending_status_change( + rf, admin_user +): + grant = GrantFactory(pending_status=Grant.Status.pending) + request = rf.get("/") + request.user = admin_user + + admin = GrantAdmin(Grant, AdminSite()) + grant.pending_status = Grant.Status.confirmed + admin.save_model(request, grant, None, True) + + assert LogEntry.objects.filter( + user=admin_user, + object_id=grant.id, + change_message="Pending status changed from 'pending' to 'confirmed'.", + ).exists() diff --git a/backend/reviews/admin.py b/backend/reviews/admin.py index 94013430ef..e388423bdd 100644 --- a/backend/reviews/admin.py +++ b/backend/reviews/admin.py @@ -24,6 +24,11 @@ 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.models import AvailableScoreOption, ReviewSession, UserReview @@ -291,16 +296,23 @@ def _review_grants_recap_view(self, request, review_session): 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 + ) + # if there are grant reimbursements and the decision is not approved, delete them all if grant.reimbursements.exists(): approved_reimbursement_categories = ( @@ -308,12 +320,27 @@ def _review_grants_recap_view(self, request, review_session): ) # If decision is not approved, delete all; else, filter and delete missing reimbursements if decision != Grant.Status.approved: - grant.reimbursements.all().delete() + # 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 - grant.reimbursements.exclude( + # Log deletions before deleting + to_delete = grant.reimbursements.exclude( category_id__in=approved_reimbursement_categories - ).delete() + ) + 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 @@ -323,23 +350,49 @@ def _review_grants_recap_view(self, request, review_session): "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}'.", + ) + + # 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 - GrantReimbursement.objects.update_or_create( - grant=grant, - category_id=reimbursement_category_id, - defaults={ - "granted_amount": reimbursement_categories[ - reimbursement_category_id - ].max_amount - }, + 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." ) diff --git a/backend/reviews/tests/test_admin.py b/backend/reviews/tests/test_admin.py index 8ab73fa578..3b151a5fc2 100644 --- a/backend/reviews/tests/test_admin.py +++ b/backend/reviews/tests/test_admin.py @@ -2,6 +2,7 @@ import pytest from django.contrib.admin import AdminSite +from django.contrib.admin.models import LogEntry from conferences.tests.factories import ConferenceFactory from grants.models import Grant @@ -364,13 +365,41 @@ def test_save_review_grants_updates_grant_and_creates_reimbursements(rf, mocker) reimbursement.category for reimbursement in grant_2.reimbursements.all() } == {ticket_category, travel_category, accommodation_category} + # Verify log entries were created + assert ( + LogEntry.objects.filter(object_id=grant_1.id).count() == 3 + ) # 1 pending_status change, 2 reimbursement additions + assert ( + LogEntry.objects.filter(object_id=grant_2.id).count() == 4 + ) # 1 pending_status change, 3 reimbursement additions + assert LogEntry.objects.filter( + user=user, + object_id__in=[str(grant_1.id), str(grant_2.id)], + change_message="[Review Session] Pending status changed from 'None' to 'approved'.", + ).exists() + assert LogEntry.objects.filter( + user=user, + object_id__in=[str(grant_1.id), str(grant_2.id)], + change_message=f"[Review Session] Reimbursement {ticket_category.name} added.", + ).exists() + assert LogEntry.objects.filter( + user=user, + object_id__in=[str(grant_1.id), str(grant_2.id)], + change_message=f"[Review Session] Reimbursement {travel_category.name} added.", + ).exists() + assert LogEntry.objects.filter( + user=user, + object_id=str(grant_2.id), + 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 ): - mock_messages = mocker.patch("reviews.admin.messages") + mocker.patch("reviews.admin.messages") user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -443,9 +472,17 @@ def test_save_review_grants_update_grants_status_to_rejected_removes_reimburseme assert grant_1.reimbursements.count() == 0 + assert LogEntry.objects.count() == 4 + for reimbursement in grant_1.reimbursements.all(): + assert LogEntry.objects.filter( + user=user, + object_id=str(reimbursement.id), + change_message=f"[Review Session] Reimbursement removed: {reimbursement.category.name}.", + ).exists() + def test_save_review_grants_modify_reimbursements(rf, mocker): - mock_messages = mocker.patch("reviews.admin.messages") + mocker.patch("reviews.admin.messages") user = UserFactory(is_staff=True, is_superuser=True) conference = ConferenceFactory() @@ -506,9 +543,183 @@ def test_save_review_grants_modify_reimbursements(rf, mocker): 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.reimbursements.count() == 1 assert { reimbursement.category for reimbursement in grant_1.reimbursements.all() } == {ticket_category} + + # Verify log entries were created + assert LogEntry.objects.count() == 2 + assert LogEntry.objects.filter( + user=user, + object_id=grant_1.id, + change_message=f"[Review Session] Reimbursement removed: {travel_category.name}.", + ).exists() + assert LogEntry.objects.filter( + user=user, + object_id=grant_1.id, + change_message=f"[Review Session] Reimbursement removed: {accommodation_category.name}.", + ).exists() + + # pending_status change should not be logged because the grant status is not changed + assert not LogEntry.objects.filter( + user=user, + object_id=grant_1.id, + change_message="[Review Session] Pending status changed from 'approved' to 'None'.", + ).exists() + + +def test_save_review_grants_waiting_list_does_not_create_reimbursments(rf, mocker): + mock_messages = mocker.patch("reviews.admin.messages") + + user = UserFactory(is_staff=True, is_superuser=True) + conference = ConferenceFactory() + + # Create reimbursement categories + travel_category = GrantReimbursementCategoryFactory( + conference=conference, + travel=True, + max_amount=Decimal("500"), + ) + ticket_category = GrantReimbursementCategoryFactory( + conference=conference, + ticket=True, + max_amount=Decimal("100"), + ) + accommodation_category = GrantReimbursementCategoryFactory( + conference=conference, + accommodation=True, + max_amount=Decimal("200"), + ) + + # 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) + + grant_1 = GrantFactory(conference=conference, status=Grant.Status.pending) + grant_2 = GrantFactory(conference=conference, status=Grant.Status.pending) + + post_data = { + f"decision-{grant_1.id}": Grant.Status.waiting_list, + f"reimbursementcategory-{grant_1.id}": [ + str(ticket_category.id), + str(travel_category.id), + ], + f"decision-{grant_2.id}": Grant.Status.waiting_list_maybe, + 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.waiting_list + assert grant_2.pending_status == Grant.Status.waiting_list_maybe + + # Verify GrantReimbursement objects were created + assert grant_1.reimbursements.count() == 0 + assert grant_2.reimbursements.count() == 0 + + # Verify log entries were created + assert ( + LogEntry.objects.filter(object_id=grant_1.id).count() == 1 + ) # 1 pending_status change, 2 reimbursement additions + assert ( + LogEntry.objects.filter(object_id=grant_2.id).count() == 1 + ) # 1 pending_status change, 3 reimbursement additions + mock_messages.success.assert_called_once() + + +def test_save_review_grants_two_times_does_not_create_duplicate_log_entries(rf, mocker): + mocker.patch("reviews.admin.messages") + + user = UserFactory(is_staff=True, is_superuser=True) + conference = ConferenceFactory() + + # Create reimbursement categories + travel_category = GrantReimbursementCategoryFactory( + conference=conference, + travel=True, + max_amount=Decimal("500"), + ) + ticket_category = GrantReimbursementCategoryFactory( + conference=conference, + ticket=True, + max_amount=Decimal("100"), + ) + accommodation_category = GrantReimbursementCategoryFactory( + conference=conference, + accommodation=True, + max_amount=Decimal("200"), + ) + + # 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) + + grant_1 = GrantFactory(conference=conference, status=Grant.Status.pending) + post_data = { + f"decision-{grant_1.id}": Grant.Status.approved, + f"reimbursementcategory-{grant_1.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()) + admin._review_grants_recap_view(request, review_session) # First save + admin._review_grants_recap_view(request, review_session) # Second save + + grant_1.refresh_from_db() + + assert grant_1.reimbursements.count() == 3 + assert { + reimbursement.category for reimbursement in grant_1.reimbursements.all() + } == {ticket_category, travel_category, accommodation_category} + + assert LogEntry.objects.count() == 4 + assert LogEntry.objects.filter( + user=user, + object_id=grant_1.id, + change_message="[Review Session] Pending status changed from 'None' to 'approved'.", + ).exists()