From fd8d498c81b7b0d6ec57d8f4d32eb0a5e1421235 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Mari=C3=A9thoz?= Date: Thu, 7 Nov 2019 08:49:14 +0100 Subject: [PATCH] instance: fix several bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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: #598. * Fixes import document using EAN from the BNF server. Closes: #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: #608. * Forces instance jinja templates to be loaded before all other templates. Co-Authored-by: Johnny Mariéthoz --- data/libraries.json | 56 ++++++++++- rero_ils/config.py | 2 +- .../documents/document-v0.0.1.json | 2 +- .../rero_ils/detailed_view_documents.html | 4 +- rero_ils/modules/documents/views.py | 95 ++++++++++++------- rero_ils/modules/ext.py | 11 +++ .../form_patrons/patron-v0.0.1.json | 5 +- .../jsonschemas/patrons/patron-v0.0.1.json | 3 +- .../security/email/reset_instructions.html | 21 ++++ .../security/email/reset_instructions.txt | 3 + rero_ils/views.py | 22 ++--- tests/api/test_patrons_rest.py | 6 +- tests/ui/patrons/test_patrons_api.py | 2 + .../select-item-type-type.component.ts | 2 +- 14 files changed, 176 insertions(+), 58 deletions(-) create mode 100644 rero_ils/templates/security/email/reset_instructions.html create mode 100644 rero_ils/templates/security/email/reset_instructions.txt diff --git a/data/libraries.json b/data/libraries.json index 3d2a30b40b..a49272bea3 100644 --- a/data/libraries.json +++ b/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" + } + ] + } + ] } -] \ No newline at end of file +] diff --git a/rero_ils/config.py b/rero_ils/config.py index ff78fb48bb..359008ebf0 100644 --- a/rero_ils/config.py +++ b/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' diff --git a/rero_ils/modules/documents/jsonschemas/documents/document-v0.0.1.json b/rero_ils/modules/documents/jsonschemas/documents/document-v0.0.1.json index f4c880f69c..f450ef5164 100644 --- a/rero_ils/modules/documents/jsonschemas/documents/document-v0.0.1.json +++ b/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": [ diff --git a/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html b/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html index 97873c3658..964980e43a 100644 --- a/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html +++ b/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html @@ -265,7 +265,8 @@

{{ item.call_number }} {% endif %} - {% if item|can_request %} + {% set locations = item|item_library_pickup_locations %} + {% if item|can_request and locations %}

diff --git a/rero_ils/modules/documents/views.py b/rero_ils/modules/documents/views.py index c0c2e6b583..883dbb6ae8 100644 --- a/rero_ils/modules/documents/views.py +++ b/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,74 @@ def cover(isbn): @api_blueprint.route("/import/bnf/") @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 +335,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 diff --git a/rero_ils/modules/ext.py b/rero_ils/modules/ext.py index cb603a6119..a47fc509f4 100644 --- a/rero_ils/modules/ext.py +++ b/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') diff --git a/rero_ils/modules/patrons/jsonschemas/form_patrons/patron-v0.0.1.json b/rero_ils/modules/patrons/jsonschemas/form_patrons/patron-v0.0.1.json index c1c6d4d53d..fac9ea51f8 100644 --- a/rero_ils/modules/patrons/jsonschemas/form_patrons/patron-v0.0.1.json +++ b/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." + } } ] }, diff --git a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json b/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json index 4fc18a7aec..35a02b2164 100644 --- a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json +++ b/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", diff --git a/rero_ils/templates/security/email/reset_instructions.html b/rero_ils/templates/security/email/reset_instructions.html new file mode 100644 index 0000000000..2f17c0ac2b --- /dev/null +++ b/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 . + +#} + +

{{ _('Click here to reset your password') }}

\ No newline at end of file diff --git a/rero_ils/templates/security/email/reset_instructions.txt b/rero_ils/templates/security/email/reset_instructions.txt new file mode 100644 index 0000000000..27cb0ffdc0 --- /dev/null +++ b/rero_ils/templates/security/email/reset_instructions.txt @@ -0,0 +1,3 @@ +{{ _('Click the link below to reset your password:') }} + +{{ reset_link.replace('/api', '') }} \ No newline at end of file diff --git a/rero_ils/views.py b/rero_ils/views.py index 5662aa05d8..d419b0fa7a 100644 --- a/rero_ils/views.py +++ b/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, diff --git a/tests/api/test_patrons_rest.py b/tests/api/test_patrons_rest.py index 3931d905b6..b35409904f 100644 --- a/tests/api/test_patrons_rest.py +++ b/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,7 +137,7 @@ 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') @@ -144,6 +145,7 @@ def test_patrons_post_put_delete(client, lib_martigny, 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) diff --git a/tests/ui/patrons/test_patrons_api.py b/tests/ui/patrons/test_patrons_api.py index 83446c2c61..4f962bbc25 100644 --- a/tests/ui/patrons/test_patrons_api.py +++ b/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' diff --git a/ui/src/app/records/editor/select-item-type-type/select-item-type-type.component.ts b/ui/src/app/records/editor/select-item-type-type/select-item-type-type.component.ts index 46c541dddc..8d855fbca4 100644 --- a/ui/src/app/records/editor/select-item-type-type/select-item-type-type.component.ts +++ b/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); } } }