Skip to content

Commit

Permalink
instance: fix several bugs
Browse files Browse the repository at this point in the history
* 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.
* Makes exception variable more explicit e -> error.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
  • Loading branch information
jma committed Nov 11, 2019
1 parent 9aa529c commit 33ab836
Show file tree
Hide file tree
Showing 20 changed files with 208 additions and 82 deletions.
56 changes: 54 additions & 2 deletions data/libraries.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
}
]
}
]
]
2 changes: 1 addition & 1 deletion rero_ils/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 3 additions & 2 deletions rero_ils/modules/apiharvester/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ def get_records(url=None, name=None, from_date=None, max=0, size=100,
else:
yield next, records
next = data.get('links', {}).get('next', None)
except Exception as e:
except Exception as error:
click.secho(
'Harvesting API ConnectionRefusedError: {e}'.format(e=e),
'Harvesting API ConnectionRefusedError: {error}'.format(
error=error),
fg='red'
)
return 0, url, []
4 changes: 2 additions & 2 deletions rero_ils/modules/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ def check_json(paths, replace, indent, sort_keys, verbose):
else:
click.echo(fname + ': ', nl=False)
click.secho('Bad indentation', fg='red')
except ValueError as e:
except ValueError as error:
click.echo(fname + ': ', nl=False)
click.secho('Invalid JSON', fg='red', nl=False)
click.echo(' -- ' + e.msg)
click.echo(' -- ' + error.msg)
error_cnt = 1

tot_error_cnt += error_cnt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1340,4 +1340,4 @@
"default": false
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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)}}">
Expand Down
99 changes: 65 additions & 34 deletions rero_ils/modules/documents/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@

import json
import re
import sys
from functools import wraps
from urllib.request import urlopen

import pycountry
import requests
Expand Down Expand Up @@ -113,46 +111,78 @@ 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(
'{title}: {detail}'.format(
title=response.get('title'),
detail=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 error:
status_code = 500
response = {
'record': {}
'metadata': {},
'errors': {
'code': status_code,
'title': 'An unexpected error has been raise.',
'detail': 'Error: {error}'.format(error=error)
}
}
return jsonify(response), 500
current_app.logger.error(
'{title}: {detail}'.format(
title=response.get('title'),
detail=response.get('detail')))

return jsonify(response), status_code


blueprint = Blueprint(
Expand Down Expand Up @@ -309,7 +339,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


Expand Down
11 changes: 11 additions & 0 deletions rero_ils/modules/ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
26 changes: 14 additions & 12 deletions rero_ils/modules/items/api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,13 @@ def jsonify_error(func):
def decorated_view(*args, **kwargs):
try:
return func(*args, **kwargs)
except NotFound as e:
raise(e)
except Exception as e:
# raise(e)
current_app.logger.error(str(e))
return jsonify({'status': 'error: {error}'.format(error=e)}), 500
except NotFound as error:
raise(error)
except Exception as error:
# raise(error)
current_app.logger.error(str(error))
return jsonify({'status': 'error: {error}'.format(
error=error)}), 500
return decorated_view


Expand Down Expand Up @@ -94,14 +95,15 @@ def decorated_view(*args, **kwargs):
'metadata': item_data.dumps_for_circulation(),
'action_applied': action_applied
})
except CirculationException as e:
except CirculationException as error:
abort(403)
except NotFound as e:
raise(e)
except Exception as e:
except NotFound as error:
raise(error)
except Exception as error:
# raise(e)
current_app.logger.error(str(e))
return jsonify({'status': 'error: {error}'.format(error=e)}), 500
current_app.logger.error(str(error))
return jsonify({'status': 'error: {error}'.format(
error=error)}), 500
return decorated_view


Expand Down
5 changes: 3 additions & 2 deletions rero_ils/modules/items/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ def jsonify_error(func):
def decorated_view(*args, **kwargs):
try:
return func(*args, **kwargs)
except Exception as e:
return jsonify({'status': 'error: {error}'.format(error=e)}), 500
except Exception as error:
return jsonify({'status': 'error: {error}'.format(
error=error)}), 500
return decorated_view


Expand Down
4 changes: 2 additions & 2 deletions rero_ils/modules/notifications/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ def replace_pids_and_refs(self):
data['loan']['profile_url'] = profile_url

return data
except Exception as e:
raise(e)
except Exception as error:
raise(error)

def init_loan(self):
"""Set loan of the notification."""
Expand Down
4 changes: 2 additions & 2 deletions rero_ils/modules/notifications/dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,5 @@ def send_mail(self, data):
# TODO: investigate why delay does not work
# send_email.delay(msg.__dict__)
# current_app.extensions['mail'].send(msg)
except Exception as e:
raise(e)
except Exception as error:
raise(error)
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@
"htmlClass": "mr-2"
},
{
"key": "phone"
"key": "phone",
"validationMessage": {
"pattern": "Phone number with the international prefix, without spaces, ie +41221234567."
}
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 21 additions & 0 deletions rero_ils/templates/security/email/reset_instructions.html
Original file line number Diff line number Diff line change
@@ -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>
3 changes: 3 additions & 0 deletions rero_ils/templates/security/email/reset_instructions.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{{ _('Click the link below to reset your password:') }}

{{ reset_link.replace('/api', '') }}
Loading

0 comments on commit 33ab836

Please sign in to comment.