From 4569f3383aed88b961b21035d91831d44ada6c00 Mon Sep 17 00:00:00 2001
From: Aly Badr <aly.badr@rero.ch>
Date: Sat, 11 May 2019 19:32:13 +0200
Subject: [PATCH] global: test coverage and docs for patrons

* BETTER increase test coverage and add docs for patrons.

Signed-off-by: Aly Badr <aly.badr@rero.ch>
---
 rero_ils/modules/patrons/api.py               |  10 +-
 .../modules/patrons/jsonschemas/__init__.py   |   2 +-
 rero_ils/modules/patrons/listener.py          |  41 +++--
 rero_ils/modules/patrons/mappings/__init__.py |   2 +-
 .../modules/patrons/mappings/v6/__init__.py   |   2 +-
 rero_ils/modules/patrons/utils.py             |   2 +-
 rero_ils/modules/patrons/views.py             |  16 +-
 tests/api/test_items_rest_views.py            |   4 +
 tests/api/test_loans_rest.py                  |   4 +-
 tests/api/test_patrons_rest.py                |   5 +
 tests/api/test_patrons_views.py               | 152 ++++++++++++++++++
 tests/data/data.json                          |   1 +
 tests/fixtures/circulation.py                 |   2 +-
 tests/ui/patrons/test_patrons_api.py          |   6 +-
 tests/ui/patrons/test_patrons_ui.py           |   5 +-
 tests/unit/test_patrons_jsonschema.py         |   2 +-
 16 files changed, 201 insertions(+), 55 deletions(-)
 create mode 100644 tests/api/test_patrons_views.py

diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py
index 12527e5fb8..aa7eed753b 100644
--- a/rero_ils/modules/patrons/api.py
+++ b/rero_ils/modules/patrons/api.py
@@ -81,7 +81,7 @@ class Patron(IlsRecord):
     @classmethod
     def create(cls, data, id_=None, delete_pid=True,
                dbcommit=False, reindex=False, **kwargs):
-        """."""
+        """Patron record creation."""
         record = super(Patron, cls).create(
             data, id_, delete_pid, dbcommit, reindex, **kwargs)
         record._update_roles()
@@ -101,7 +101,7 @@ def update(self, data, dbcommit=False, reindex=False):
 
     @cached_property
     def user(self):
-        """."""
+        """Invenio user of a patron."""
         email = self.get('email')
         user = _datastore.find_user(email=email)
         if not user:
@@ -116,7 +116,7 @@ def user(self):
         return user
 
     def _update_roles(self):
-        """."""
+        """Update user roles."""
         db_roles = self.user.roles
         for role in self.available_roles:
             in_db = role in db_roles
@@ -127,7 +127,7 @@ def _update_roles(self):
                 self.remove_role(role)
 
     def _remove_roles(self):
-        """."""
+        """Remove roles."""
         db_roles = self.user.roles
         for role in self.available_roles:
             if role in db_roles:
@@ -162,7 +162,7 @@ def get_pid_by_email(cls, email):
 
     @classmethod
     def get_librarian_pickup_location_pid(cls):
-        """."""
+        """Returns pickup locations for a librarian."""
         if 'librarian' in current_patron['roles']:
             library = Library.get_record_by_pid(
                 current_patron.replace_refs()['library']['pid']
diff --git a/rero_ils/modules/patrons/jsonschemas/__init__.py b/rero_ils/modules/patrons/jsonschemas/__init__.py
index 416bb9164d..25ed3f6001 100644
--- a/rero_ils/modules/patrons/jsonschemas/__init__.py
+++ b/rero_ils/modules/patrons/jsonschemas/__init__.py
@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""JSON schemas for rero-ils."""
+"""JSON schemas for patrons."""
diff --git a/rero_ils/modules/patrons/listener.py b/rero_ils/modules/patrons/listener.py
index 5a46d22ca9..022768130f 100644
--- a/rero_ils/modules/patrons/listener.py
+++ b/rero_ils/modules/patrons/listener.py
@@ -40,24 +40,23 @@ def func_item_at_desk(sender, *args, **kwargs):
     """Function for signal item_at_desk."""
     item = kwargs['item']
 
-    # Get patron for holding.
-    holdings = item.get('_circulation', {}).get('holdings', [])
-    if holdings:
-        patron_barcode = holdings[0].get('patron_barcode')
-        patron = Patron.get_patron_by_barcode(patron_barcode)
-
-        if patron:
-            # Send at desk mail
-            subject = _('Document at desk')
-            email = patron.get('email')
-            recipients = [email]
-            template = 'patron_request_at_desk'
-            send_mail(
-                subject=subject,
-                recipients=recipients,
-                template=template,
-                language='eng',
-                document=Document.get_document_by_itemid(item.id),
-                # holding=item.dumps().get('_circulation').get('holdings')[0],
-                holding='get loans with state ITEM_AT_DESK, To be implemented',
-            )
+    requests = item.number_of_requests()
+    if requests:
+        for request in item.get_requests():
+            if request.get('state') == 'ITEM_AT_DESK':
+                patron = Patron.get_record_by_pid(request.get('patron_pid'))
+                if patron:
+                    # Send at desk mail
+                    subject = _('Document at desk')
+                    document_pid = request.get('document_pid')
+                    email = patron.get('email')
+                    recipients = [email]
+                    template = 'patron_request_at_desk'
+                    send_mail(
+                        subject=subject,
+                        recipients=recipients,
+                        template=template,
+                        language='eng',
+                        document=Document.get_record_by_pid(document_pid),
+                        holding=request
+                    )
diff --git a/rero_ils/modules/patrons/mappings/__init__.py b/rero_ils/modules/patrons/mappings/__init__.py
index 37d287d54f..f94c864d20 100644
--- a/rero_ils/modules/patrons/mappings/__init__.py
+++ b/rero_ils/modules/patrons/mappings/__init__.py
@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""Elasticsearch mappings."""
+"""Elasticsearch mappings for patrons."""
diff --git a/rero_ils/modules/patrons/mappings/v6/__init__.py b/rero_ils/modules/patrons/mappings/v6/__init__.py
index 37d287d54f..f94c864d20 100644
--- a/rero_ils/modules/patrons/mappings/v6/__init__.py
+++ b/rero_ils/modules/patrons/mappings/v6/__init__.py
@@ -22,4 +22,4 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""Elasticsearch mappings."""
+"""Elasticsearch mappings for patrons."""
diff --git a/rero_ils/modules/patrons/utils.py b/rero_ils/modules/patrons/utils.py
index 3490161b72..a42f99081f 100644
--- a/rero_ils/modules/patrons/utils.py
+++ b/rero_ils/modules/patrons/utils.py
@@ -22,7 +22,7 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""Utilities functions for rero-ils."""
+"""Utilities functions for patrons."""
 
 
 from flask_login import current_user
diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py
index dd75f4445b..66b8c50f34 100644
--- a/rero_ils/modules/patrons/views.py
+++ b/rero_ils/modules/patrons/views.py
@@ -26,8 +26,6 @@
 
 from __future__ import absolute_import, print_function
 
-from functools import wraps
-
 from flask import Blueprint, current_app, jsonify, render_template, request
 from flask_babelex import gettext as _
 from flask_login import current_user, login_required
@@ -42,7 +40,6 @@
 from ..libraries.api import Library
 from ..loans.api import get_loans_by_patron_pid
 from ..locations.api import Location
-from ...permissions import login_and_librarian
 
 api_blueprint = Blueprint(
     'api_patrons',
@@ -53,16 +50,6 @@
 )
 
 
-def check_permission(fn):
-    """."""
-    @wraps(fn)
-    def decorated_view(*args, **kwargs):
-        """."""
-        login_and_librarian()
-        return fn(*args, **kwargs)
-    return decorated_view
-
-
 blueprint = Blueprint(
     'patrons',
     __name__,
@@ -121,7 +108,6 @@ def profile():
     if patron is None:
         raise NotFound()
     loans = get_loans_by_patron_pid(patron.pid)
-
     checkouts = []
     requests = []
     if loans:
@@ -159,7 +145,7 @@ def get_patron_from_barcode(value):
 
 @blueprint.app_template_filter('get_patron_from_checkout_item_pid')
 def get_patron_from_checkout_item_pid(item_pid):
-    """Get patron from a checkout item pid."""
+    """Get patron from a checked out item pid."""
     from invenio_circulation.api import get_loan_for_item
     patron_pid = get_loan_for_item(item_pid)['patron_pid']
     return Patron.get_record_by_pid(patron_pid)
diff --git a/tests/api/test_items_rest_views.py b/tests/api/test_items_rest_views.py
index 5cdeb84102..e67720092b 100644
--- a/tests/api/test_items_rest_views.py
+++ b/tests/api/test_items_rest_views.py
@@ -79,6 +79,10 @@ def test_checkout_no_loan_given(client, librarian_martigny_no_email,
     data = get_json(res)
     loan_pid = data.get('action_applied')[LoanAction.CHECKOUT].get('loan_pid')
 
+    from rero_ils.modules.patrons.views import  \
+        get_patron_from_checkout_item_pid
+    assert get_patron_from_checkout_item_pid(item.pid) == patron
+
     res = client.post(
         url_for('api_item.checkin'),
         data=json.dumps(
diff --git a/tests/api/test_loans_rest.py b/tests/api/test_loans_rest.py
index 46ee9168e0..7ff8fed671 100644
--- a/tests/api/test_loans_rest.py
+++ b/tests/api/test_loans_rest.py
@@ -31,7 +31,7 @@
 from invenio_accounts.testutils import login_user_via_session
 
 
-def test_loans_permissions(client, loan_pedning, json_header):
+def test_loans_permissions(client, loan_pending, json_header):
     """Test record retrieval."""
     item_url = url_for('invenio_records_rest.loanid_item', pid_value='1')
     post_url = url_for('invenio_records_rest.loanid_list')
@@ -56,7 +56,7 @@ def test_loans_permissions(client, loan_pedning, json_header):
     assert res.status_code == 401
 
 
-def test_loans_logged_permissions(client, loan_pedning,
+def test_loans_logged_permissions(client, loan_pending,
                                   librarian_martigny_no_email,
                                   json_header):
     """Test record retrieval."""
diff --git a/tests/api/test_patrons_rest.py b/tests/api/test_patrons_rest.py
index 2cd430b00f..2f6fd6bb6e 100644
--- a/tests/api/test_patrons_rest.py
+++ b/tests/api/test_patrons_rest.py
@@ -27,8 +27,13 @@
 
 import mock
 from flask import url_for
+from invenio_accounts.testutils import login_user_via_session
 from utils import VerifyRecordPermissionPatch, get_json, to_relative_url
 
+from rero_ils.modules.items.api import ItemStatus
+from rero_ils.modules.loans.api import Loan, LoanAction
+from rero_ils.modules.patrons.utils import user_has_patron
+
 
 def test_patrons_permissions(client, librarian_martigny_no_email,
                              json_header):
diff --git a/tests/api/test_patrons_views.py b/tests/api/test_patrons_views.py
new file mode 100644
index 0000000000..a48a2a7a4e
--- /dev/null
+++ b/tests/api/test_patrons_views.py
@@ -0,0 +1,152 @@
+#
+# This file is part of RERO ILS.
+# Copyright (C) 2017 RERO.
+#
+# RERO ILS is free software; you can redistribute it
+# and/or modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of the
+# License, or (at your option) any later version.
+#
+# RERO ILS 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
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with RERO ILS; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307, USA.
+#
+# In applying this license, RERO does not
+# waive the privileges and immunities granted to it by virtue of its status
+# as an Intergovernmental Organization or submit itself to any jurisdiction.
+
+"""Tests Views for patrons."""
+
+import json
+
+import mock
+import pytest
+from flask import url_for
+from invenio_accounts.testutils import login_user_via_session
+from utils import VerifyRecordPermissionPatch, get_json, to_relative_url
+
+from rero_ils.modules.items.api import ItemStatus
+from rero_ils.modules.loans.api import Loan, LoanAction
+from rero_ils.modules.patrons.cli import import_users
+from rero_ils.modules.patrons.listener import func_item_at_desk, \
+    listener_item_at_desk
+from rero_ils.modules.patrons.utils import user_has_patron
+
+
+def test_patron_can_delete(client, librarian_martigny_no_email,
+                           patron_martigny_no_email, loc_public_martigny,
+                           item_lib_martigny, json_header,
+                           circulation_policies):
+    """Test patron can delete."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    item = item_lib_martigny
+    patron = patron_martigny_no_email
+    location = loc_public_martigny
+    # request
+    res = client.post(
+        url_for('api_item.librarian_request'),
+        data=json.dumps(
+            dict(
+                item_pid=item.pid,
+                pickup_location_pid=location.pid,
+                patron_pid=patron.pid
+            )
+        ),
+        content_type='application/json',
+    )
+    assert res.status_code == 200
+    data = get_json(res)
+    loan_pid = data.get('action_applied')[LoanAction.REQUEST].get('loan_pid')
+
+    links = patron_martigny_no_email.get_links_to_me()
+    assert 'loans' in links
+
+    assert not patron_martigny_no_email.can_delete
+
+    reasons = patron_martigny_no_email.reasons_not_to_delete()
+    assert 'links' in reasons
+
+    item.cancel_loan(loan_pid=loan_pid)
+    assert item.status == ItemStatus.ON_SHELF
+
+
+def test_patron_utils(client, librarian_martigny_no_email,
+                      patron_martigny_no_email, loc_public_martigny,
+                      item_lib_martigny, json_header,
+                      circulation_policies, librarian_martigny):
+    """Test patron utils."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    item = item_lib_martigny
+    patron = patron_martigny_no_email
+    location = loc_public_martigny
+
+    from rero_ils.modules.patrons.views import get_location_name_from_pid
+    assert get_location_name_from_pid(loc_public_martigny.pid) == \
+        location.get('name')
+
+    from rero_ils.modules.patrons.views import get_patron_from_pid
+    assert get_patron_from_pid(patron.pid) == patron
+
+    from rero_ils.modules.patrons.views import get_checkout_loan_for_item
+    assert not get_checkout_loan_for_item(item.pid)
+
+    from rero_ils.modules.patrons.views import get_patron_from_barcode
+    assert get_patron_from_barcode(patron.get('barcode')) == patron
+
+
+def test_librarian_pickup_locations(client, librarian_martigny_no_email,
+                                    lib_martigny, loc_public_martigny,
+                                    patron_martigny_no_email,
+                                    patron_martigny):
+    """Test get librarian pickup locations."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+
+    with mock.patch(
+        'rero_ils.modules.patrons.api.current_patron',
+        librarian_martigny_no_email
+    ):
+        assert user_has_patron
+        pick = librarian_martigny_no_email.get_librarian_pickup_location_pid()
+        assert pick == loc_public_martigny.pid
+
+    with mock.patch(
+        'rero_ils.modules.patrons.api.current_patron',
+        patron_martigny
+    ):
+        assert user_has_patron
+        pick = librarian_martigny_no_email.get_librarian_pickup_location_pid()
+        assert not pick
+        record = patron_martigny
+        del record['roles']
+        assert user_has_patron
+
+
+def test_patron_listener(client, librarian_martigny_no_email,
+                         item_lib_fully,
+                         lib_fully, loc_public_martigny,
+                         patron_martigny_no_email,
+                         patron_martigny,
+                         loan_pending):
+    """Test patron listener."""
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    requests = item_lib_fully.number_of_requests()
+    assert requests == 1
+    for request in item_lib_fully.get_requests():
+        item_lib_fully.validate_request(**request)
+        item_lib_fully.receive(**loan_pending)
+
+    with mock.patch(
+        'rero_ils.modules.ext.current_admin',
+        {}
+    ):
+        sender = {}
+        data = {'item': item_lib_fully}
+        with mock.patch('rero_ils.utils.send_mail'):
+            with pytest.raises(AttributeError):
+                assert listener_item_at_desk(sender, **data)
diff --git a/tests/data/data.json b/tests/data/data.json
index 6cd6d84f25..600d8395d0 100644
--- a/tests/data/data.json
+++ b/tests/data/data.json
@@ -1073,6 +1073,7 @@
     "patron_pid": "ptrn1",
     "start_date": "2019-01-09T08:18:22.576291+00:00",
     "end_date": "2019-02-08T08:18:22.576291+00:00",
+    "pickup_location_pid": "loc1",
     "transaction_date": "2019-01-09T08:18:22.576291+00:00",
     "transaction_location_pid": "loc1",
     "transaction_user_pid": "ptrn1"
diff --git a/tests/fixtures/circulation.py b/tests/fixtures/circulation.py
index 08a084d37d..461eac2dca 100644
--- a/tests/fixtures/circulation.py
+++ b/tests/fixtures/circulation.py
@@ -247,7 +247,7 @@ def loan_data_tmp(data):
 
 
 @pytest.fixture(scope="module")
-def loan_pedning(
+def loan_pending(
         app,
         item_lib_fully,
         loc_public_martigny,
diff --git a/tests/ui/patrons/test_patrons_api.py b/tests/ui/patrons/test_patrons_api.py
index 8b06471ab8..562e565a7e 100644
--- a/tests/ui/patrons/test_patrons_api.py
+++ b/tests/ui/patrons/test_patrons_api.py
@@ -22,7 +22,7 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""CircPolicy Record tests."""
+"""Patrons Record tests."""
 
 from __future__ import absolute_import, print_function
 
@@ -92,7 +92,7 @@ def test_patron_es_mapping(
 
 
 def test_get_patron(librarian_martigny):
-    """."""
+    """Test patron retrieval."""
     patron = librarian_martigny
     assert Patron.get_patron_by_email(patron.get('email')) == patron
     assert not Patron.get_patron_by_email('not exists')
@@ -105,6 +105,6 @@ class user:
 
 
 def test_user_librarian_can_delete(librarian_martigny):
-    """Test can  delete"""
+    """Test can delete a librarian."""
     assert librarian_martigny.get_links_to_me() == {}
     assert librarian_martigny.can_delete
diff --git a/tests/ui/patrons/test_patrons_ui.py b/tests/ui/patrons/test_patrons_ui.py
index d974f9b806..8f09bcdb04 100644
--- a/tests/ui/patrons/test_patrons_ui.py
+++ b/tests/ui/patrons/test_patrons_ui.py
@@ -32,8 +32,8 @@
 from utils import get_json, to_relative_url
 
 
-def test_patrons_profile(client, librarian_martigny_no_email):
-    """."""
+def test_patrons_profile(client, librarian_martigny_no_email, loan_pending):
+    """Test patron profile."""
     # check redirection
     res = client.get(url_for('patrons.profile'))
     assert res.status_code == 302
@@ -61,7 +61,6 @@ class locale:
         'rero_ils.modules.patrons.views.current_i18n',
         current_i18n
     ):
-
         login_user_via_session(client, librarian_martigny_no_email.user)
         res = client.get(url_for('patrons.logged_user'))
         assert res.status_code == 200
diff --git a/tests/unit/test_patrons_jsonschema.py b/tests/unit/test_patrons_jsonschema.py
index 5b5468711b..8cf0e80676 100644
--- a/tests/unit/test_patrons_jsonschema.py
+++ b/tests/unit/test_patrons_jsonschema.py
@@ -22,7 +22,7 @@
 # waive the privileges and immunities granted to it by virtue of its status
 # as an Intergovernmental Organization or submit itself to any jurisdiction.
 
-"""patron JSON schema tests."""
+"""Patron JSON schema tests."""
 
 from __future__ import absolute_import, print_function