From cc39288004edd8615278cbaaa5b92021f8117926 Mon Sep 17 00:00:00 2001 From: Aly Badr Date: Wed, 6 Nov 2019 13:27:26 +0100 Subject: [PATCH] renewals: add renew buttons for patrons checked-out items * Displays renew button when renewal of checked-out item is possible. * Hides renew button when circulation policies do not permit the renewal action. * Increases units testing coverage. * Corrects indentation of the patron_profile Jinja template. Co-Authored-by: Aly Badr --- rero_ils/modules/items/api.py | 39 ++-- .../templates/rero_ils/patron_profile.html | 183 +++++++++--------- rero_ils/modules/patrons/views.py | 63 +++--- tests/ui/holdings/test_holdings_item.py | 3 + tests/ui/patrons/test_patrons_ui.py | 41 +++- 5 files changed, 207 insertions(+), 122 deletions(-) diff --git a/rero_ils/modules/items/api.py b/rero_ils/modules/items/api.py index baa4b2d0a1..195ec8bc75 100644 --- a/rero_ils/modules/items/api.py +++ b/rero_ils/modules/items/api.py @@ -33,6 +33,7 @@ from .models import ItemIdentifier, ItemStatus from ..api import IlsRecord, IlsRecordError, IlsRecordIndexer, IlsRecordsSearch +from ..circ_policies.api import CircPolicy from ..documents.api import Document, DocumentsSearch from ..errors import InvalidRecordID from ..fetchers import id_fetcher @@ -547,10 +548,33 @@ def last_location_pid(self): return loan_location_pid return self.location_pid + def can_extend(self, loan): + """Checks if the patron has the rights to renew this item.""" + from ..loans.utils import extend_loan_data_is_valid + can_extend = True + patron_pid = loan.get('patron_pid') + patron_type_pid = Patron.get_record_by_pid( + patron_pid).patron_type_pid + circ_policy = CircPolicy.provide_circ_policy( + self.library_pid, + patron_type_pid, + self.item_type_pid + ) + extension_count = loan.get('extension_count', 0) + if not ( + circ_policy.get('number_renewals') > 0 and + extension_count < circ_policy.get('number_renewals') and + extend_loan_data_is_valid( + loan.get('end_date'), + circ_policy.get('renewal_duration'), + self.library_pid + ) + ) or self.number_of_requests(): + can_extend = False + return can_extend + def action_filter(self, action, loan): """Filter actions.""" - from ..circ_policies.api import CircPolicy - from ..loans.utils import extend_loan_data_is_valid patron_pid = loan.get('patron_pid') patron_type_pid = Patron.get_record_by_pid( patron_pid).patron_type_pid @@ -564,16 +588,7 @@ def action_filter(self, action, loan): 'new_action': None } if action == 'extend': - extension_count = loan.get('extension_count', 0) - if not ( - circ_policy.get('number_renewals') > 0 and - extension_count < circ_policy.get('number_renewals') and - extend_loan_data_is_valid( - loan.get('end_date'), - circ_policy.get('renewal_duration'), - self.library_pid - ) - ) or self.number_of_requests(): + if not self.can_extend(loan): data['action_validated'] = False if action == 'checkout': if not circ_policy.get('allow_checkout'): diff --git a/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html b/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html index 8cd16b9a07..3aa2fee272 100644 --- a/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html +++ b/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html @@ -19,115 +19,124 @@ {%- extends 'rero_ils/page.html' %} {%- block body %} - {% include('rero_ils/_patron_profile_head.html') %} +{% include('rero_ils/_patron_profile_head.html') %} -
-
- +
+
+
+ {% if checkouts|length > 0 %} + {{ build_table('checkouts', checkouts) }} + {% else %} +

{{ _('No loan') }}

+ {% endif %} +
+
+ {% if pendings|length > 0 %} + {{ build_table('requests', pendings) }} + {% else %} +

{{ _('No pending') }}

+ {% endif %} +
+
+ {% include('rero_ils/_patron_profile_personal.html') %} +
-{%- endblock body %} +
+ +{%- endblock body %} {% macro build_table(type, loans) %}
- - - - - - {% if type != 'checkouts' %} - - {% endif %} - - {% if type == 'checkouts' %} - - {% endif %} - - - - {% for loan in loans %} - - - - {% if type != 'checkouts' %} - - {% endif %} - + {% if type == 'checkouts' %} + + + {% endif %} + + {% endfor %} + +
{{ _('Title') }}{{ _('Call Number') }} - {{ _('Pickup library') }} - - {% if type == 'checkouts' %} - {{ _('Due date') }} - {% else %} - {{ _('Reservation date') }} - {% endif %} - - {{ _('Renewals') }} -
{{ loan.document_title }}{{ loan.item_call_number }} - {{ loan.pickup_library_name }} - - {% if type == 'checkouts' %} - {{ loan.end_date|format_date( + + + + + + {% if type != 'checkouts' %} + + {% endif %} + + {% if type == 'checkouts' %} + + {% endif %} + + + + {% for loan in loans %} + + + + {% if type != 'checkouts' %} + + {% endif %} + - {% if type == 'checkouts' %} - - {% endif %} - - {% endfor %} - -
{{ _('Title') }}{{ _('Call Number') }} + {{ _('Pickup library') }} + + {% if type == 'checkouts' %} + {{ _('Due date') }} + {% else %} + {{ _('Reservation date') }} + {% endif %} + + {{ _('Renewals') }} +
{{ loan.document_title }}{{ loan.item_call_number }} + {{ loan.pickup_library_name }} + + {% if type == 'checkouts' %} + {{ loan.end_date|format_date( format='short_date', locale=current_i18n.locale.language )}} - {% else %} - {{ loan.transaction_date|format_date( + {% else %} + {{ loan.transaction_date|format_date( format='short_date', locale=current_i18n.locale.language )}} - {% endif %} - - {{ loan.extension_count }} -
+ {% endif %} + 		+ {{ loan.extension_count }} + + {% if loan.can_renew %} + {%- with form = can_renew_form %} +
+ + +
+ {%- endwith %} + {% endif %} +
{% endmacro %} + diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py index bc752de1b7..0c71d712a8 100644 --- a/rero_ils/modules/patrons/views.py +++ b/rero_ils/modules/patrons/views.py @@ -19,7 +19,8 @@ from __future__ import absolute_import, print_function -from flask import Blueprint, current_app, jsonify, render_template, request +from flask import Blueprint, current_app, flash, jsonify, render_template, \ + request from flask_babelex import gettext as _ from flask_login import current_user, login_required from flask_menu import register_menu @@ -31,7 +32,7 @@ from ..documents.api import Document from ..items.api import Item from ..libraries.api import Library -from ..loans.api import get_loans_by_patron_pid +from ..loans.api import get_loans_by_patron_pid, Loan from ..locations.api import Location api_blueprint = Blueprint( @@ -88,7 +89,8 @@ def logged_user(): return jsonify(data) -@blueprint.route('/global/patrons/profile', defaults={'viewcode': 'global'}) +@blueprint.route('/global/patrons/profile', defaults={'viewcode': 'global'}, + methods=['GET', 'POST']) @blueprint.route('//patrons/profile') @login_required @register_menu( @@ -102,28 +104,45 @@ def profile(viewcode): patron = Patron.get_patron_by_user(current_user) if patron is None: raise NotFound() + if request.method == 'POST': + loan = Loan.get_record_by_pid(request.values.get('loan_pid')) + item = Item.get_record_by_pid(loan.get('item_pid')) + data = { + 'item_pid': item.pid, + 'pid': request.values.get('loan_pid'), + 'transaction_location_pid': item.location_pid + } + try: + item.extend_loan(**data) + flash(_('The item %(item_id)s has been renewed.', + item_id=item.pid), 'success') + except Exception: + flash(_('Error during the renewal of the item %(item_id)s.', + item_id=item.pid), 'danger') + loans = get_loans_by_patron_pid(patron.pid) checkouts = [] requests = [] - if loans: - for loan in loans: - item_pid = loan.get('item_pid') - item = Item.get_record_by_pid(item_pid).replace_refs() - document = Document.get_record_by_pid(item['document']['pid']) - loan['document_title'] = document['title'] - loan['item_call_number'] = item['call_number'] - if loan['state'] == 'ITEM_ON_LOAN': - checkouts.append(loan) - elif loan['state'] in ( - 'PENDING', - 'ITEM_AT_DESK', - 'ITEM_IN_TRANSIT_FOR_PICKUP' - ): - pickup_loc = Location.get_record_by_pid( - loan['pickup_location_pid']) - loan['pickup_library_name'] = \ - pickup_loc.get_library().get('name') - requests.append(loan) + for loan in loans: + item_pid = loan.get('item_pid') + item = Item.get_record_by_pid(item_pid) + document = Document.get_record_by_pid( + item.replace_refs()['document']['pid']) + loan['document_title'] = document['title'] + loan['item_call_number'] = item['call_number'] + if loan['state'] == 'ITEM_ON_LOAN': + loan['can_renew'] = item.can_extend(loan) + checkouts.append(loan) + elif loan['state'] in ( + 'PENDING', + 'ITEM_AT_DESK', + 'ITEM_IN_TRANSIT_FOR_PICKUP' + ): + pickup_loc = Location.get_record_by_pid( + loan['pickup_location_pid']) + loan['pickup_library_name'] = \ + pickup_loc.get_library().get('name') + requests.append(loan) return render_template( 'rero_ils/patron_profile.html', record=patron, diff --git a/tests/ui/holdings/test_holdings_item.py b/tests/ui/holdings/test_holdings_item.py index af0a033d75..aa258c7454 100644 --- a/tests/ui/holdings/test_holdings_item.py +++ b/tests/ui/holdings/test_holdings_item.py @@ -85,6 +85,9 @@ def test_holding_item_links(client, holding_lib_martigny, item_lib_martigny, assert not holding_lib_martigny.delete_from_index() holding_lib_martigny.dbcommit(forceindex=True) + # test item count by holdings pid + assert holding_lib_martigny.get_items_count_by_holding_pid == 2 + def test_holding_delete_after_item_deletion( client, holding_lib_martigny, item_lib_martigny): diff --git a/tests/ui/patrons/test_patrons_ui.py b/tests/ui/patrons/test_patrons_ui.py index 17f17bb465..81ca9a5d56 100644 --- a/tests/ui/patrons/test_patrons_ui.py +++ b/tests/ui/patrons/test_patrons_ui.py @@ -22,12 +22,16 @@ import mock from flask import url_for +from rero_ils.modules.items.api import Item from invenio_accounts.testutils import login_user_via_session from utils import get_json, to_relative_url def test_patrons_profile( - client, librarian_martigny_no_email, loan_pending_martigny): + client, librarian_martigny_no_email, loan_pending_martigny, + lib_martigny, patron_martigny_no_email, loc_public_martigny, + item_type_standard_martigny, item_lib_martigny, json_header, + circ_policy_short_martigny): """Test patron profile.""" # check redirection res = client.get(url_for('patrons.profile')) @@ -40,6 +44,41 @@ def test_patrons_profile( res = client.get(url_for('patrons.profile')) assert res.status_code == 200 + # create patron transactions + data = { + 'patron_pid': patron_martigny_no_email.pid, + 'item_pid': item_lib_martigny.pid, + 'pickup_location_pid': loc_public_martigny.pid + } + loan = item_lib_martigny.request(**data) + loan_pid = loan[1].get('request').get('pid') + + loan = item_lib_martigny.checkout(**data) + + # patron visits his profile to list checked-out items + login_user_via_session(client, patron_martigny_no_email.user) + res = client.get(url_for('patrons.profile')) + assert res.status_code == 200 + + # patron successfully renew the item + res = client.post( + url_for('patrons.profile'), + data={'loan_pid': loan_pid} + ) + assert res.status_code == 200 + + # disable possiblity to renew the item + circ_policy_short_martigny['number_renewals'] = 0 + circ_policy_short_martigny.update( + circ_policy_short_martigny, dbcommit=True, reindex=True) + + # patron fails to renew the item + res = client.post( + url_for('patrons.profile'), + data={'loan_pid': loan_pid} + ) + assert res.status_code == 200 + def test_patrons_logged_user(client, librarian_martigny_no_email): """Test logged user info API."""