From e3377195626a011fc8239852655d089c4f0bc0de Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Tue, 1 Oct 2024 16:34:37 +0200 Subject: [PATCH 1/8] IA-3463: update backend - move all code related to payment lot creation to new task "create_payment_lot" - make sure payment lot is deleted if there's any error with the conversion of any potential payment into a payment - add "task" field to potential payment - add migration - update tests --- iaso/api/payments/views.py | 63 ++++++----------- iaso/migrations/0301_potentialpayment_task.py | 24 +++++++ iaso/models/payments.py | 3 + ...m_payment_lot.py => create_payment_lot.py} | 70 ++++++++++++------- iaso/tests/api/payments/test_payment_lots.py | 6 +- 5 files changed, 96 insertions(+), 70 deletions(-) create mode 100644 iaso/migrations/0301_potentialpayment_task.py rename iaso/tasks/{create_payments_from_payment_lot.py => create_payment_lot.py} (54%) diff --git a/iaso/api/payments/views.py b/iaso/api/payments/views.py index 1702776d2a..f013268025 100644 --- a/iaso/api/payments/views.py +++ b/iaso/api/payments/views.py @@ -22,7 +22,8 @@ from iaso.api.tasks import TaskSerializer from iaso.models import OrgUnitChangeRequest, Payment, PaymentLot, PotentialPayment from iaso.models.payments import PaymentStatuses -from iaso.tasks.create_payments_from_payment_lot import create_payments_from_payment_lot +from iaso.tasks.create_payment_lot import create_payment_lot +from rest_framework.exceptions import ValidationError from iaso.tasks.payments_bulk_update import mark_payments_as_read from .serializers import ( @@ -209,42 +210,22 @@ def update(self, request, *args, **kwargs): responses={status.HTTP_201_CREATED: PaymentLotSerializer()}, ) def create(self, request): - with transaction.atomic(): - # Extract user, name, comment, and potential_payments IDs from request data - user = self.request.user - name = request.data.get("name") - comment = request.data.get("comment") - potential_payment_ids = request.data.get("potential_payments", []) # Expecting a list of IDs - - audit_logger = PaymentLotAuditLogger() - - # Link the potential Payments to the payment lot to enable front-end to filter them out while the task is creating the Payments - - # Create the PaymentLot instance but don't save it yet - payment_lot = PaymentLot(name=name, comment=comment, created_by=request.user, updated_by=request.user) - - # Save the PaymentLot instance to ensure it has a primary key - payment_lot.save() - potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) - payment_lot.potential_payments.add(*potential_payments) - payment_lot.save() - audit_logger.log_modification(old_data_dump=None, instance=payment_lot, request_user=user) - - # Launch a atask in the worker to update payments, delete potehtial payments, update change requests, update payment_lot status - # and log everything - task = create_payments_from_payment_lot( - payment_lot_id=payment_lot.pk, - potential_payment_ids=potential_payment_ids, - user=user, - ) - payment_lot.task = task - # not logging the task assignment to avoid cluttering the audit logs - payment_lot.save() - - # Return the created PaymentLot instance - # It will be incomplete as the task has to run for all the data to be correct - serializer = self.get_serializer(payment_lot) - return Response(serializer.data, status=status.HTTP_201_CREATED) + # with transaction.atomic(): + # Extract user, name, comment, and potential_payments IDs from request data + user = self.request.user + name = request.data.get("name") + comment = request.data.get("comment") + potential_payment_ids = request.data.get("potential_payments", []) # Expecting a list of IDs + # TODO move this in valdate method + if not potential_payment_ids: + raise ValidationError("At least one potential payment required") + + task = create_payment_lot(user=user, name=name, potential_payment_ids=potential_payment_ids, comment=comment) + # Return the created Task + return Response( + {"task": TaskSerializer(instance=task).data}, + status=status.HTTP_201_CREATED, + ) def retrieve(self, request, *args, **kwargs): csv_format = bool(request.query_params.get("csv")) @@ -455,8 +436,8 @@ def get_queryset(self): queryset = ( PotentialPayment.objects.prefetch_related("change_requests") .filter(change_requests__created_by__iaso_profile__account=self.request.user.iaso_profile.account) - # Filter out potential payments already linked to a payment lot as this means there's already a task running converting them into Payment - .filter(payment_lot__isnull=True) + # Filter out potential payments already linked to a task as this means there's a task running converting them into Payment + .filter(task__isnull=True) .distinct() ) queryset = queryset.annotate(change_requests_count=Count("change_requests")) @@ -476,8 +457,8 @@ def calculate_new_potential_payments(self): created_by_id=user["created_by"], status=OrgUnitChangeRequest.Statuses.APPROVED, payment__isnull=True, - # Filter out potential payments already linked to a payment lot as this means there's already a task running converting them into Payment - potential_payment__payment_lot__isnull=True, + # Filter out potential payments with task as they are being converted to payments + potential_payment__task__isnull=True, ) if change_requests.exists(): potential_payment, created = PotentialPayment.objects.get_or_create( diff --git a/iaso/migrations/0301_potentialpayment_task.py b/iaso/migrations/0301_potentialpayment_task.py new file mode 100644 index 0000000000..b95f93822c --- /dev/null +++ b/iaso/migrations/0301_potentialpayment_task.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.13 on 2024-10-01 11:47 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("iaso", "0300_merge_20240916_1512"), + ] + + operations = [ + migrations.AddField( + model_name="potentialpayment", + name="task", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="potential_payments", + to="iaso.task", + ), + ), + ] diff --git a/iaso/models/payments.py b/iaso/models/payments.py index 21422c7e84..046ab6a5b6 100644 --- a/iaso/models/payments.py +++ b/iaso/models/payments.py @@ -44,6 +44,9 @@ class PotentialPayment(models.Model): payment_lot = models.ForeignKey( "PaymentLot", on_delete=models.SET_NULL, null=True, blank=True, related_name="potential_payments" ) + task = models.ForeignKey( + "Task", on_delete=models.SET_NULL, null=True, blank=True, related_name="potential_payments" + ) class PaymentLot(models.Model): diff --git a/iaso/tasks/create_payments_from_payment_lot.py b/iaso/tasks/create_payment_lot.py similarity index 54% rename from iaso/tasks/create_payments_from_payment_lot.py rename to iaso/tasks/create_payment_lot.py index bd56202e4f..aec445295d 100644 --- a/iaso/tasks/create_payments_from_payment_lot.py +++ b/iaso/tasks/create_payment_lot.py @@ -47,9 +47,10 @@ def create_payment_from_payment_lot(user, payment_lot, *, potential_payment): ) -@task_decorator(task_name="create_payments_from_payment_lot") -def create_payments_from_payment_lot( - payment_lot_id: int, +@task_decorator(task_name="create_payment_lot") +def create_payment_lot( + comment: str, + name: str, potential_payment_ids: List[int], task: Task, ): @@ -58,38 +59,55 @@ def create_payments_from_payment_lot( """ start = time() the_task = task + user = the_task.launcher the_task.report_progress_and_stop_if_killed(progress_message="Searching for Payments to modify") + payment_lot = PaymentLot(name=name, comment=comment, created_by=user, updated_by=user) + payment_lot.task = the_task + payment_lot.save() + + # Addding a try/except so if there's an error while adding the potential payments, we can delete the payment lot and avoid empty payment lots in the DB + # TODO Not sure we need to add the potential payments to the payment lot anymore try: - payment_lot = PaymentLot.objects.get(id=payment_lot_id) - except ObjectDoesNotExist: + potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids, task__isnull=True) + payment_lot.potential_payments.add(*potential_payments, bulk=False) + payment_lot.save() + except: the_task.status = ERRORED the_task.ended_at = timezone.now() - the_task.result = {"result": ERRORED, "message": "Payment Lot not found"} + the_task.result = {"result": ERRORED, "message": "Error while getting potential payments"} the_task.save() + payment_lot.delete() - potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) total = len(potential_payment_ids) if potential_payments.count() != total: the_task.status = ERRORED the_task.ended_at = timezone.now() the_task.result = {"result": ERRORED, "message": "One or several Potential payments not found"} the_task.save() - raise ObjectDoesNotExist - - # audit stuff - user = the_task.launcher - audit_logger = PaymentLotAuditLogger() - old_payment_lot_dump = audit_logger.serialize_instance(payment_lot) - - with transaction.atomic(): - for index, potential_payment in enumerate(potential_payments.iterator()): - res_string = "%.2f sec, processed %i payments" % (time() - start, index) - the_task.report_progress_and_stop_if_killed( - progress_message=res_string, end_value=total, progress_value=index - ) - create_payment_from_payment_lot(user=user, payment_lot=payment_lot, potential_payment=potential_payment) - # Compute status, although it should be NEW since we just created all the Payments - payment_lot.compute_status() - payment_lot.save() - audit_logger.log_modification(instance=payment_lot, old_data_dump=old_payment_lot_dump, request_user=user) - the_task.report_success(message="%d modified" % total) + payment_lot.delete() + else: + with transaction.atomic(): + for index, potential_payment in enumerate(potential_payments.iterator()): + potential_payment.task = the_task + potential_payment.payment_lot = payment_lot + potential_payment.save() + res_string = "%.2f sec, processed %i payments" % (time() - start, index) + the_task.report_progress_and_stop_if_killed( + progress_message=res_string, end_value=total, progress_value=index + ) + create_payment_from_payment_lot(user=user, payment_lot=payment_lot, potential_payment=potential_payment) + # If potential payments haven't been deleted, it means there was a problem with the above transaction and it was reverted. + # In this case we delete the payment lot and ERROR the task + if payment_lot.potential_payments.count(): + the_task.status = ERRORED + the_task.ended_at = timezone.now() + the_task.result = {"result": ERRORED, "message": "Error while creating one or several payments"} + the_task.save() + payment_lot.delete() + else: + audit_logger = PaymentLotAuditLogger() + # Compute status, although it should be NEW since we just created all the Payments + payment_lot.compute_status() + payment_lot.save() + audit_logger.log_modification(instance=payment_lot, old_data_dump=None, request_user=user) + the_task.report_success(message="%d modified" % total) diff --git a/iaso/tests/api/payments/test_payment_lots.py b/iaso/tests/api/payments/test_payment_lots.py index 999d167220..debcd874cf 100644 --- a/iaso/tests/api/payments/test_payment_lots.py +++ b/iaso/tests/api/payments/test_payment_lots.py @@ -83,7 +83,7 @@ def test_create_payment_lot(self): self.assertJSONResponse(response, 201) data = response.json() - task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="create_payments_from_payment_lot") + task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="create_payment_lot") self.assertEqual(task.launcher, self.user) # Run the task @@ -110,8 +110,8 @@ def test_create_payment_lot(self): self.assertIsNone(self.third_change_request.potential_payment) self.assertFalse(m.PotentialPayment.objects.filter(id=self.potential_payment.pk).exists()) - # Changes have been logged: 1 for Payment lot at creation, 1 for payment, 1 for change request, 1 for Payment lot after all payments have been created - self.assertEqual(4, am.Modification.objects.count()) + # Changes have been logged: 1 for payment, 1 for change request, 1 for Payment lot after all payments have been created + self.assertEqual(3, am.Modification.objects.count()) def test_update_payment_lot_mark_payments_as_sent(self): self.client.force_authenticate(self.user) From 6490b93b76f1deb4aeb52280403edc888b1a8f47 Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Tue, 1 Oct 2024 16:45:35 +0200 Subject: [PATCH 2/8] IA-3463: update front-end - Add custom message when creating payment lot --- hat/assets/js/apps/Iaso/domains/app/translations/en.json | 1 + hat/assets/js/apps/Iaso/domains/app/translations/fr.json | 7 ++++--- .../domains/payments/hooks/requests/useSavePaymentLot.ts | 4 ++++ hat/assets/js/apps/Iaso/domains/payments/messages.ts | 5 +++++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/hat/assets/js/apps/Iaso/domains/app/translations/en.json b/hat/assets/js/apps/Iaso/domains/app/translations/en.json index 7642ff5d74..bf290e4824 100644 --- a/hat/assets/js/apps/Iaso/domains/app/translations/en.json +++ b/hat/assets/js/apps/Iaso/domains/app/translations/en.json @@ -1228,6 +1228,7 @@ "iaso.snackBar.newEmptyVersionSavedSuccess": "New empty version saved successfully", "iaso.snackBar.patchTaskError": "An error occurred while updating the task.", "iaso.snackBar.patchTaskSuccess": "The task has been updated.", + "iaso.snackBar.paymentLotTaskLaunched": "The creation of the payment lot may take a few moments. Try refreshing the page in a few seconds", "iaso.snackBar.paymentsBulkUpdateLaunched": "The modifications to the payments will take a few moments to take effect", "iaso.snackBar.permissionError": "You don't have permission to perform this action", "iaso.snackBar.reload": "Reload", diff --git a/hat/assets/js/apps/Iaso/domains/app/translations/fr.json b/hat/assets/js/apps/Iaso/domains/app/translations/fr.json index 597a89aa15..629645e5c8 100644 --- a/hat/assets/js/apps/Iaso/domains/app/translations/fr.json +++ b/hat/assets/js/apps/Iaso/domains/app/translations/fr.json @@ -320,8 +320,8 @@ "iaso.groups.sourceVersion": "Version de la source", "iaso.groups.update": "Mettre le groupe à jour", "iaso.groupsets.dialog.delete": "Etes-vous certain de vouloir effacer cet ensemble de groupe?", - "iaso.groupsets.dialog.deleteText":"Cette opération ne peut-être annulée", - "iaso.groupsets.groupBelonging": "Appartenance aux groupes", + "iaso.groupsets.dialog.deleteText": "Cette opération ne peut-être annulée", + "iaso.groupsets.groupBelonging": "Appartenance aux groupes", "iaso.groupsets.validation.field_required": "Ce champ est obligatoire", "iaso.hospital": "Hôpital", "iaso.instance.coordinate": "Coordonnées", @@ -1228,6 +1228,7 @@ "iaso.snackBar.newEmptyVersionSavedSuccess": "Nouvelle version vide sauvegardée avec succès", "iaso.snackBar.patchTaskError": "Une erreur est survenue lors de la mise à jour de la tâche.", "iaso.snackBar.patchTaskSuccess": "La tâche a été mise à jour.", + "iaso.snackBar.paymentLotTaskLaunched": "La création du lot de paiments peut prendre un instant. Essayez de rafraîchir la page dans quelques secondes", "iaso.snackBar.paymentsBulkUpdateLaunched": "Les modifications peuvent prendre un moment avant d'être visibles", "iaso.snackBar.permissionError": "Vous n'avez pas la permission d'effectuer cette action", "iaso.snackBar.reload": "Recharger", @@ -1509,4 +1510,4 @@ "trypelim.permissions.zones": "Zones", "trypelim.permissions.zones_edit": "Edit zones", "trypelim.permissions.zones_shapes_edit": "Editer les contours géographiques des zones de santé" -} +} \ No newline at end of file diff --git a/hat/assets/js/apps/Iaso/domains/payments/hooks/requests/useSavePaymentLot.ts b/hat/assets/js/apps/Iaso/domains/payments/hooks/requests/useSavePaymentLot.ts index 305e0c5c4d..6573a7c1b6 100644 --- a/hat/assets/js/apps/Iaso/domains/payments/hooks/requests/useSavePaymentLot.ts +++ b/hat/assets/js/apps/Iaso/domains/payments/hooks/requests/useSavePaymentLot.ts @@ -2,6 +2,7 @@ import { UseMutationResult } from 'react-query'; import { patchRequest, postRequest } from '../../../../libs/Api'; import { useSnackMutation } from '../../../../libs/apiHooks'; +import MESSAGES from '../../messages'; export type CreatePaymentLotQuery = { id?: number; @@ -57,11 +58,14 @@ export const useSavePaymentLot = ( type: 'create' | 'edit', onSuccess?: () => void, ): UseMutationResult => { + const snackSuccessMessage = + type === 'create' ? MESSAGES.paymentLotTaskLaunched : undefined; return useSnackMutation({ mutationFn: (data: Partial) => createEditPaymentLot(data, type), invalidateQueryKey: ['paymentLots', 'potentialPayments'], options: { onSuccess }, + snackSuccessMessage, }); }; diff --git a/hat/assets/js/apps/Iaso/domains/payments/messages.ts b/hat/assets/js/apps/Iaso/domains/payments/messages.ts index 8ad3d17512..90054c2a80 100644 --- a/hat/assets/js/apps/Iaso/domains/payments/messages.ts +++ b/hat/assets/js/apps/Iaso/domains/payments/messages.ts @@ -202,6 +202,11 @@ const MESSAGES = defineMessages({ id: 'iaso.label.viewChangeRequestsForPayment', defaultMessage: 'View change requests for this payment', }, + paymentLotTaskLaunched: { + id: 'iaso.snackBar.paymentLotTaskLaunched', + defaultMessage: + 'The creation of the payment lot may take a few moments. Try refreshing the page in a few seconds', + }, }); export default MESSAGES; From 3f51da50348d24f8f1c89dbfbc133c91e5f54ffe Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Tue, 1 Oct 2024 17:00:15 +0200 Subject: [PATCH 3/8] IA-3463: DRY up code --- iaso/tasks/create_payment_lot.py | 35 +++++++++++++++++--------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/iaso/tasks/create_payment_lot.py b/iaso/tasks/create_payment_lot.py index aec445295d..e53d2728b0 100644 --- a/iaso/tasks/create_payment_lot.py +++ b/iaso/tasks/create_payment_lot.py @@ -47,6 +47,14 @@ def create_payment_from_payment_lot(user, payment_lot, *, potential_payment): ) +def end_task_and_delete_payment_lot(payment_lot, task, message): + task.status = ERRORED + task.ended_at = timezone.now() + task.result = {"result": ERRORED, "message": message} + task.save() + payment_lot.delete() + + @task_decorator(task_name="create_payment_lot") def create_payment_lot( comment: str, @@ -54,7 +62,7 @@ def create_payment_lot( potential_payment_ids: List[int], task: Task, ): - """Background Task to bulk update payments related to specific PaymentLot + """Background Task to create Payment Lot and convert Potential Payments into Payments Used exclusively within the context of the PaymentLot API. Users won't launch it directly from another endpoint. """ start = time() @@ -72,19 +80,16 @@ def create_payment_lot( payment_lot.potential_payments.add(*potential_payments, bulk=False) payment_lot.save() except: - the_task.status = ERRORED - the_task.ended_at = timezone.now() - the_task.result = {"result": ERRORED, "message": "Error while getting potential payments"} - the_task.save() - payment_lot.delete() + end_task_and_delete_payment_lot( + payment_lot=payment_lot, task=the_task, message="Error while getting potential payments" + ) total = len(potential_payment_ids) if potential_payments.count() != total: - the_task.status = ERRORED - the_task.ended_at = timezone.now() - the_task.result = {"result": ERRORED, "message": "One or several Potential payments not found"} - the_task.save() - payment_lot.delete() + end_task_and_delete_payment_lot( + payment_lot=payment_lot, task=the_task, message="One or several Potential payments not found" + ) + else: with transaction.atomic(): for index, potential_payment in enumerate(potential_payments.iterator()): @@ -99,11 +104,9 @@ def create_payment_lot( # If potential payments haven't been deleted, it means there was a problem with the above transaction and it was reverted. # In this case we delete the payment lot and ERROR the task if payment_lot.potential_payments.count(): - the_task.status = ERRORED - the_task.ended_at = timezone.now() - the_task.result = {"result": ERRORED, "message": "Error while creating one or several payments"} - the_task.save() - payment_lot.delete() + end_task_and_delete_payment_lot( + payment_lot=payment_lot, task=the_task, message="Error while creating one or several payments" + ) else: audit_logger = PaymentLotAuditLogger() # Compute status, although it should be NEW since we just created all the Payments From 22a5756221c1b04ac601cba6229e65293a0871bf Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Wed, 2 Oct 2024 10:43:03 +0200 Subject: [PATCH 4/8] IA-3463: fix 500 on payment lot creation - Assign task to potential payments before returning 201 - Update filter in task function --- iaso/api/payments/views.py | 4 ++++ iaso/tasks/create_payment_lot.py | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/iaso/api/payments/views.py b/iaso/api/payments/views.py index f013268025..dd5038f7dd 100644 --- a/iaso/api/payments/views.py +++ b/iaso/api/payments/views.py @@ -221,6 +221,10 @@ def create(self, request): raise ValidationError("At least one potential payment required") task = create_payment_lot(user=user, name=name, potential_payment_ids=potential_payment_ids, comment=comment) + # Assign task to potential payments to avoid racing condition when calling potential payments API + potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) + task.potential_payments.add(*potential_payments, bulk=False) + # Return the created Task return Response( {"task": TaskSerializer(instance=task).data}, diff --git a/iaso/tasks/create_payment_lot.py b/iaso/tasks/create_payment_lot.py index e53d2728b0..07385fc394 100644 --- a/iaso/tasks/create_payment_lot.py +++ b/iaso/tasks/create_payment_lot.py @@ -10,8 +10,8 @@ from iaso.models import Task from iaso.models.payments import Payment, PaymentLot, PaymentStatuses, PotentialPayment from django.utils import timezone -from django.core.exceptions import ObjectDoesNotExist from iaso.models.base import ERRORED +from django.db.models import Q def create_payment_from_payment_lot(user, payment_lot, *, potential_payment): @@ -76,7 +76,11 @@ def create_payment_lot( # Addding a try/except so if there's an error while adding the potential payments, we can delete the payment lot and avoid empty payment lots in the DB # TODO Not sure we need to add the potential payments to the payment lot anymore try: - potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids, task__isnull=True) + # We want to filter out potential payments assigned to another task. + # Since the task is assigned in the view after it's created, we filter out potential payments with no task or with the current task assigned (for safety) + potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids).filter( + Q(task__isnull=True) | Q(task__id=the_task.id) + ) payment_lot.potential_payments.add(*potential_payments, bulk=False) payment_lot.save() except: @@ -93,7 +97,6 @@ def create_payment_lot( else: with transaction.atomic(): for index, potential_payment in enumerate(potential_payments.iterator()): - potential_payment.task = the_task potential_payment.payment_lot = payment_lot potential_payment.save() res_string = "%.2f sec, processed %i payments" % (time() - start, index) From 3a004f0dd921134badc2986eb40fbdeaea37961d Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Wed, 2 Oct 2024 14:04:53 +0200 Subject: [PATCH 5/8] IA_3463: add tests + fix bugs --- iaso/api/payments/views.py | 14 +++--- iaso/tasks/create_payment_lot.py | 39 ++++++++++++----- iaso/tests/api/payments/test_payment_lots.py | 45 ++++++++++++++++++++ 3 files changed, 82 insertions(+), 16 deletions(-) diff --git a/iaso/api/payments/views.py b/iaso/api/payments/views.py index dd5038f7dd..22dbbef24b 100644 --- a/iaso/api/payments/views.py +++ b/iaso/api/payments/views.py @@ -97,7 +97,8 @@ class PaymentLotsViewSet(ModelViewSet): http_method_names = ["get", "post", "patch", "head", "options", "trace"] def get_queryset(self): - queryset = PaymentLot.objects.all() + # Filter out PaymentLot with task because they're still being created by the worker + queryset = PaymentLot.objects.filter(task__isnull=True) change_requests_prefetch = Prefetch( "payments__change_requests", @@ -215,15 +216,18 @@ def create(self, request): user = self.request.user name = request.data.get("name") comment = request.data.get("comment") - potential_payment_ids = request.data.get("potential_payments", []) # Expecting a list of IDs + potential_payment_ids = request.data.getlist("potential_payments", []) # Expecting a list of IDs + potential_payment_ids = [int(pp_id) for pp_id in potential_payment_ids] # TODO move this in valdate method if not potential_payment_ids: raise ValidationError("At least one potential payment required") - + potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) task = create_payment_lot(user=user, name=name, potential_payment_ids=potential_payment_ids, comment=comment) # Assign task to potential payments to avoid racing condition when calling potential payments API - potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) - task.potential_payments.add(*potential_payments, bulk=False) + for potential_payment in potential_payments: + if potential_payment.task is None: + potential_payment.task = task + potential_payment.save() # Return the created Task return Response( diff --git a/iaso/tasks/create_payment_lot.py b/iaso/tasks/create_payment_lot.py index 07385fc394..e3bf3a9415 100644 --- a/iaso/tasks/create_payment_lot.py +++ b/iaso/tasks/create_payment_lot.py @@ -47,12 +47,17 @@ def create_payment_from_payment_lot(user, payment_lot, *, potential_payment): ) -def end_task_and_delete_payment_lot(payment_lot, task, message): +def end_task_and_delete_payment_lot(payment_lot, potential_payments, task, message): task.status = ERRORED task.ended_at = timezone.now() task.result = {"result": ERRORED, "message": message} task.save() + potential_payments.update(task=None) + # update doesn't call save(), so we need to loop through the queryset and save each instance + for potential_payment in potential_payments: + potential_payment.save() payment_lot.delete() + raise Exception("Error while creating Payment Lot") @task_decorator(task_name="create_payment_lot") @@ -78,25 +83,32 @@ def create_payment_lot( try: # We want to filter out potential payments assigned to another task. # Since the task is assigned in the view after it's created, we filter out potential payments with no task or with the current task assigned (for safety) - potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids).filter( - Q(task__isnull=True) | Q(task__id=the_task.id) - ) - payment_lot.potential_payments.add(*potential_payments, bulk=False) + potential_payments = PotentialPayment.objects.filter(id__in=potential_payment_ids) + for p in potential_payments: + print(p.task.id) + potential_payments_for_lot = potential_payments.filter(task=the_task) + # potential_payments_for_lot = potential_payments.filter((Q(task__isnull=True) | Q(task=the_task))) + payment_lot.potential_payments.add(*potential_payments_for_lot, bulk=False) payment_lot.save() except: end_task_and_delete_payment_lot( - payment_lot=payment_lot, task=the_task, message="Error while getting potential payments" + payment_lot=payment_lot, + potential_payments=potential_payments, + task=the_task, + message="Error while getting potential payments", ) - total = len(potential_payment_ids) - if potential_payments.count() != total: + if potential_payments_for_lot.count() != total: end_task_and_delete_payment_lot( - payment_lot=payment_lot, task=the_task, message="One or several Potential payments not found" + payment_lot=payment_lot, + potential_payments=potential_payments, + task=the_task, + message="One or several Potential payments not found", ) else: with transaction.atomic(): - for index, potential_payment in enumerate(potential_payments.iterator()): + for index, potential_payment in enumerate(potential_payments_for_lot.iterator()): potential_payment.payment_lot = payment_lot potential_payment.save() res_string = "%.2f sec, processed %i payments" % (time() - start, index) @@ -108,12 +120,17 @@ def create_payment_lot( # In this case we delete the payment lot and ERROR the task if payment_lot.potential_payments.count(): end_task_and_delete_payment_lot( - payment_lot=payment_lot, task=the_task, message="Error while creating one or several payments" + payment_lot=payment_lot, + potential_payments=potential_payments, + task=the_task, + message="Error while creating one or several payments", ) else: audit_logger = PaymentLotAuditLogger() # Compute status, although it should be NEW since we just created all the Payments payment_lot.compute_status() + # Set task to null, so we can filter out active tasks in the payment lot API + payment_lot.task = None payment_lot.save() audit_logger.log_modification(instance=payment_lot, old_data_dump=None, request_user=user) the_task.report_success(message="%d modified" % total) diff --git a/iaso/tests/api/payments/test_payment_lots.py b/iaso/tests/api/payments/test_payment_lots.py index debcd874cf..bc7460fe0e 100644 --- a/iaso/tests/api/payments/test_payment_lots.py +++ b/iaso/tests/api/payments/test_payment_lots.py @@ -58,6 +58,11 @@ def setUpTestData(cls): payment=cls.second_payment, ) cls.potential_payment = m.PotentialPayment.objects.create(user=cls.payment_beneficiary) + running_task = m.Task.objects.create(launcher=cls.user, account=cls.user.iaso_profile.account, status="SUCCESS") + cls.potential_payment_with_task = m.PotentialPayment.objects.create( + user=cls.payment_beneficiary, task=running_task + ) + cls.third_change_request = m.OrgUnitChangeRequest.objects.create( org_unit=cls.org_unit, new_name="Wetlands", @@ -254,3 +259,43 @@ def test_retrieve_payment_lot_to_xlsx(self): }, }, ) + + def test_payment_lot_not_created_if_potential_payment_has_task(self): + self.client.force_authenticate(self.user) + response = self.client.post( + "/api/payments/lots/", + { + "name": "New Payment Lot", + "potential_payments": [self.potential_payment.pk, self.potential_payment_with_task.pk], + }, + ) + + self.assertJSONResponse(response, 201) + data = response.json() + task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="create_payment_lot") + self.assertEqual(task.launcher, self.user) + + # Run the task + self.runAndValidateTask(task, "ERRORED") + # No new payment lot created, we find only the one from setup + self.assertEqual(m.PaymentLot.objects.count(), 1) + + def test_payment_lot_not_created_if_potential_payment_not_found(self): + self.client.force_authenticate(self.user) + response = self.client.post( + "/api/payments/lots/", + { + "name": "New Payment Lot", + "potential_payments": [self.potential_payment.pk, self.potential_payment_with_task.pk + 100], + }, + ) + + self.assertJSONResponse(response, 201) + data = response.json() + task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="create_payment_lot") + self.assertEqual(task.launcher, self.user) + + # Run the task + self.runAndValidateTask(task, "ERRORED") + # No new payment lot created, we find only the one from setup + self.assertEqual(m.PaymentLot.objects.count(), 1) From 4f837dea07d19113d3f29f1a7c84d47765e33ccf Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Wed, 2 Oct 2024 14:25:23 +0200 Subject: [PATCH 6/8] IA-3463: fix error 500 --- iaso/api/payments/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iaso/api/payments/views.py b/iaso/api/payments/views.py index 22dbbef24b..07b5f5eed7 100644 --- a/iaso/api/payments/views.py +++ b/iaso/api/payments/views.py @@ -216,7 +216,7 @@ def create(self, request): user = self.request.user name = request.data.get("name") comment = request.data.get("comment") - potential_payment_ids = request.data.getlist("potential_payments", []) # Expecting a list of IDs + potential_payment_ids = request.data.get("potential_payments", []) # Expecting a list of IDs potential_payment_ids = [int(pp_id) for pp_id in potential_payment_ids] # TODO move this in valdate method if not potential_payment_ids: From 758a722a831eb1b07c1814a9ea1a0d3ec622cb67 Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Thu, 3 Oct 2024 13:23:51 +0200 Subject: [PATCH 7/8] IA-3463: improve payment lot snack bar msg --- hat/assets/js/apps/Iaso/domains/app/translations/en.json | 2 +- hat/assets/js/apps/Iaso/domains/app/translations/fr.json | 4 ++-- hat/assets/js/apps/Iaso/domains/payments/messages.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hat/assets/js/apps/Iaso/domains/app/translations/en.json b/hat/assets/js/apps/Iaso/domains/app/translations/en.json index bf290e4824..5ab1744cde 100644 --- a/hat/assets/js/apps/Iaso/domains/app/translations/en.json +++ b/hat/assets/js/apps/Iaso/domains/app/translations/en.json @@ -1228,7 +1228,7 @@ "iaso.snackBar.newEmptyVersionSavedSuccess": "New empty version saved successfully", "iaso.snackBar.patchTaskError": "An error occurred while updating the task.", "iaso.snackBar.patchTaskSuccess": "The task has been updated.", - "iaso.snackBar.paymentLotTaskLaunched": "The creation of the payment lot may take a few moments. Try refreshing the page in a few seconds", + "iaso.snackBar.paymentLotTaskLaunched": "The creation of the payment lot may take a few moments. Try refreshing the Payment Lots page if it doesn't appear", "iaso.snackBar.paymentsBulkUpdateLaunched": "The modifications to the payments will take a few moments to take effect", "iaso.snackBar.permissionError": "You don't have permission to perform this action", "iaso.snackBar.reload": "Reload", diff --git a/hat/assets/js/apps/Iaso/domains/app/translations/fr.json b/hat/assets/js/apps/Iaso/domains/app/translations/fr.json index 629645e5c8..e69959a012 100644 --- a/hat/assets/js/apps/Iaso/domains/app/translations/fr.json +++ b/hat/assets/js/apps/Iaso/domains/app/translations/fr.json @@ -320,7 +320,7 @@ "iaso.groups.sourceVersion": "Version de la source", "iaso.groups.update": "Mettre le groupe à jour", "iaso.groupsets.dialog.delete": "Etes-vous certain de vouloir effacer cet ensemble de groupe?", - "iaso.groupsets.dialog.deleteText": "Cette opération ne peut-être annulée", + "iaso.groupsets.dialog.deleteText": "Cette opération ne peut être annulée", "iaso.groupsets.groupBelonging": "Appartenance aux groupes", "iaso.groupsets.validation.field_required": "Ce champ est obligatoire", "iaso.hospital": "Hôpital", @@ -1228,7 +1228,7 @@ "iaso.snackBar.newEmptyVersionSavedSuccess": "Nouvelle version vide sauvegardée avec succès", "iaso.snackBar.patchTaskError": "Une erreur est survenue lors de la mise à jour de la tâche.", "iaso.snackBar.patchTaskSuccess": "La tâche a été mise à jour.", - "iaso.snackBar.paymentLotTaskLaunched": "La création du lot de paiments peut prendre un instant. Essayez de rafraîchir la page dans quelques secondes", + "iaso.snackBar.paymentLotTaskLaunched": "La création du lot de paiements peut prendre un instant. Essayez de rafraîchir la page Lots de Paiements s'il n'apparaît pas", "iaso.snackBar.paymentsBulkUpdateLaunched": "Les modifications peuvent prendre un moment avant d'être visibles", "iaso.snackBar.permissionError": "Vous n'avez pas la permission d'effectuer cette action", "iaso.snackBar.reload": "Recharger", diff --git a/hat/assets/js/apps/Iaso/domains/payments/messages.ts b/hat/assets/js/apps/Iaso/domains/payments/messages.ts index 90054c2a80..95dd5c84af 100644 --- a/hat/assets/js/apps/Iaso/domains/payments/messages.ts +++ b/hat/assets/js/apps/Iaso/domains/payments/messages.ts @@ -205,7 +205,7 @@ const MESSAGES = defineMessages({ paymentLotTaskLaunched: { id: 'iaso.snackBar.paymentLotTaskLaunched', defaultMessage: - 'The creation of the payment lot may take a few moments. Try refreshing the page in a few seconds', + "The creation of the payment lot may take a few moments. Try refreshing the Payment Lots page if it doesn't appear", }, }); From 66434dc850ba9f42b96ed9569693776355eda7dc Mon Sep 17 00:00:00 2001 From: Quang Son Le Date: Thu, 3 Oct 2024 14:56:52 +0200 Subject: [PATCH 8/8] IA-3463:merge migrations --- iaso/migrations/0303_merge_20241003_1256.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 iaso/migrations/0303_merge_20241003_1256.py diff --git a/iaso/migrations/0303_merge_20241003_1256.py b/iaso/migrations/0303_merge_20241003_1256.py new file mode 100644 index 0000000000..1a0d8f82aa --- /dev/null +++ b/iaso/migrations/0303_merge_20241003_1256.py @@ -0,0 +1,12 @@ +# Generated by Django 4.2.13 on 2024-10-03 12:56 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("iaso", "0301_potentialpayment_task"), + ("iaso", "0302_merge_20241003_0911"), + ] + + operations = []