Skip to content

Commit

Permalink
Merge pull request #1671 from BLSQ/IA-3463_improve_payment_lot_creation
Browse files Browse the repository at this point in the history
IA-3463 improve payment lot creation
  • Loading branch information
quang-le authored Oct 3, 2024
2 parents 5e113a4 + 66434dc commit 1a4b7fd
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 140 deletions.
1 change: 1 addition & 0 deletions hat/assets/js/apps/Iaso/domains/app/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1231,6 +1231,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 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",
Expand Down
3 changes: 2 additions & 1 deletion hat/assets/js/apps/Iaso/domains/app/translations/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -1231,6 +1231,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 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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<SavePaymentLotQuery>) =>
createEditPaymentLot(data, type),
invalidateQueryKey: ['paymentLots', 'potentialPayments'],
options: { onSuccess },
snackSuccessMessage,
});
};

Expand Down
5 changes: 5 additions & 0 deletions hat/assets/js/apps/Iaso/domains/payments/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 Payment Lots page if it doesn't appear",
},
});

export default MESSAGES;
71 changes: 30 additions & 41 deletions iaso/api/payments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -96,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",
Expand Down Expand Up @@ -209,42 +211,29 @@ 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()
# 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
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
for potential_payment in potential_payments:
if potential_payment.task is None:
potential_payment.task = task
potential_payment.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)
# 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"))
Expand Down Expand Up @@ -455,8 +444,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"))
Expand All @@ -476,8 +465,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(
Expand Down
24 changes: 24 additions & 0 deletions iaso/migrations/0301_potentialpayment_task.py
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
12 changes: 12 additions & 0 deletions iaso/migrations/0303_merge_20241003_1256.py
Original file line number Diff line number Diff line change
@@ -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 = []
3 changes: 3 additions & 0 deletions iaso/models/payments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
136 changes: 136 additions & 0 deletions iaso/tasks/create_payment_lot.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
from time import time
from django.db import transaction
from beanstalk_worker import task_decorator
from hat.audit import models as audit_models
from iaso.api.org_unit_change_requests.serializers import (
OrgUnitChangeRequestAuditLogger,
)
from typing import List
from iaso.api.payments.serializers import PaymentAuditLogger, PaymentLotAuditLogger
from iaso.models import Task
from iaso.models.payments import Payment, PaymentLot, PaymentStatuses, PotentialPayment
from django.utils import timezone
from iaso.models.base import ERRORED
from django.db.models import Q


def create_payment_from_payment_lot(user, payment_lot, *, potential_payment):
"""Used within the context of a bulk operation"""
payment = Payment.objects.create(
status=PaymentStatuses.PENDING,
user=potential_payment.user,
created_by=user,
updated_by=user,
payment_lot=payment_lot,
)
# Save payment modification
audit_logger = PaymentAuditLogger()
change_request_audit_logger = OrgUnitChangeRequestAuditLogger()
change_requests = potential_payment.change_requests.all()
# Add change requests from potential payment to the newly created payment
for change_request in change_requests:
old_change_request_dump = change_request_audit_logger.serialize_instance(change_request)
change_request.payment = payment
change_request.potential_payment = None
change_request.save()
# Save change request modification
change_request_audit_logger.log_modification(
instance=change_request,
old_data_dump=old_change_request_dump,
request_user=user,
source=audit_models.PAYMENT_LOT_API,
)
potential_payment.delete()
# Log the payment change after the fk to change request has been set
audit_logger.log_modification(
instance=payment, old_data_dump=None, request_user=user, source=audit_models.PAYMENT_LOT_API
)


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")
def create_payment_lot(
comment: str,
name: str,
potential_payment_ids: List[int],
task: Task,
):
"""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()
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:
# 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)
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,
potential_payments=potential_payments,
task=the_task,
message="Error while getting potential payments",
)
total = len(potential_payment_ids)
if potential_payments_for_lot.count() != total:
end_task_and_delete_payment_lot(
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_for_lot.iterator()):
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():
end_task_and_delete_payment_lot(
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)
Loading

0 comments on commit 1a4b7fd

Please sign in to comment.