instance: fix several bugs

* Fixes item types type validation.
* Fixes validation message for patron phone number.
* Adds opening hours to the library of the 3th organisation.
* Fixes document detailed view for patrons with organisation without
pickup locations. Closes: rero#598.
* Fixes import document using EAN from the BNF server. Closes: rero#607.
* Improves logs to BNF document importation.
* Fixes logged user initial user menu.
* Fixes reset password links in the user email when a librarian create a
patron. Closes: rero#608.
* Forces instance jinja templates to be loaded before all other
templates.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>

data/libraries.json

@@ -544,6 +544,58 @@
     "name": "Biblioth\u00e8que cantonale",
     "organisation": {
       "$ref": "https://ils.rero.ch/api/organisations/3"
-    }
+    },
+    "opening_hours": [
+      {
+        "day": "monday",
+        "is_open": true,
+        "times": [
+          {
+            "start_time": "07:00",
+            "end_time": "19:00"
+          }
+        ]
+      },
+      {
+        "day": "tuesday",
+        "is_open": false,
+        "times": []
+      },
+      {
+        "day": "wednesday",
+        "is_open": false,
+        "times": []
+      },
+      {
+        "day": "thursday",
+        "is_open": true,
+        "times": [
+          {
+            "start_time": "07:00",
+            "end_time": "19:00"
+          }
+        ]
+      },
+      {
+        "day": "friday",
+        "is_open": false,
+        "times": []
+      },
+      {
+        "day": "saturday",
+        "is_open": false,
+        "times": []
+      },
+      {
+        "day": "sunday",
+        "is_open": true,
+        "times": [
+          {
+            "start_time": "14:00",
+            "end_time": "18:00"
+          }
+        ]
+      }
+    ]
   }
-]
+]

rero_ils/config.py

@@ -1031,7 +1031,7 @@ def _(x):
 RERO_ILS_APP_IMPORT_BNF_EAN = 'http://catalogue.bnf.fr/api/SRU?'\
                               'version=1.2&operation=searchRetrieve'\
                               '&recordSchema=unimarcxchange&maximumRecords=1'\
-                              '&startRecord=1&query=bib.ean%%20all%%20"%s"'
+                              '&startRecord=1&query=bib.ean all "{}"'
 
 RERO_ILS_APP_HELP_PAGE = (
     'https://github.com/rero/rero-ils/wiki/Public-demo-help'

rero_ils/modules/documents/jsonschemas/documents/document-v0.0.1.json

@@ -562,7 +562,7 @@
       "title": "Authors",
       "description": "Author(s) of the resource. Can be either persons or organisations.",
       "type": "array",
-      "minItems": 0,
+      "minItems": 1,
       "items": {
         "type": "object",
         "required": [

rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html

@@ -265,7 +265,8 @@ <h3>
                   {{ item.call_number }}
                   {% endif %}
                 </div>
-                {% if item|can_request %}
+                {% set locations = item|item_library_pickup_locations %}
+                {% if item|can_request and locations %}
                 <div class="col">
                   <a href="#" type="button" class="btn btn-primary btn-sm" data-toggle="dropdown" aria-haspopup="true"
                     aria-expanded="false" id="dropdownMenu">
@@ -276,7 +277,6 @@ <h3>
                   <div class="dropdown-menu dropdown-menu-right" aria-labelledby="dropdownMenu">
                     <h6 class="dropdown-header">{{ _('Select a Pickup Location') }}</h6>
                     <div class="dropdown-divider"></div>
-                    {% set locations = item|item_library_pickup_locations %}
                     {% for location in locations %}
                     <a class="dropdown-item"
                       href="{{ url_for('item.patron_request', viewcode=viewcode, item_pid=item.pid, pickup_location_pid=location.pid)}}">

rero_ils/modules/documents/views.py

@@ -21,9 +21,7 @@
 
 import json
 import re
-import sys
 from functools import wraps
-from urllib.request import urlopen
 
 import pycountry
 import requests
@@ -113,46 +111,76 @@ def cover(isbn):
 @api_blueprint.route("/import/bnf/<int:ean>")
 @check_permission
 def import_bnf_ean(ean):
-    """Import record from BNFr given a isbn 13 without dashes."""
+    """Import record from BNFr given a isbn 13 without dashes.
+
+    See: https://catalogue.bnf.fr/api/test.do
+    """
     bnf_url = current_app.config['RERO_ILS_APP_IMPORT_BNF_EAN']
     try:
-        with urlopen(bnf_url % ean) as response:
-            if response.status != 200:
-                abort(502)
-            # read the xml date from the HTTP response
-            xml_data = response.read()
-
-            # create a xml file in memory
-            xml_file = six.BytesIO()
-            xml_file.write(xml_data)
-            xml_file.seek(0)
-
-            # get the record in xml if exists
-            # note: the request should returns one record max
-            xml_record = next(split_stream(xml_file))
-
-            # convert xml in marc json
-            json_data = create_record(xml_record)
-
-            # convert marc json to local json format
-            record = unimarctojson.do(json_data)
-            response = {
-                'metadata': record
-            }
-            return jsonify(response)
+        with requests.get(bnf_url.format(ean)) as response:
+            if not response.ok:
+                status_code = 502
+                response = {
+                    'metadata': {},
+                    'errors': {
+                        'code': status_code,
+                        'title': 'The BNF server returns a bad status code.',
+                        'detail': 'Status code: {}'.format(
+                            response.status_code)
+                    }
+                }
+                current_app.logger.error(
+                    '{}: {}'.format(
+                        response.get('title'), response.get('detail')))
+
+            else:
+                # read the xml date from the HTTP response
+                xml_data = response.content
+
+                # create a xml file in memory
+                xml_file = six.BytesIO()
+                xml_file.write(xml_data)
+                xml_file.seek(0)
+
+                # get the record in xml if exists
+                # note: the request should returns one record max
+                xml_record = next(split_stream(xml_file))
+
+                # convert xml in marc json
+                json_data = create_record(xml_record)
+
+                # convert marc json to local json format
+                record = unimarctojson.do(json_data)
+                response = {
+                    'metadata': record
+                }
+                status_code = 200
     # no record found!
     except StopIteration:
+        status_code = 404
         response = {
-            'record': {}
+            'metadata': {},
+            'errors': {
+                'code': status_code,
+                'title': 'The EAN was not found on the BNF server.'
+            }
         }
-        return jsonify(response), 404
     # other errors
-    except Exception:
-        sys.stdout.flush()
+    except Exception as e:
+        status_code = 500
         response = {
-            'record': {}
+            'metadata': {},
+            'errors': {
+                'code': status_code,
+                'title': 'An unexpected error has been raise.',
+                'detail': 'Error: {}'.format(e)
+            }
         }
-        return jsonify(response), 500
+        current_app.logger.error(
+            '{}: {}'.format(
+                response.get('title'), response.get('detail')))
+
+    return jsonify(response), status_code
 
 
 blueprint = Blueprint(
@@ -309,7 +337,8 @@ def item_library_pickup_locations(item):
     for library in organisation.get_libraries():
         location = Location.get_record_by_pid(
             library.get_pickup_location_pid())
-        locations.append(location)
+        if location:
+            locations.append(location)
     return locations
 
 

rero_ils/modules/ext.py

@@ -19,6 +19,7 @@
 
 from __future__ import absolute_import, print_function
 
+import jinja2
 from invenio_admin import current_admin
 from invenio_circulation.signals import loan_state_changed
 from invenio_indexer.signals import before_record_index
@@ -50,6 +51,16 @@ def __init__(self, app=None):
         from ..permissions import can_access_item, can_edit
         if app:
             self.init_app(app)
+            # force to load ils template before others
+            # it is require for Flask-Security see:
+            # https://pythonhosted.org/Flask-Security/customizing.html#emails
+            ils_loader = jinja2.ChoiceLoader([
+                jinja2.PackageLoader('rero_ils', 'templates'),
+                app.jinja_loader
+            ])
+            app.jinja_loader = ils_loader
+
+            # register filters
             app.add_template_filter(format_date_filter, name='format_date')
             app.add_template_filter(to_pretty_json, name='tojson_pretty')
             app.add_template_filter(can_edit, name='can_edit')

rero_ils/modules/patrons/jsonschemas/form_patrons/patron-v0.0.1.json

@@ -41,7 +41,10 @@
         "htmlClass": "mr-2"
       },
       {
-        "key": "phone"
+        "key": "phone",
+        "validationMessage": {
+          "pattern": "Phone number with the international prefix, without spaces, ie +41221234567."
+        }
       }
     ]
   },

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

@@ -78,8 +78,7 @@
       "title": "Phone number",
       "description": "Phone number with the international prefix, without spaces.",
       "type": "string",
-      "pattern": "^\\+[0-9]*$",
-      "validationMessage": "Phone number with the international prefix, without spaces, ie +41221234567."
+      "pattern": "^\\+[0-9]*$"
     },
     "patron_type": {
       "title": "Patron Type",

rero_ils/templates/security/email/reset_instructions.html

@@ -0,0 +1,21 @@
+{# -*- coding: utf-8 -*-
+
+    RERO ILS
+    Copyright (C) 2019 RERO
+    Copyright (C) 2015-2018 CERN
+
+    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 <http://www.gnu.org/licenses/>.
+
+#}
+
+<p><a href="{{ reset_link.replace('/api', '') }}">{{ _('Click here to reset your password') }}</a></p>

rero_ils/templates/security/email/reset_instructions.txt

@@ -0,0 +1,3 @@
+{{ _('Click the link below to reset your password:') }}
+
+{{ reset_link.replace('/api', '') }}

rero_ils/views.py

@@ -33,7 +33,7 @@
 from invenio_jsonschemas.errors import JSONSchemaNotFound
 
 from rero_ils.modules.organisations.api import Organisation
-from rero_ils.modules.patrons.api import Patron
+from rero_ils.modules.patrons.api import current_patron
 
 from .modules.babel_extractors import translate
 from .version import __version__
@@ -99,19 +99,15 @@ def hide_language(lang):
 def init_menu_profile():
     """Create the profile header menu."""
     item = current_menu.submenu('main.profile')
-    account = _('My Account')
-    user_initials = session.get('user_initials')
-    if session.get('user_id'):
-        if user_initials:
-            account = user_initials
-        else:
-            patron = Patron.get_patron_by_email(current_user.email)
-            if patron:
-                account = patron.initial
-                session['user_initials'] = account
+    if current_patron:
+        session['user_initials'] = current_patron.initial
     else:
-        if user_initials:
-            del session['user_initials']
+        try:
+            session['user_initials'] = current_user.email
+        # AnonymousUser
+        except AttributeError:
+            session.pop('user_initials', None)
+    account = session.get('user_initials', _('My Account'))
 
     item.register(
         endpoint=None,

tests/api/test_patrons_rest.py

@@ -18,6 +18,7 @@
 """Tests REST API patrons."""
 
 import json
+import re
 from copy import deepcopy
 
 import mock
@@ -136,14 +137,15 @@ def test_patrons_get(client, librarian_martigny_no_email):
 def test_patrons_post_put_delete(client, lib_martigny,
                                  patron_type_children_martigny,
                                  librarian_martigny_data, json_header,
-                                 roles):
+                                 roles, mailbox):
     """Test record retrieval."""
     item_url = url_for('invenio_records_rest.ptrn_item', pid_value='1')
     list_url = url_for('invenio_records_rest.ptrn_list', q='pid:1')
     patron_data = librarian_martigny_data
 
     pids = len(Patron.get_all_pids())
     uuids = len(Patron.get_all_ids())
+    assert len(mailbox) == 0
 
     # Create record / POST
     patron_data['pid'] = '1'
@@ -158,6 +160,8 @@ def test_patrons_post_put_delete(client, lib_martigny,
     assert res.status_code == 201
     assert len(Patron.get_all_pids()) == pids + 1
     assert len(Patron.get_all_ids()) == uuids + 1
+    assert len(mailbox) == 1
+    assert re.search(r'localhost/lost-password', mailbox[0].body)
 
     # Check that the returned record matches the given data
     data = get_json(res)

tests/ui/patrons/test_patrons_api.py

@@ -20,6 +20,7 @@
 from __future__ import absolute_import, print_function
 
 from utils import get_mapping
+import re
 
 from rero_ils.modules.patrons.api import Patron, PatronsSearch, \
     patron_id_fetcher
@@ -42,6 +43,7 @@ def test_patron_create(app, roles, librarian_martigny_data_tmp,
     user_roles = [r.name for r in user.roles]
     assert set(user_roles) == set(ptrn.get('roles'))
     assert len(mailbox) == 1
+    assert re.search(r'localhost/lost-password', mailbox[0].body)
     assert ptrn.get('email') in mailbox[0].recipients
     assert ptrn == librarian_martigny_data_tmp
     assert ptrn.get('pid') == '1'

ui/src/app/records/editor/select-item-type-type/select-item-type-type.component.ts

@@ -78,7 +78,7 @@ export class SelectItemTypeTypeComponent implements OnInit {
         pid
       );
     } else {
-      return of();
+      return of(true);
     }
   }
 }