From 6cc8d2f8eed7ca3937200a1b9b5114aadc083c1f Mon Sep 17 00:00:00 2001 From: Renaud Michotte Date: Mon, 7 Feb 2022 15:46:25 +0100 Subject: [PATCH] PartonTransactionEvent: Fix decimal amount problem. * Creates an extension to check if event amount contains a maximum of 2 decimals. * Reorganizing some codes from `PatronTransaction` resource to be more readable/usable. * Closes rero/rero-ils#2602. Co-Authored-by: Renaud Michotte --- rero_ils/modules/loans/listener.py | 7 +- rero_ils/modules/notifications/api.py | 4 +- .../modules/patron_transaction_events/api.py | 32 +- .../patron_transaction_events/extentions.py | 67 ++++ .../patron_transaction_event-v0.0.1.json | 7 +- .../patron_transaction_events/models.py | 9 + rero_ils/modules/patron_transactions/api.py | 302 +++--------------- rero_ils/modules/patron_transactions/utils.py | 209 ++++++++++++ rero_ils/modules/patron_types/api.py | 9 +- rero_ils/modules/patrons/api.py | 11 +- rero_ils/modules/patrons/views.py | 6 +- rero_ils/modules/selfcheck/api.py | 18 +- .../circulation/test_actions_views_checkin.py | 5 +- .../test_patron_payments_rest.py | 60 ++-- .../test_patron_transactions_rest.py | 12 +- tests/ui/loans/test_loans_api.py | 5 +- .../test_patron_transaction_events_api.py | 2 +- 17 files changed, 426 insertions(+), 339 deletions(-) create mode 100644 rero_ils/modules/patron_transaction_events/extentions.py create mode 100644 rero_ils/modules/patron_transactions/utils.py rename tests/api/{ => patron_transaction_events}/test_patron_payments_rest.py (54%) diff --git a/rero_ils/modules/loans/listener.py b/rero_ils/modules/loans/listener.py index 52be9f14af..0aceb9af4f 100644 --- a/rero_ils/modules/loans/listener.py +++ b/rero_ils/modules/loans/listener.py @@ -22,7 +22,8 @@ from ..items.api import Item from ..loans.logs.api import LoanOperationLog -from ..patron_transactions.api import PatronTransaction +from ..patron_transactions.utils import \ + create_patron_transaction_from_overdue_loan def enrich_loan_data(sender, json=None, record=None, index=None, @@ -58,8 +59,6 @@ def listener_loan_state_changed( # Create fees for checkin or extend operations if trigger in ['checkin', 'extend']: - PatronTransaction.create_patron_transaction_from_overdue_loan( - initial_loan - ) + create_patron_transaction_from_overdue_loan(initial_loan) LoanOperationLog.create(loan) diff --git a/rero_ils/modules/notifications/api.py b/rero_ils/modules/notifications/api.py index 23c3bd2d9c..1bd29b3af1 100644 --- a/rero_ils/modules/notifications/api.py +++ b/rero_ils/modules/notifications/api.py @@ -32,6 +32,8 @@ from ..minters import id_minter from ..patron_transactions.api import PatronTransaction, \ PatronTransactionsSearch +from ..patron_transactions.utils import \ + create_patron_transaction_from_notification from ..providers import Provider # notification provider @@ -102,7 +104,7 @@ def create(cls, data, id_=None, delete_pid=False, dbcommit=False, data.setdefault('status', NotificationStatus.CREATED) record = super().create(data, id_, delete_pid, dbcommit, reindex, **kwargs) - PatronTransaction.create_patron_transaction_from_notification( + create_patron_transaction_from_notification( notification=record, dbcommit=dbcommit, reindex=reindex, delete_pid=delete_pid ) diff --git a/rero_ils/modules/patron_transaction_events/api.py b/rero_ils/modules/patron_transaction_events/api.py index df45f3f328..6fc3486d94 100644 --- a/rero_ils/modules/patron_transaction_events/api.py +++ b/rero_ils/modules/patron_transaction_events/api.py @@ -23,8 +23,9 @@ from flask_babelex import gettext as _ +from .extentions import DecimalAmountExtension from .models import PatronTransactionEventIdentifier, \ - PatronTransactionEventMetadata + PatronTransactionEventMetadata, PatronTransactionEventType from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch from ..fetchers import id_fetcher from ..minters import id_minter @@ -76,15 +77,16 @@ class PatronTransactionEvent(IlsRecord): } } + _extensions = [ + DecimalAmountExtension('amount') + ] + @classmethod def create(cls, data, id_=None, delete_pid=False, dbcommit=False, reindex=False, update_parent=True, **kwargs): """Create patron transaction event record.""" if 'creation_date' not in data: data['creation_date'] = datetime.now(timezone.utc).isoformat() - if 'amount' in data: - # ensure multiple of 0.01 - data['amount'] = round(data['amount'], 2) record = super().create( data, id_, delete_pid, dbcommit, reindex, **kwargs) if update_parent: @@ -149,18 +151,20 @@ def update_parent_patron_transaction(self): # digits, we can multiply amounts by 100, cast result as integer, # do operation with these values, and (at the end) divide the result # by 100. - patron_transaction = self.patron_transaction() - total_amount = int(patron_transaction.get('total_amount') * 100) - if self.event_type == 'fee': - total_amount += int(self.amount * 100) - elif self.event_type in ('payment', 'cancel'): - total_amount -= int(self.amount * 100) - patron_transaction['total_amount'] = total_amount / 100 + pttr = self.patron_transaction + total_amount = int(pttr.get('total_amount') * 100) + amount = int(self.amount * 100) + if self.event_type == PatronTransactionEventType.FEE: + total_amount += amount + elif self.event_type in [PatronTransactionEventType.PAYMENT, + PatronTransactionEventType.CANCEL]: + total_amount -= amount + pttr['total_amount'] = total_amount / 100 if total_amount == 0: - patron_transaction['status'] = 'closed' - patron_transaction.update( - patron_transaction, dbcommit=True, reindex=True) + pttr['status'] = 'closed' + pttr.update(pttr, dbcommit=True, reindex=True) + @property def patron_transaction(self): """Return the parent patron transaction of the event.""" from ..patron_transactions.api import PatronTransaction diff --git a/rero_ils/modules/patron_transaction_events/extentions.py b/rero_ils/modules/patron_transaction_events/extentions.py new file mode 100644 index 0000000000..9bdc805183 --- /dev/null +++ b/rero_ils/modules/patron_transaction_events/extentions.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2022 RERO +# Copyright (C) 2022 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""PatronTransactionEvent record extensions.""" + +from invenio_records.extensions import RecordExtension +from jsonschema import ValidationError + + +class DecimalAmountExtension(RecordExtension): + """Check and transform a decimal amount into integer representation.""" + + def __init__(self, field_name, decimals=2): + """Extension initialization. + + :param field_name: the field name where found the amount to analyze. + :param decimals: the number of decimal accepted. + """ + self.amount_field = field_name + self.multiplier = pow(10, decimals) + + def _check_amount(self, record): + """Check if the record has the correct decimal number. + + :param record: the record to validate. + :raise ValidationError if the amount doesn't respect the decimal amount + """ + # Check `self.amount_field` exists into the record. If not, we can + # return without record modification. + if self.amount_field not in record: + return + + # Check that the amount field has the correct decimals. If not, a + # `ValidationError` error will be raised. + # NOTE: + # an amount of "123,450" is true despite if we configure only 2 + # decimals. In same way "123,4" is also a valid value. + # DEV NOTE : + # rounding the number, keeping one decimal will prevent the floating + # numbers problem + # >>> 2.2 * 100 --> 220.00000000000003 + # >>> round(2.2 * 100, 1) --> 220.0 + # >>> round(2.234 * 100, 1) --> 223.4 + # >>> round(2.2345 * 100, 1) --> 223.5 + quotient = round(record[self.amount_field] * self.multiplier, 1) + if (quotient % 1) != 0: + decimal = 1 / self.multiplier + msg = f'`{self.amount_field}` must be multiple of {decimal}' + raise ValidationError(msg) + + pre_commit = _check_amount + pre_create = _check_amount diff --git a/rero_ils/modules/patron_transaction_events/jsonschemas/patron_transaction_events/patron_transaction_event-v0.0.1.json b/rero_ils/modules/patron_transaction_events/jsonschemas/patron_transaction_events/patron_transaction_event-v0.0.1.json index f0016cbded..120f01828d 100644 --- a/rero_ils/modules/patron_transaction_events/jsonschemas/patron_transaction_events/patron_transaction_event-v0.0.1.json +++ b/rero_ils/modules/patron_transaction_events/jsonschemas/patron_transaction_events/patron_transaction_event-v0.0.1.json @@ -42,9 +42,9 @@ }, "amount": { "title": "Amount", + "description": "The fee amount as cents", "type": "number", - "exclusiveMinimum": 0, - "multipleOf": 0.01 + "exclusiveMinimum": 0 }, "steps": { "title": "Steps", @@ -60,8 +60,7 @@ }, "amount": { "type": "number", - "exclusiveMinimum": 0, - "multipleOf": 0.01 + "exclusiveMinimum": 0 } }, "required": [ diff --git a/rero_ils/modules/patron_transaction_events/models.py b/rero_ils/modules/patron_transaction_events/models.py index b372164ffa..3b34b29b9e 100644 --- a/rero_ils/modules/patron_transaction_events/models.py +++ b/rero_ils/modules/patron_transaction_events/models.py @@ -40,3 +40,12 @@ class PatronTransactionEventMetadata(db.Model, RecordMetadataBase): """PatronTransactionEvent record metadata.""" __tablename__ = 'patron_transaction_event_metadata' + + +class PatronTransactionEventType: + """Type of PatronTransactionEvent.""" + + FEE = 'fee' + PAYMENT = 'payment' + DISPUTE = 'dispute' + CANCEL = 'cancel' diff --git a/rero_ils/modules/patron_transactions/api.py b/rero_ils/modules/patron_transactions/api.py index 1d95376292..0f7ed525f2 100644 --- a/rero_ils/modules/patron_transactions/api.py +++ b/rero_ils/modules/patron_transactions/api.py @@ -1,8 +1,8 @@ # -*- coding: utf-8 -*- # # RERO ILS -# Copyright (C) 2019 RERO -# Copyright (C) 2020 UCLouvain +# Copyright (C) 2022 RERO +# Copyright (C) 2022 UCLouvain # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by @@ -18,10 +18,9 @@ """API for manipulating patron transactions.""" -from datetime import datetime, timezone from functools import partial -from flask_babelex import gettext as _ +from werkzeug.utils import cached_property from .models import PatronTransactionIdentifier, PatronTransactionMetadata from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch @@ -31,7 +30,7 @@ from ..patron_transaction_events.api import PatronTransactionEvent, \ PatronTransactionEventsSearch from ..providers import Provider -from ..utils import extracted_data_from_ref, get_ref_for_pid, sorted_pids +from ..utils import extracted_data_from_ref, sorted_pids # provider PatronTransactionProvider = type( @@ -78,6 +77,10 @@ class PatronTransaction(IlsRecord): } } + # API METHODS ============================================================= + # Overriding the `IlsRecord` default behavior for create and update + # Invenio API methods. + @classmethod def create(cls, data, id_=None, delete_pid=False, dbcommit=False, reindex=False, steps=None, **kwargs): @@ -103,156 +106,57 @@ def update(self, data, commit=True, dbcommit=True, reindex=True): return super().update( data=data, commit=commit, dbcommit=dbcommit, reindex=reindex) - @classmethod - def _build_transaction_query(cls, patron_pid, status=None, types=None): - """Private function to build a transaction query linked to a patron. - - :param patron_pid: the patron pid being searched - :param status: (optional) array of transaction status filter, - :param types: (optional) array of transaction types filter, - :return: return prepared query. - """ - query = PatronTransactionsSearch() \ - .filter('term', patron__pid=patron_pid) - if status: - query = query.filter('term', status=status) - if types: - query = query.filter('terms', type=types) - return query - - @classmethod - def get_transactions_pids_for_patron(cls, patron_pid, status=None): - """Get patron transactions linked to a patron. - - :param patron_pid: the patron pid being searched - :param status: (optional) transaction status filter, - """ - query = cls._build_transaction_query(patron_pid, status) - results = query.source('pid').scan() - for result in results: - yield result.pid - - @classmethod - def get_transactions_total_amount_for_patron(cls, patron_pid, status=None, - types=None, - with_subscription=True): - """Get total amount transactions linked to a patron. - - :param patron_pid: the patron pid being searched - :param status: (optional) transaction status filter, - :param types: (optional) transaction type filter, - :param with_subscription: (optional) include or exclude subscription - type filter. - :return: return total amount of transactions. - """ - search = cls._build_transaction_query(patron_pid, status, types) - if not with_subscription: - search = search.exclude('terms', type=['subscription']) - search.aggs.metric('transactions_total_amount', - 'sum', field='total_amount') - # set the from/size to 0 - search = search[0:0] - results = search.execute() - return results.aggregations.transactions_total_amount.value - - @classmethod - def get_transactions_count_for_patron(cls, patron_pid, status=None): - """Get patron transactions count linked to a patron. - - :param patron_pid: the patron pid being searched - :param status: (optional) transaction status filter, - """ - query = cls._build_transaction_query(patron_pid, status) - return query.source().count() - - @classmethod - def get_transactions_pids_for_patron(cls, patron_pid, status=None): - """Get patron transactions pids linked to a patron. - - :param patron_pid: the patron pid being searched - :param status: (optional) transaction status filter, - """ - query = cls._build_transaction_query(patron_pid, status) - return sorted_pids(query) - - @classmethod - def get_transactions_by_patron_pid(cls, patron_pid, status=None): - """Return opened fees for patron. + # GETTER & SETTER ========================================================= + # * Define some properties as shortcut to quickly access object attrs. + # * Defines some getter methods to access specific object values. - :param patron_pid: the patron PID - :param status: the status of transaction - :return: return an array of transaction - """ - query = cls._build_transaction_query(patron_pid, status) - return query\ - .params(preserve_order=True)\ - .sort({'creation_date': {'order': 'asc'}})\ - .scan() - - @classmethod - def get_last_transaction_by_loan_pid(cls, loan_pid, status=None): - """Return last fee for loan. + @property + def loan_pid(self): + """Get the `Loan` pid related to this transaction.""" + if self.get('loan'): + return extracted_data_from_ref(self['loan']) - :param loan_pid: the loan PID - :param status: the status of transaction - :return: return last transaction transaction - """ - query = PatronTransactionsSearch() \ - .filter('term', loan__pid=loan_pid) - if status: - query = query.filter('term', status=status) - results = query\ - .sort({'creation_date': {'order': 'desc'}})\ - .source('pid').scan() - try: - pid = next(results).pid - return PatronTransaction.get_record_by_pid(pid) - except StopIteration: - pass + @cached_property + def loan(self): + """Get the `Loan` record related to this transaction.""" + from ..loans.api import Loan + pid = self.loan_pid + if pid: + return Loan.get_record_by_pid(pid) @property def document_pid(self): - """Return the document pid of the the patron transaction.""" - return self.loan.document_pid if self.loan else None + """Get the `Document` pid related to this transaction.""" + loan = self.loan + if loan: + return loan.document_pid @property def library_pid(self): - """Return the library pid of the the patron transaction.""" - return self.loan.library_pid if self.loan else None + """Get the `Library` pid related to this transaction.""" + loan = self.loan + if loan: + return loan.library_pid @property def patron_pid(self): - """Return the patron pid of the patron transaction.""" + """Get the `Patron` pid related to this transaction.""" return extracted_data_from_ref(self.get('patron')) @property def total_amount(self): - """Return the total_amount of the patron transaction.""" + """Shortcut to get the transaction total_amount of the transaction.""" return self.get('total_amount') - @property - def loan_pid(self): - """Return the loan pid linked to the patron transaction.""" - if self.get('loan'): - return extracted_data_from_ref(self['loan']) - - @property - def loan(self): - """Return the `Loan` linked to the patron transaction.""" - from ..loans.api import Loan - pid = self.loan_pid - if pid: - return Loan.get_record_by_pid(pid) - @property def notification_pid(self): - """Return the notification pid of the patron transaction.""" + """Get the `Notification` pid related to this transaction.""" if self.get('notification'): return extracted_data_from_ref(self.get('notification')) - @property + @cached_property def notification(self): - """Return the notification of the patron transaction.""" + """Get the `Notification` record related to this transaction.""" from ..notifications.api import Notification pid = self.notification_pid if pid: @@ -279,118 +183,6 @@ def status(self): """Return the status of the patron transaction.""" return self.get('status') - @classmethod - def create_patron_transaction_from_overdue_loan( - cls, loan, dbcommit=True, reindex=True, delete_pid=False): - """Create a patron transaction for an overdue loan.""" - from ..loans.utils import sum_for_fees - fees = loan.get_overdue_fees - total_amount = sum_for_fees(fees) - if total_amount > 0: - data = { - 'loan': { - '$ref': get_ref_for_pid('loans', loan.pid) - }, - 'patron': { - '$ref': get_ref_for_pid('ptrn', loan.patron_pid) - }, - 'organisation': { - '$ref': get_ref_for_pid('org', loan.organisation_pid) - }, - 'type': 'overdue', - 'status': 'open', - 'note': 'incremental overdue fees', - 'total_amount': total_amount, - 'creation_date': datetime.now(timezone.utc).isoformat(), - } - steps = [ - {'timestamp': fee[1].isoformat(), 'amount': fee[0]} - for fee in fees - ] - return cls.create( - data, - dbcommit=dbcommit, - reindex=reindex, - delete_pid=delete_pid, - steps=steps - ) - - @classmethod - def create_patron_transaction_from_notification( - cls, notification=None, dbcommit=None, reindex=None, - delete_pid=None): - """Create a patron transaction from notification.""" - from ..notifications.utils import calculate_notification_amount - total_amount = calculate_notification_amount(notification) - if total_amount > 0: # no need to create transaction if amount <= 0 ! - data = { - 'notification': { - '$ref': get_ref_for_pid('notif', notification.pid) - }, - 'loan': { - '$ref': get_ref_for_pid('loans', notification.loan_pid) - }, - 'patron': { - '$ref': get_ref_for_pid('ptrn', notification.patron_pid) - }, - 'organisation': { - '$ref': get_ref_for_pid( - 'org', - notification.organisation_pid - ) - }, - 'total_amount': total_amount, - 'creation_date': datetime.now(timezone.utc).isoformat(), - 'type': 'overdue', - 'status': 'open' - } - return cls.create( - data, - dbcommit=dbcommit, - reindex=reindex, - delete_pid=delete_pid - ) - - @classmethod - def create_subscription_for_patron(cls, patron, patron_type, start_date, - end_date, dbcommit=None, reindex=None, - delete_pid=None): - """Create a subscription patron transaction for a patron. - - :param patron: the patron linked to the subscription - :param patron_type: the patron_type for which we need to create the - subscription - :param start_date: As `datetime`, the starting date of the subscription - :param end_date: As `datetime`, the ending date of the subscription - """ - record = {} - if patron_type.is_subscription_required: - data = { - 'patron': { - '$ref': get_ref_for_pid('ptrn', patron.pid) - }, - 'organisation': { - '$ref': get_ref_for_pid('org', patron.organisation_pid) - }, - 'total_amount': patron_type.get('subscription_amount'), - 'creation_date': datetime.now(timezone.utc).isoformat(), - 'type': 'subscription', - 'status': 'open', - 'note': _("Subscription for '{name}' from {start} to {end}") - .format( - name=patron_type.get('name'), - start=start_date.strftime('%Y-%m-%d'), - end=end_date.strftime('%Y-%m-%d') - ) - } - record = cls.create( - data, - dbcommit=dbcommit, - reindex=reindex, - delete_pid=delete_pid - ) - return record - @property def currency(self): """Return patron transaction currency.""" @@ -401,21 +193,23 @@ def currency(self): @property def events(self): """Shortcut for events of the patron transaction.""" - results = PatronTransactionEventsSearch()\ - .filter('term', parent__pid=self.pid).source('pid').scan() - for result in results: + query = PatronTransactionEventsSearch()\ + .filter('term', parent__pid=self.pid)\ + .source('pid') + for result in query.scan(): yield PatronTransactionEvent.get_record_by_pid(result.pid) def get_number_of_patron_transaction_events(self): """Get number of patron transaction events.""" return PatronTransactionEventsSearch()\ - .filter('term', parent__pid=self.pid).source().count() + .filter('term', parent__pid=self.pid)\ + .count() def get_links_to_me(self, get_pids=False): - """Record links. + """Get the links between this record and other records. - :param get_pids: if True list of linked pids - if False count of linked records + :param get_pids: if True, return list of linked pids, otherwise return + count of linked records """ links = {} query = PatronTransactionEventsSearch() \ @@ -427,11 +221,11 @@ def get_links_to_me(self, get_pids=False): def reasons_not_to_delete(self): """Get reasons not to delete record.""" - cannot_delete = {} + reasons = {} links = self.get_links_to_me() if links: - cannot_delete['links'] = links - return cannot_delete + reasons['links'] = links + return reasons class PatronTransactionsIndexer(IlsRecordsIndexer): diff --git a/rero_ils/modules/patron_transactions/utils.py b/rero_ils/modules/patron_transactions/utils.py new file mode 100644 index 0000000000..1e2189cb79 --- /dev/null +++ b/rero_ils/modules/patron_transactions/utils.py @@ -0,0 +1,209 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2022 RERO +# Copyright (C) 2022 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Utility functions about PatronTransactions.""" +from datetime import datetime, timezone + +from flask_babelex import gettext as _ + +from .api import PatronTransaction, PatronTransactionsSearch +from ..utils import get_ref_for_pid + + +def _build_transaction_query(patron_pid, status=None, types=None): + """Private function to build a transaction query linked to a patron. + + :param patron_pid: the patron pid being searched + :param status: (optional) array of transaction status filter, + :param types: (optional) array of transaction types filter, + :return: return prepared query. + """ + query = PatronTransactionsSearch() \ + .filter('term', patron__pid=patron_pid) + if status: + query = query.filter('term', status=status) + if types: + query = query.filter('terms', type=types) + return query + + +def get_transactions_pids_for_patron(patron_pid, status=None): + """Get patron transactions linked to a patron. + + :param patron_pid: the patron pid being searched + :param status: (optional) transaction status filter, + """ + query = _build_transaction_query(patron_pid, status) + for result in query.source('pid').scan(): + yield result.pid + + +def get_transactions_total_amount_for_patron( + patron_pid, status=None, types=None, with_subscription=True): + """Get total amount transactions linked to a patron. + + :param patron_pid: the patron pid being searched + :param status: (optional) transaction status filter, + :param types: (optional) transaction type filter, + :param with_subscription: (optional) include or exclude subscription + type filter. + :return: return total amount of transactions. + """ + search = _build_transaction_query(patron_pid, status, types) + if not with_subscription: + search = search.exclude('terms', type=['subscription']) + search.aggs.metric('pttr_total_amount', 'sum', field='total_amount') + search = search[0:0] # set the from/size to 0 ; no need es hits + results = search.execute() + return results.aggregations.pttr_total_amount.value + + +def get_last_transaction_by_loan_pid(loan_pid, status=None): + """Return last fee for loan. + + :param loan_pid: the loan pid to search. + :param status: (optional) the status of transaction. + :return: return last transaction transaction matching criteria. + """ + query = PatronTransactionsSearch() \ + .filter('term', loan__pid=loan_pid) + if status: + query = query.filter('term', status=status) + results = query\ + .sort({'creation_date': {'order': 'desc'}})\ + .source('pid').scan() + try: + pid = next(results).pid + return PatronTransaction.get_record_by_pid(pid) + except StopIteration: + return None + + +def create_patron_transaction_from_overdue_loan( + loan, dbcommit=True, reindex=True, delete_pid=False): + """Create a patron transaction for an overdue loan.""" + from ..loans.utils import sum_for_fees + fees = loan.get_overdue_fees + total_amount = sum_for_fees(fees) + if total_amount > 0: + data = { + 'loan': { + '$ref': get_ref_for_pid('loans', loan.pid) + }, + 'patron': { + '$ref': get_ref_for_pid('ptrn', loan.patron_pid) + }, + 'organisation': { + '$ref': get_ref_for_pid('org', loan.organisation_pid) + }, + 'type': 'overdue', + 'status': 'open', + 'note': 'incremental overdue fees', + 'total_amount': total_amount, + 'creation_date': datetime.now(timezone.utc).isoformat(), + } + steps = [ + {'timestamp': fee[1].isoformat(), 'amount': fee[0]} + for fee in fees + ] + return PatronTransaction.create( + data, + dbcommit=dbcommit, + reindex=reindex, + delete_pid=delete_pid, + steps=steps + ) + + +def create_patron_transaction_from_notification( + notification=None, dbcommit=None, reindex=None, + delete_pid=None): + """Create a patron transaction from notification.""" + from ..notifications.utils import calculate_notification_amount + total_amount = calculate_notification_amount(notification) + if total_amount > 0: # no need to create transaction if amount <= 0 ! + data = { + 'notification': { + '$ref': get_ref_for_pid('notif', notification.pid) + }, + 'loan': { + '$ref': get_ref_for_pid('loans', notification.loan_pid) + }, + 'patron': { + '$ref': get_ref_for_pid('ptrn', notification.patron_pid) + }, + 'organisation': { + '$ref': get_ref_for_pid( + 'org', + notification.organisation_pid + ) + }, + 'total_amount': total_amount, + 'creation_date': datetime.now(timezone.utc).isoformat(), + 'type': 'overdue', + 'status': 'open' + } + return PatronTransaction.create( + data, + dbcommit=dbcommit, + reindex=reindex, + delete_pid=delete_pid + ) + + +def create_subscription_for_patron( + patron, patron_type, start_date, end_date, dbcommit=None, reindex=None, + delete_pid=None): + """Create a subscription patron transaction for a patron. + + :param patron: the patron linked to the subscription + :param patron_type: the patron_type for which we need to create the + subscription + :param start_date: As `datetime`, the starting date of the subscription + :param end_date: As `datetime`, the ending date of the subscription + :param dbcommit: is the database must be directly committed. + :param reindex: is the indexes must be directly updated. + :param delete_pid: is the database should be cleaned from deleted pids. + """ + record = {} + if patron_type.is_subscription_required: + data = { + 'patron': { + '$ref': get_ref_for_pid('ptrn', patron.pid) + }, + 'organisation': { + '$ref': get_ref_for_pid('org', patron.organisation_pid) + }, + 'total_amount': patron_type.get('subscription_amount'), + 'creation_date': datetime.now(timezone.utc).isoformat(), + 'type': 'subscription', + 'status': 'open', + 'note': _("Subscription for '{name}' from {start} to {end}") + .format( + name=patron_type.get('name'), + start=start_date.strftime('%Y-%m-%d'), + end=end_date.strftime('%Y-%m-%d') + ) + } + record = PatronTransaction.create( + data, + dbcommit=dbcommit, + reindex=reindex, + delete_pid=delete_pid + ) + return record diff --git a/rero_ils/modules/patron_types/api.py b/rero_ils/modules/patron_types/api.py index 580882e742..7dbeeafddb 100644 --- a/rero_ils/modules/patron_types/api.py +++ b/rero_ils/modules/patron_types/api.py @@ -33,7 +33,8 @@ get_overdue_loan_pids from ..loans.models import LoanState from ..minters import id_minter -from ..patron_transactions.api import PatronTransaction +from ..patron_transactions.utils import \ + get_transactions_total_amount_for_patron from ..patrons.api import Patron, PatronsSearch from ..providers import Provider from ..utils import get_patron_from_arguments, sorted_pids @@ -349,8 +350,7 @@ def check_fee_amount_limit(self, patron): if default_limit: # get total amount for open transactions on overdue and without # subscription fee - patron_total_amount = PatronTransaction. \ - get_transactions_total_amount_for_patron( + patron_total_amount = get_transactions_total_amount_for_patron( patron.pid, status='open', types=['overdue'], with_subscription=False) return patron_total_amount < default_limit @@ -368,8 +368,7 @@ def check_unpaid_subscription(self, patron): .get('unpaid_subscription', True) if not unpaid_subscription_limit: return True, None - unpaid_amount = PatronTransaction. \ - get_transactions_total_amount_for_patron( + unpaid_amount = get_transactions_total_amount_for_patron( patron.pid, status='open', types=['subscription'], with_subscription=True) return unpaid_amount == 0 diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 087f3dadeb..d7e5ad94c7 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -38,6 +38,8 @@ from ..minters import id_minter from ..organisations.api import Organisation from ..patron_transactions.api import PatronTransaction +from ..patron_transactions.utils import create_subscription_for_patron, \ + get_transactions_pids_for_patron from ..providers import Provider from ..templates.api import TemplatesSearch from ..users.api import User @@ -501,15 +503,14 @@ def get_links_to_me(self, get_pids=False): .exclude('terms', state=exclude_states) template_query = TemplatesSearch()\ .filter('term', creator__pid=self.pid) + transactions = get_transactions_pids_for_patron( + self.pid, status='open') if get_pids: loans = sorted_pids(loan_query) - transactions = PatronTransaction \ - .get_transactions_pids_for_patron(self.pid, status='open') templates = sorted_pids(template_query) else: loans = loan_query.count() - transactions = PatronTransaction \ - .get_transactions_count_for_patron(self.pid, status='open') + transactions = len(list(transactions)) templates = template_query.count() if loans: links['loans'] = loans @@ -655,7 +656,7 @@ def add_subscription(self, patron_type, start_date, end_date, :param start_date: As `datetime`, the subscription start date :param end_date: As `datetime`, the subscription end date (excluded) """ - transaction = PatronTransaction.create_subscription_for_patron( + transaction = create_subscription_for_patron( self, patron_type, start_date, end_date, dbcommit=dbcommit, reindex=reindex, delete_pid=delete_pids) if transaction: diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py index 64ff43488e..fa431fefbf 100644 --- a/rero_ils/modules/patrons/views.py +++ b/rero_ils/modules/patrons/views.py @@ -43,7 +43,8 @@ from ..loans.api import get_loans_stats_by_patron_pid, get_overdue_loans from ..loans.utils import sum_for_fees from ..locations.api import Location -from ..patron_transactions.api import PatronTransaction +from ..patron_transactions.utils import \ + get_transactions_total_amount_for_patron from ..patron_types.api import PatronType, PatronTypesSearch from ..users.api import User from ..utils import extracted_data_from_ref, get_base_url @@ -74,8 +75,7 @@ def patron_circulation_informations(patron_pid): sum_for_fees(loan.get_overdue_fees) for loan in get_overdue_loans(patron.pid) ) - engaged_amount = PatronTransaction\ - .get_transactions_total_amount_for_patron(patron.pid, status='open') + engaged_amount = get_transactions_total_amount_for_patron(patron.pid, status='open') return jsonify({ 'fees': { 'engaged': engaged_amount, diff --git a/rero_ils/modules/selfcheck/api.py b/rero_ils/modules/selfcheck/api.py index fa609de375..d3a983796e 100644 --- a/rero_ils/modules/selfcheck/api.py +++ b/rero_ils/modules/selfcheck/api.py @@ -39,6 +39,9 @@ get_loans_by_patron_pid from ..loans.models import LoanAction, LoanState from ..patron_transactions.api import PatronTransaction +from ..patron_transactions.utils import get_last_transaction_by_loan_pid, \ + get_transactions_pids_for_patron, \ + get_transactions_total_amount_for_patron from ..patrons.api import Patron @@ -162,8 +165,7 @@ def patron_status(barcode, **kwargs): valid_patron=patron.is_patron ) - fee_amount = PatronTransaction \ - .get_transactions_total_amount_for_patron( + fee_amount = get_transactions_total_amount_for_patron( patron.pid, status='open', with_subscription=False) patron_status_response['fee_amount'] = '%.2f' % fee_amount return patron_status_response @@ -231,15 +233,13 @@ def patron_information(barcode, **kwargs): item.get('barcode') ) - fee_amount = PatronTransaction \ - .get_transactions_total_amount_for_patron( + fee_amount = get_transactions_total_amount_for_patron( patron.pid, status='open', with_subscription=False) patron_account_information['fee_amount'] = '%.2f' % fee_amount # check for fine items if fee_amount > 0: # Check if fine items exist - transaction_pids = PatronTransaction \ - .get_transactions_pids_for_patron( + transaction_pids = get_transactions_pids_for_patron( patron.pid, status='open') for transaction_pid in transaction_pids: # TODO: return screen message to notify patron if there are @@ -307,8 +307,7 @@ def item_information(item_barcode, **kwargs): if loan: # format the end date according selfcheck language item_information['due_date'] = loan['end_date'] - transaction = PatronTransaction. \ - get_last_transaction_by_loan_pid( + transaction = get_last_transaction_by_loan_pid( loan_pid=loan.pid, status='open') if transaction: item_information['fee_amount'] = \ @@ -523,8 +522,7 @@ def selfcheck_renew(transaction_user_pid, item_barcode, **kwargs): renew['renewal'] = True renew['desensitize'] = True renew['due_date'] = loan['end_date'] - transaction = PatronTransaction. \ - get_last_transaction_by_loan_pid( + transaction = get_last_transaction_by_loan_pid( loan_pid=loan.pid, status='open') if transaction: # TODO: map transaction type diff --git a/tests/api/circulation/test_actions_views_checkin.py b/tests/api/circulation/test_actions_views_checkin.py index 9e277d4c87..0207f5631e 100644 --- a/tests/api/circulation/test_actions_views_checkin.py +++ b/tests/api/circulation/test_actions_views_checkin.py @@ -25,7 +25,8 @@ from rero_ils.modules.items.api import Item from rero_ils.modules.items.models import ItemStatus from rero_ils.modules.loans.utils import get_circ_policy, sum_for_fees -from rero_ils.modules.patron_transactions.api import PatronTransaction +from rero_ils.modules.patron_transactions.utils import \ + get_last_transaction_by_loan_pid def test_checkin_an_item( @@ -185,7 +186,7 @@ def test_checkin_overdue_item( assert item.status == ItemStatus.ON_SHELF # check if overdue transaction are created - trans = PatronTransaction.get_last_transaction_by_loan_pid(loan.pid) + trans = get_last_transaction_by_loan_pid(loan.pid) assert trans.total_amount == total_fees events = list(trans.events) assert len(events) == 1 diff --git a/tests/api/test_patron_payments_rest.py b/tests/api/patron_transaction_events/test_patron_payments_rest.py similarity index 54% rename from tests/api/test_patron_payments_rest.py rename to tests/api/patron_transaction_events/test_patron_payments_rest.py index a2ffd59d21..ddb6531ee5 100644 --- a/tests/api/test_patron_payments_rest.py +++ b/tests/api/patron_transaction_events/test_patron_payments_rest.py @@ -28,48 +28,54 @@ def test_patron_payment( client, librarian_martigny, - librarian_sion, patron_transaction_overdue_event_martigny): + patron_transaction_overdue_event_martigny): """Test patron payment.""" - transaction = \ - patron_transaction_overdue_event_martigny.patron_transaction() - calculated_amount = sum([event.amount - for event in - transaction.events]) + ptre = patron_transaction_overdue_event_martigny + transaction = ptre.patron_transaction + calculated_amount = sum(event.amount for event in transaction.events) transaction = PatronTransaction.get_record_by_pid(transaction.pid) assert calculated_amount == transaction.total_amount == 2.00 login_user_via_session(client, librarian_martigny.user) post_entrypoint = 'invenio_records_rest.ptre_list' - payment = deepcopy(patron_transaction_overdue_event_martigny) + payment = deepcopy(ptre) - # partial payment + # STEP#1 :: PARTIAL PAYMENT WITH TOO MUCH DECIMAL + # Try to pay a part of the transaction amount, but according to + # event amount restriction, only 2 decimals are allowed. del payment['pid'] payment['type'] = 'payment' payment['subtype'] = 'cash' - payment['amount'] = 0.54 - payment['operator'] = {'$ref': get_ref_for_pid( - 'patrons', librarian_martigny.pid)} - res, _ = postdata( - client, - post_entrypoint, - payment - ) + payment['amount'] = 0.545 + payment['operator'] = { + '$ref': get_ref_for_pid('patrons', librarian_martigny.pid) + } + res, _ = postdata(client, post_entrypoint, payment) + assert res.status_code == 400 + + # STEP#2 :: PARTIAL PAYMENT WITH GOOD NUMBER OF DECIMALS + # Despite if a set a number with 3 decimals, if this number represent + # the value of a 2 decimals, it's allowed + payment['amount'] = 0.540 + res, _ = postdata(client, post_entrypoint, payment) assert res.status_code == 201 transaction = PatronTransaction.get_record_by_pid(transaction.pid) assert transaction.total_amount == 1.46 assert transaction.status == 'open' - # full payment - payment['type'] = 'payment' - payment['subtype'] = 'cash' - payment['amount'] = 1.46 - res, _ = postdata( - client, - post_entrypoint, - payment - ) - assert res.status_code == 201 + # STEP#3 :: PAY TOO MUCH MONEY + # Try to proceed a payment with too much money, the system must + # reject the payment + payment['amount'] = 2 + res, data = postdata(client, post_entrypoint, payment) + assert res.status_code == 400 + # STEP#4 :: PAY THE REST + # Conclude the transaction by creation of a payment for the rest of the + # transaction + payment['amount'] = transaction.total_amount + res, _ = postdata(client, post_entrypoint, payment) + assert res.status_code == 201 transaction = PatronTransaction.get_record_by_pid(transaction.pid) - assert transaction.total_amount == 0.00 + assert transaction.total_amount == 0 assert transaction.status == 'closed' diff --git a/tests/api/patron_transactions/test_patron_transactions_rest.py b/tests/api/patron_transactions/test_patron_transactions_rest.py index 6f3e6dbc35..c19b72ed5f 100644 --- a/tests/api/patron_transactions/test_patron_transactions_rest.py +++ b/tests/api/patron_transactions/test_patron_transactions_rest.py @@ -27,7 +27,8 @@ from utils import VerifyRecordPermissionPatch, get_json, postdata, \ to_relative_url -from rero_ils.modules.patron_transactions.api import PatronTransaction +from rero_ils.modules.patron_transactions.utils import \ + create_subscription_for_patron, get_transactions_pids_for_patron from rero_ils.modules.utils import add_years @@ -470,7 +471,7 @@ def test_patron_subscription_transaction( assert subscription_end_date.month == subscription_start_date.month assert subscription_end_date.day == subscription_start_date.day - subscription = PatronTransaction.create_subscription_for_patron( + subscription = create_subscription_for_patron( patron_sion, patron_type_youngsters_sion, subscription_start_date, @@ -489,12 +490,9 @@ def test_patron_subscription_transaction( def test_get_transactions_pids_for_patron(patron_sion): """Test function get_transactions_pids_for_patron.""" - assert PatronTransaction.get_transactions_count_for_patron( - patron_sion.pid - ) == 2 - assert len(list(PatronTransaction.get_transactions_pids_for_patron( + assert len(list(get_transactions_pids_for_patron( patron_sion.pid, status='open' ))) == 2 - assert len(list(PatronTransaction.get_transactions_pids_for_patron( + assert len(list(get_transactions_pids_for_patron( patron_sion.pid, status='closed' ))) == 0 diff --git a/tests/ui/loans/test_loans_api.py b/tests/ui/loans/test_loans_api.py index fc3b33ba9a..8aca03edbd 100644 --- a/tests/ui/loans/test_loans_api.py +++ b/tests/ui/loans/test_loans_api.py @@ -42,6 +42,8 @@ from rero_ils.modules.notifications.tasks import create_notifications, \ process_notifications from rero_ils.modules.patron_transactions.api import PatronTransaction +from rero_ils.modules.patron_transactions.utils import \ + get_transactions_pids_for_patron def test_loan_es_mapping(es_clear, db): @@ -237,8 +239,7 @@ def test_anonymizer_job( loan['transaction_date'] = one_year_ago.isoformat() loan = loan.update(loan, dbcommit=True, reindex=True) # close open transactions and notifications - for hit in PatronTransaction.get_transactions_by_patron_pid( - patron.get('pid'), 'open'): + for hit in get_transactions_pids_for_patron(patron.get('pid'), 'open'): transaction = PatronTransaction.get_record_by_pid(hit.pid) transaction['status'] = 'closed' transaction.update(transaction, dbcommit=True, reindex=True) diff --git a/tests/ui/patron_transaction_events/test_patron_transaction_events_api.py b/tests/ui/patron_transaction_events/test_patron_transaction_events_api.py index 4f6a5d1e32..305efb8b1b 100644 --- a/tests/ui/patron_transaction_events/test_patron_transaction_events_api.py +++ b/tests/ui/patron_transaction_events/test_patron_transaction_events_api.py @@ -43,7 +43,7 @@ def test_patron_transaction_event_create( next_pid = PatronTransactionEvent.provider.identifier.next() patron_event['type'] = 'fee' - patron_event['amount'] = 2.1999999999999997 + patron_event['amount'] = 2.2 record = PatronTransactionEvent.create(patron_event, delete_pid=True) next_pid += 1 assert record == patron_event