From 4e15fed66ba3d4f78434d3f69c2adb849ee77490 Mon Sep 17 00:00:00 2001 From: Aly Badr Date: Wed, 3 Feb 2021 09:59:36 +0100 Subject: [PATCH 1/2] users: add new resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds a new 'user' resource to manage the user profiles. It contains the user personal data as opposed to the patron data that are link to a specific organisation. * Writes a REST API: * A GET, POST, PUT methods to create, update and retrieves the patron personal information. * A search endpoint to retrieve the patron personal information given an email or a username. * Creates units testing. * Centralizes several views and methods in a specific `User` python class. * Disables the validation email for freshly created user. * Uses the standard ngx-formly `widget` field for the user JSON schema. Co-Authored-by: Aly Badr Co-Authored-by: Johnny Mariéthoz --- rero_ils/accounts_views.py | 8 +- rero_ils/config.py | 5 +- rero_ils/modules/ext.py | 35 ++- .../jsonschemas/patrons/patron-v0.0.1.json | 13 +- rero_ils/modules/patrons/views.py | 38 +--- rero_ils/modules/users/__init__.py | 18 ++ rero_ils/modules/users/api.py | 201 ++++++++++++++++++ .../modules/users/jsonschemas/__init__.py | 18 ++ .../users/jsonschemas/users/user-v0.0.1.json | 160 ++++++++++++++ rero_ils/modules/users/views.py | 142 +++++++++++++ rero_ils/modules/views.py | 28 --- rero_ils/theme/assets/js/reroils/login.js | 6 +- rero_ils/theme/views.py | 3 +- setup.py | 1 + tests/api/patrons/test_patrons_rest.py | 89 -------- tests/api/test_users.py | 106 --------- tests/api/users/test_users_api.py | 183 ++++++++++++++++ tests/data/data.json | 12 ++ tests/fixtures/circulation.py | 7 + tests/unit/conftest.py | 8 + tests/unit/test_users_jsonschema.py | 53 +++++ 21 files changed, 859 insertions(+), 275 deletions(-) create mode 100644 rero_ils/modules/users/__init__.py create mode 100644 rero_ils/modules/users/api.py create mode 100644 rero_ils/modules/users/jsonschemas/__init__.py create mode 100644 rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json create mode 100644 rero_ils/modules/users/views.py delete mode 100644 tests/api/test_users.py create mode 100644 tests/api/users/test_users_api.py create mode 100644 tests/unit/test_users_jsonschema.py diff --git a/rero_ils/accounts_views.py b/rero_ils/accounts_views.py index b418fab5f6..f982ccd162 100644 --- a/rero_ils/accounts_views.py +++ b/rero_ils/accounts_views.py @@ -32,6 +32,7 @@ from .modules.patrons.api import Patron, current_patron from .modules.patrons.permissions import PatronPermission +from .modules.users.api import User current_datastore = LocalProxy( lambda: current_app.extensions['security'].datastore) @@ -60,12 +61,7 @@ class LoginView(CoreLoginView): @classmethod def get_user(cls, email=None, **kwargs): """Retrieve a user by the provided arguments.""" - try: - profile = UserProfile.get_by_username(email) - return profile.user - except NoResultFound: - pass - return current_datastore.get_user(email) + return User.get_by_username_or_email(email).user @use_kwargs(post_args) def post(self, **kwargs): diff --git a/rero_ils/config.py b/rero_ils/config.py index b0b7ad64db..5dec88095b 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -2288,7 +2288,8 @@ def _(x): SECURITY_CHANGEABLE = True #: Allow user to confirm their email address. -SECURITY_CONFIRMABLE = True +# TODO: decide what should be the workflow of the login user +SECURITY_CONFIRMABLE = False #: Allow password recovery by users. SECURITY_RECOVERABLE = True @@ -2300,7 +2301,7 @@ def _(x): SECURITY_SEND_REGISTER_EMAIL = True #: Allow users to login without first confirming their email address. -SECURITY_LOGIN_WITHOUT_CONFIRMATION = False +SECURITY_LOGIN_WITHOUT_CONFIRMATION = True #: TODO: remove this when the email is sent only if the user has an email # address diff --git a/rero_ils/modules/ext.py b/rero_ils/modules/ext.py index a6509bb345..6a140540ff 100644 --- a/rero_ils/modules/ext.py +++ b/rero_ils/modules/ext.py @@ -27,7 +27,9 @@ from invenio_indexer.signals import before_record_index from invenio_oaiharvester.signals import oaiharvest_finished from invenio_records.signals import after_record_insert, after_record_update +from invenio_records_rest.errors import JSONSchemaValidationError from invenio_userprofiles.signals import after_profile_update +from jsonschema.exceptions import ValidationError from .apiharvester.signals import apiharvest_part from .collections.listener import enrich_collection_data @@ -50,6 +52,7 @@ from .patron_transactions.listener import enrich_patron_transaction_data from .patrons.listener import create_subscription_patron_transaction, \ enrich_patron_data, update_from_profile +from .users.views import UsersCreateResource, UsersResource from ..filter import empty_data, format_date_filter, jsondumps, lib_url, \ node_assets, text_to_id, to_pretty_json @@ -87,9 +90,10 @@ def init_app(self, app): Wiki(app) self.init_config(app) app.extensions['rero-ils'] = self - self.register_api_blueprint(app) + self.register_import_api_blueprint(app) + self.register_users_api_blueprint(app) - def register_api_blueprint(self, app): + def register_import_api_blueprint(self, app): """Imports bluprint initialization.""" api_blueprint = Blueprint( 'api_imports', @@ -106,7 +110,6 @@ def register_api_blueprint(self, app): '/import_{endpoint}/'.format(endpoint=endpoint), view_func=imports_search ) - app.register_blueprint(api_blueprint) imports_record = ImportsResource.as_view( 'imports_record', @@ -124,6 +127,32 @@ def handle_bad_request(e): ResultNotFoundOnTheRemoteServer, handle_bad_request) app.register_blueprint(api_blueprint) + def register_users_api_blueprint(self, app): + """User bluprint initialization.""" + api_blueprint = Blueprint( + 'api_users', + __name__ + ) + api_blueprint.add_url_rule( + '/users/', + view_func=UsersResource.as_view( + 'users_item' + ) + ) + api_blueprint.add_url_rule( + '/users/', + view_func=UsersCreateResource.as_view( + 'users_list' + ) + ) + + @api_blueprint.errorhandler(ValidationError) + def validation_error(error): + """Catch validation errors.""" + return JSONSchemaValidationError(error=error).get_response() + + app.register_blueprint(api_blueprint) + def init_config(self, app): """Initialize configuration.""" # Use theme's base template if theme is installed 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 10d8cc1a58..011bb09a45 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 @@ -30,8 +30,17 @@ "type": "string" }, "user_id": { - "title": "User ID", - "type": "number" + "title": "Personal Informations", + "description": "", + "type": "number", + "form": { + "templateOptions": { + "wrappers": [ + "form-field", + "user-id" + ] + } + } }, "first_name": { "title": "First name", diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py index fede6ca8ab..ec4138eaa4 100644 --- a/rero_ils/modules/patrons/views.py +++ b/rero_ils/modules/patrons/views.py @@ -22,7 +22,6 @@ import re from functools import wraps -from elasticsearch_dsl import Q from flask import Blueprint, abort, current_app, flash, jsonify, \ render_template, request, url_for from flask_babelex import format_currency @@ -35,7 +34,7 @@ from werkzeug.exceptions import NotFound from werkzeug.utils import redirect -from .api import Patron, PatronsSearch +from .api import Patron from .permissions import get_allowed_roles_management from .utils import user_has_patron from ..items.api import Item @@ -71,41 +70,6 @@ def is_logged_librarian(*args, **kwargs): return fn(*args, **kwargs) return is_logged_librarian - -@api_blueprint.route('/count/', methods=['GET']) -@check_permission -def number_of_patrons(): - """Returns the number of patrons matching the query. - - The query should be one of the following forms: - - `/api/patrons/count/?q=email:"test@test.ch" - - `/api/patrons/count/?q=email:"test@test.ch" NOT pid:1 - - `/api/patrons/count/?q=username:"test" - - `/api/patrons/count/?q=username:"test" NOT pid:1 - - :return: The number of existing user account corresponding to the given - email or username. - :rtype: A JSON of the form:{"hits": {"total": 1}} - """ - query = request.args.get('q') - email = _EMAIL_REGEX.search(query) - username = _USERNAME_REGEX.search(query) - if not email and not username: - abort(400) - if email: - email = email.group(1) - s = PatronsSearch().query('match', email__analyzed=email) - else: - username = username.group(1) - s = PatronsSearch().query('match', username__analyzed=username) - exclude_pid = _PID_REGEX.search(query) - if exclude_pid: - exclude_pid = exclude_pid.group(1) - s = s.filter('bool', must_not=[Q('term', pid=exclude_pid)]) - response = dict(hits=dict(total=s.count())) - return jsonify(response) - - @api_blueprint.route('//circulation_informations', methods=['GET']) @check_permission def patron_circulation_informations(patron_pid): diff --git a/rero_ils/modules/users/__init__.py b/rero_ils/modules/users/__init__.py new file mode 100644 index 0000000000..44acf0aa0e --- /dev/null +++ b/rero_ils/modules/users/__init__.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""User Record.""" diff --git a/rero_ils/modules/users/api.py b/rero_ils/modules/users/api.py new file mode 100644 index 0000000000..33c3892ff0 --- /dev/null +++ b/rero_ils/modules/users/api.py @@ -0,0 +1,201 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""API for manipulating users.""" + +from datetime import datetime + +from flask import current_app, url_for +from flask_security.confirmable import confirm_user +from flask_security.recoverable import send_reset_password_instructions +from invenio_accounts.ext import hash_password +from invenio_accounts.models import User as BaseUser +from invenio_db import db +from invenio_records.validators import PartialDraft4Validator +from invenio_userprofiles.models import UserProfile +from jsonschema import FormatChecker +from sqlalchemy.orm.exc import NoResultFound +from werkzeug.local import LocalProxy + +from ...utils import remove_empties_from_dict + +_records_state = LocalProxy(lambda: current_app.extensions['invenio-records']) + + +class User(object): + """User API.""" + + profile_fields = [ + 'first_name', 'last_name', 'street', 'postal_code', + 'city', 'birth_date', 'username', 'phone', 'keep_history' + ] + + def __init__(self, user): + """User class initializer.""" + self.user = user + + @classmethod + def create(cls, data, **kwargs): + """User record creation. + + :param cls - class object + :param data - dictionary representing a user record + """ + with db.session.begin_nested(): + email = data.pop('email', None) + roles = data.pop('roles', None) + cls._validate(data=data) + user = BaseUser( + password=hash_password( + data.get('password', + data.get('birth_date', '123456'))), + profile=data, active=True) + db.session.add(user) + profile = user.profile + for field in cls.profile_fields: + value = data.get(field) + if value is not None: + if field == 'birth_date': + value = datetime.strptime(value, '%Y-%m-%d') + setattr(profile, field, value) + # send the reset password notification for new users + if email: + user.email = email + db.session.merge(user) + db.session.commit() + if user.email: + send_reset_password_instructions(user) + confirm_user(user) + return cls(user) + + def update(self, data): + """User record update. + + :param data - dictionary representing a user record to update + """ + self._validate(data=data) + email = data.pop('email', None) + roles = data.pop('roles', None) + user = self.user + with db.session.begin_nested(): + if user.profile is None: + user.profile = UserProfile(user_id=user.id) + profile = user.profile + for field in self.profile_fields: + if field == 'birth_date': + setattr( + profile, field, + datetime.strptime(data.get(field), '%Y-%m-%d')) + else: + setattr(profile, field, data.get(field, '')) + + # change password + if data.get('password'): + user.password = hash_password(data['password']) + + # send reset password if email is changed + if email and email != user.email: + user.email = email + send_reset_password_instructions(user) + db.session.merge(user) + db.session.commit() + confirm_user(user) + return self + + @classmethod + def _validate(cls, data, **kwargs): + """Validate user record against schema.""" + format_checker = FormatChecker() + schema = data.pop('$schema', None) + if schema: + _records_state.validate( + data, + schema, + format_checker=format_checker, + cls=PartialDraft4Validator + ) + return data + + @classmethod + def get_by_id(cls, user_id): + """Get a user by a user_id. + + :param user_id - the user_id + :return: the user record + """ + user = BaseUser.query.filter_by(id=user_id).first() + if not user: + return None + return cls(user) + + def dumps(self): + """Return pure Python dictionary with record metadata.""" + data = { + 'id': self.user.id, + 'links': {'self': url_for( + 'api_users.users_item', _external=True, id=self.user.id)}, + 'metadata': {} + } + if self.user.profile: + for field in self.profile_fields: + value = getattr(self.user.profile, field) + if field == 'birth_date': + value = datetime.strftime(value, '%Y-%m-%d') + data['metadata'][field] = value + data['metadata']['email'] = self.user.email + data['metadata']['roles'] = [r.name for r in self.user.roles] + data = remove_empties_from_dict(data) + return data + + @classmethod + def get_by_username(cls, username): + """Get a user by a username. + + :param username - the user name + :return: the user record + """ + try: + profile = UserProfile.get_by_username(username) + return cls(profile.user) + except NoResultFound: + return None + + @classmethod + def get_by_email(cls, email): + """Get a user by email. + + :param email - the email of the user + :return: the user record + """ + user = BaseUser.query.filter_by(email=email).first() + if not user: + return None + return cls(user) + + @classmethod + def get_by_username_or_email(cls, username_or_email): + """Get a user by email or username. + + :param username_or_email - the username or the email of a user + :return: the user record + """ + user = cls.get_by_email(username_or_email) + if not user: + return cls.get_by_username(username_or_email) + return user + + diff --git a/rero_ils/modules/users/jsonschemas/__init__.py b/rero_ils/modules/users/jsonschemas/__init__.py new file mode 100644 index 0000000000..fed4c9fb9c --- /dev/null +++ b/rero_ils/modules/users/jsonschemas/__init__.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""JSON schemas for users.""" diff --git a/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json b/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json new file mode 100644 index 0000000000..8ebc3ca46c --- /dev/null +++ b/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json @@ -0,0 +1,160 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "title": "User Personal Informations", + "description": "JSON schema for a user.", + "additionalProperties": false, + "propertiesOrder": [ + "first_name", + "last_name", + "username", + "email", + "password", + "street", + "postal_code", + "city", + "birth_date", + "phone", + "keep_history" + ], + "required": [ + "$schema", + "first_name", + "last_name", + "birth_date", + "username" + ], + "properties": { + "$schema": { + "title": "Schema", + "description": "Schema to validate user records against.", + "type": "string", + "minLength": 9, + "default": "https://ils.rero.ch/schemas/users/user-v0.0.1.json" + }, + "first_name": { + "title": "First name", + "type": "string", + "minLength": 2, + "widget": { + "formlyConfig": { + "focus": true + } + } + }, + "last_name": { + "title": "Last name", + "type": "string", + "minLength": 2 + }, + "birth_date": { + "title": "Date of birth", + "type": "string", + "format": "date", + "pattern": "^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$", + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "pattern": "Should be in the following format: 2022-12-31 (YYYY-MM-DD)." + } + }, + "templateOptions": { + "placeholder": "Example: 1985-12-29" + } + } + } + }, + "username": { + "title": "Username", + "description": "Login username for the web interface.", + "type": "string", + "pattern": "^[a-zA-Z][a-zA-Z0-9-_]{2}[a-zA-Z0-9-_]*$", + "minLength": 3, + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "patternMessage": "Username must start with a letter, be at least three characters long and only contain alphanumeric characters, dashes and underscores.", + "uniqueUsernameMessage": "This username is already taken." + } + } + } + } + }, + "street": { + "title": "Street", + "description": "Street and number of the address.", + "type": "string", + "minLength": 1 + }, + "postal_code": { + "title": "Postal code", + "type": "string", + "minLength": 1 + }, + "city": { + "title": "City", + "type": "string", + "minLength": 1 + }, + "phone": { + "title": "Phone number", + "description": "Phone number with the international prefix, without spaces.", + "type": "string", + "pattern": "^\\+[0-9]*$", + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "pattern": "Phone number with the international prefix, without spaces, ie +41221234567." + } + }, + "templateOptions": { + "placeholder": "Example: +41791231212" + } + } + } + }, + "keep_history": { + "title": "Keep history", + "description": "If enabled, the loan history is saved for a maximum of six months. It is visible to the user and the library staff.", + "type": "boolean", + "default": false + }, + "email": { + "title": "Email", + "type": "string", + "format": "email", + "pattern": "^.*@.*\\..+$", + "minLength": 6, + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "patternMessage": "The email is not valid.", + "uniqueEmailMessage": "This email is already taken." + } + } + } + } + }, + "password": { + "title": "Password", + "type": "string", + "minLength": 6, + "widget": { + "templateOptions": { + "type": "password" + } + } + } + }, + "widget": { + "formlyConfig": { + "templateOptions": { + "hideLabel": true + } + } + } +} diff --git a/rero_ils/modules/users/views.py b/rero_ils/modules/users/views.py new file mode 100644 index 0000000000..282fb98ea1 --- /dev/null +++ b/rero_ils/modules/users/views.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""Blueprint used for loading templates.""" + +from __future__ import absolute_import, print_function + +import json +from functools import wraps + +from flask import request +from invenio_rest import ContentNegotiatedMethodView + +from .api import User +from ...permissions import login_and_librarian + + +def check_permission(fn): + """Decorate to check permission access. + + The access is allow when the connected user is a librarian. + """ + @wraps(fn) + def is_logged_librarian(*args, **kwargs): + """Decorated view.""" + login_and_librarian() + return fn(*args, **kwargs) + return is_logged_librarian + + +class UsersResource(ContentNegotiatedMethodView): + """User REST resource.""" + + def __init__(self, **kwargs): + """Init.""" + super().__init__( + method_serializers={ + 'GET': { + 'application/json': json.dumps + }, + 'PUT': { + 'application/json': json.dumps + } + }, + serializers_query_aliases={ + 'json': json.dumps + }, + default_method_media_type={ + 'GET': 'application/json', + 'PUT': 'application/json' + }, + default_media_type='application/json', + **kwargs + ) + + @check_permission + def get(self, id): + """Implement the GET.""" + user = User.get_by_id(id) + return user.dumps() + + @check_permission + def put(self, id): + """Implement the PUT.""" + user = User.get_by_id(id) + user = user.update(request.get_json()) + return user.dumps() + +class UsersCreateResource(ContentNegotiatedMethodView): + """User REST resource.""" + + def __init__(self, **kwargs): + """Init.""" + super().__init__( + method_serializers={ + 'GET': { + 'application/json': json.dumps + }, + 'POST': { + 'application/json': json.dumps + } + }, + serializers_query_aliases={ + 'json': json.dumps + }, + default_method_media_type={ + 'GET': 'application/json', + 'POST': 'application/json' + }, + default_media_type='application/json', + **kwargs + ) + + @check_permission + def get(self): + """Get user info for the professionnal view.""" + email_or_username = request.args.get('q', None).strip() + hits = { + 'hits': { + 'hits': [], + 'total': { + 'relation': 'eq', + 'value': 0 + } + } + } + if not email_or_username: + return hits + if email_or_username.startswith('email:'): + user = User.get_by_email( + email_or_username[len('email:'):]) + elif email_or_username.startswith('username:'): + user = User.get_by_username( + email_or_username[len('username:'):]) + else: + user = User.get_by_username_or_email(email_or_username) + if not user: + return hits + data = user.dumps() + hits['hits']['hits'].append(data) + hits['hits']['total']['value'] = 1 + return hits + + @check_permission + def post(self): + """Implement the POST.""" + user = User.create(request.get_json()) + return user.dumps() diff --git a/rero_ils/modules/views.py b/rero_ils/modules/views.py index 4728cc058d..c3e35fbba6 100644 --- a/rero_ils/modules/views.py +++ b/rero_ils/modules/views.py @@ -27,9 +27,7 @@ from flask_babelex import get_domain from flask_login import current_user -from .patrons.api import Patron from .permissions import record_permissions -from ..accounts_views import LoginView from ..permissions import librarian_permission api_blueprint = Blueprint( @@ -95,29 +93,3 @@ def translations(ln): for entry in po: data[entry.msgid] = entry.msgstr or entry.msgid return jsonify(data) - - -@api_blueprint.route('/user_info/') -@check_authentication -def user_info(email_or_username): - """Get user info for the professionnal view.""" - user = LoginView.get_user(email_or_username) - if not user: - return jsonify({}) - patron = Patron.get_patron_by_user(user) - if patron: - # TODO: after the multiple org support return this only if the patron - # is not used in the current organisation - return jsonify({}) - data = { - 'id': user.id, - } - if user.email: - data['email'] = user.email - profile_attrs = ['username', 'first_name', 'last_name', 'city'] - if user.profile: - for attr in profile_attrs: - attr_value = getattr(user.profile, attr) - if attr_value: - data[attr] = attr_value - return jsonify(data) diff --git a/rero_ils/theme/assets/js/reroils/login.js b/rero_ils/theme/assets/js/reroils/login.js index 0b2829a62b..9198b61bb3 100644 --- a/rero_ils/theme/assets/js/reroils/login.js +++ b/rero_ils/theme/assets/js/reroils/login.js @@ -37,7 +37,11 @@ $("#login-user").submit(function (e) { error: function (data) { var response = JSON.parse(data.responseText); var alert = $("#js-alert"); - var msg = $("#js-alert span.msg").html(response.errors[0].message); + var message = response.message; + if(response.errors) { + message = response.errors[0].message; + } + $("#js-alert span.msg").html(message); alert.show(); } }); diff --git a/rero_ils/theme/views.py b/rero_ils/theme/views.py index 0a8ab0241a..47fec0119e 100644 --- a/rero_ils/theme/views.py +++ b/rero_ils/theme/views.py @@ -425,7 +425,8 @@ def prepare_jsonschema(schema): """Json schema prep.""" schema = copy.deepcopy(schema) schema.pop('$schema', None) - schema['required'].remove('pid') + if 'pid' in schema.get('required', []): + schema['required'].remove('pid') return schema diff --git a/setup.py b/setup.py index b9c6eb5a12..8e0631129b 100644 --- a/setup.py +++ b/setup.py @@ -254,6 +254,7 @@ def run(self): 'templates = rero_ils.modules.templates.jsonschemas', 'vendors = rero_ils.modules.vendors.jsonschemas', 'operation_logs = rero_ils.modules.operation_logs.jsonschemas', + 'users = rero_ils.modules.users.jsonschemas', ], 'invenio_search.mappings': [ 'acq_accounts = rero_ils.modules.acq_accounts.mappings', diff --git a/tests/api/patrons/test_patrons_rest.py b/tests/api/patrons/test_patrons_rest.py index b33e30da5b..9e6d5e424a 100644 --- a/tests/api/patrons/test_patrons_rest.py +++ b/tests/api/patrons/test_patrons_rest.py @@ -484,95 +484,6 @@ def test_patrons_dirty_barcode( assert not librarian_martigny.get('patron', {}).get('barcode') -def test_patrons_count(client, patron_sion, - librarian_martigny, - system_librarian_sion): - """Test number of email address.""" - - librarian_email = librarian_martigny.get('email') - url = url_for('api_patrons.number_of_patrons', q=librarian_email) - - res = client.get(url) - assert res.status_code == 401 - - login_user_via_session(client, patron_sion.user) - res = client.get(url) - assert res.status_code == 403 - - login_user_via_session(client, librarian_martigny.user) - # malformed url - res = client.get(url) - assert res.status_code == 400 - - # librarian email - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}"'.format( - email=librarian_martigny.get('email') - )) - res = client.get(url) - assert res.status_code == 200 - assert get_json(res) == dict(hits=dict(total=1)) - - # patron email - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}"'.format( - email=patron_sion.get('email') - )) - res = client.get(url) - assert res.status_code == 200 - assert get_json(res) == dict(hits=dict(total=1)) - - # patron email excluding itself - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}" NOT pid:{pid}'.format( - email=patron_sion.get('email'), - pid=patron_sion.pid - )) - res = client.get(url) - assert get_json(res) == dict(hits=dict(total=0)) - - # patron email excluding itself - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}"'.format( - email='foo@foo.org' - )) - res = client.get(url) - assert get_json(res) == dict(hits=dict(total=0)) - - # librarian email uppercase - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}"'.format( - email=librarian_email.upper() - )) - res = client.get(url) - assert get_json(res) == dict(hits=dict(total=1)) - - # librarian email with spaces - url = url_for('api_patrons.number_of_patrons', - q='email:" {email} "'.format( - email=librarian_email.upper() - )) - res = client.get(url) - assert get_json(res) == dict(hits=dict(total=1)) - - # system librarian email containing a + char - url = url_for('api_patrons.number_of_patrons', - q='email:"{email}"'.format( - email=system_librarian_sion.get('email').upper() - )) - res = client.get(url) - assert get_json(res) == dict(hits=dict(total=1)) - - # patron username - url = url_for('api_patrons.number_of_patrons', - q='username:"{username}"'.format( - username=patron_sion.get('username') - )) - res = client.get(url) - assert res.status_code == 200 - assert get_json(res) == dict(hits=dict(total=1)) - - def test_patrons_circulation_informations( client, patron_sion, librarian_martigny, patron3_martigny_blocked): diff --git a/tests/api/test_users.py b/tests/api/test_users.py deleted file mode 100644 index 554306c8bd..0000000000 --- a/tests/api/test_users.py +++ /dev/null @@ -1,106 +0,0 @@ -# -*- coding: utf-8 -*- -# -# RERO ILS -# Copyright (C) 2021 RERO -# -# 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 . - -"""Test user_info API.""" -from flask import url_for -from invenio_accounts.testutils import login_user_via_session -from utils import get_json - - -def test_user_info( - client, document, user_with_profile, patron_martigny, - system_librarian_martigny): - """Test users API.""" - # failed: no logged user - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username='lroduit@gmail.com' - ) - ) - assert res.status_code == 401 - - # patron should not have the permission - login_user_via_session(client, patron_martigny.user) - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username='lroduit@gmail.com' - ) - ) - assert res.status_code == 403 - - # librarian - login_user_via_session(client, system_librarian_martigny.user) - - # user does not exists - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username='does not exists' - ) - ) - data = get_json(res) - assert data == {} - assert res.status_code == 200 - - # user already linked to a patron account - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username='lroduit@gmail.com' - ) - ) - data = get_json(res) - assert data == {} - assert res.status_code == 200 - - # by email - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username=user_with_profile.email - ) - ) - data = get_json(res) - assert data == { - 'email': user_with_profile.email, - 'city': user_with_profile.profile.city, - 'first_name': user_with_profile.profile.first_name, - 'id': user_with_profile.id, - 'last_name': user_with_profile.profile.last_name, - 'username': user_with_profile.profile.username - } - assert res.status_code == 200 - - # by user - res = client.get( - url_for( - 'api_blueprint.user_info', - email_or_username=user_with_profile.profile.username - ) - ) - data = get_json(res) - assert data == { - 'email': user_with_profile.email, - 'city': user_with_profile.profile.city, - 'first_name': user_with_profile.profile.first_name, - 'id': user_with_profile.id, - 'last_name': user_with_profile.profile.last_name, - 'username': user_with_profile.profile.username - } - assert res.status_code == 200 diff --git a/tests/api/users/test_users_api.py b/tests/api/users/test_users_api.py new file mode 100644 index 0000000000..b9a25aaf64 --- /dev/null +++ b/tests/api/users/test_users_api.py @@ -0,0 +1,183 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""Users Record tests.""" + +from __future__ import absolute_import, print_function + +import json + +from flask import url_for +from invenio_accounts.testutils import login_user_via_session +from utils import get_json, postdata + + +def test_users_api( + client, user_data_tmp, librarian_martigny, json_header): + """Test users REST api for retrieve, create and update.""" + first_name = user_data_tmp.get('first_name') + + # test unauthorized create + user_data_tmp['toto'] = 'toto' + res, data = postdata( + client, + 'api_users.users_list', + user_data_tmp + ) + assert res.status_code == 401 + + login_user_via_session(client, librarian_martigny.user) + + # test invalid create + user_data_tmp['toto'] = 'toto' + res, data = postdata( + client, + 'api_users.users_list', + user_data_tmp + ) + assert res.status_code == 400 + + + user_data_tmp.pop('toto') + user_data_tmp['first_name'] = 1 + res, data = postdata( + client, + 'api_users.users_list', + user_data_tmp + ) + assert res.status_code == 400 + + # test valid create + user_data_tmp['first_name'] = first_name + res, data = postdata( + client, + 'api_users.users_list', + user_data_tmp + ) + assert res.status_code == 200 + user = get_json(res) + assert user['id'] == 2 + assert user['metadata']['first_name'] == user_data_tmp.get('first_name') + + # test get + res = client.get( + url_for( + 'api_users.users_item', + id=2 + ) + ) + assert res.status_code == 200 + user = get_json(res) + assert user['id'] == 2 + assert user['metadata']['first_name'] == user_data_tmp.get('first_name') + + # test valid update + user_data_tmp['first_name'] = 'Johnny' + res = client.put( + url_for( + 'api_users.users_item', + id=2), + data=json.dumps(user_data_tmp), + headers=json_header + ) + assert res.status_code == 200 + user = get_json(res) + assert user['id'] == 2 + assert user['metadata']['first_name'] == 'Johnny' + + # test invalid update + user_data_tmp['first_name'] = 1 + res = client.put( + url_for( + 'api_users.users_item', + id=2), + data=json.dumps(user_data_tmp), + headers=json_header + ) + assert res.status_code == 400 + + +def test_users_search_api(client, librarian_martigny, patron_martigny): + """Test users search REST API.""" + res = client.get( + url_for( + 'api_users.users_list', + q='' + ) + ) + assert res.status_code == 401 + + login_user_via_session(client, librarian_martigny.user) + # empty query => no result + res = client.get( + url_for( + 'api_users.users_list', + q='' + ) + ) + assert res.status_code == 200 + hits = get_json(res) + assert hits['hits']['hits'] == [] + assert hits['hits']['total']['value'] == 0 + + # all by username + res = client.get( + url_for( + 'api_users.users_list', + q=patron_martigny['username'] + ) + ) + assert res.status_code == 200 + hits = get_json(res) + assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id'] + assert hits['hits']['total']['value'] == 1 + + # all by email + res = client.get( + url_for( + 'api_users.users_list', + q=patron_martigny['email'] + ) + ) + assert res.status_code == 200 + hits = get_json(res) + assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id'] + assert hits['hits']['total']['value'] == 1 + + # by username + res = client.get( + url_for( + 'api_users.users_list', + q='username:' + patron_martigny['username'] + ) + ) + assert res.status_code == 200 + hits = get_json(res) + assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id'] + assert hits['hits']['total']['value'] == 1 + + # by email + res = client.get( + url_for( + 'api_users.users_list', + q='email:' + patron_martigny['email'] + ) + ) + assert res.status_code == 200 + hits = get_json(res) + assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id'] + assert hits['hits']['total']['value'] == 1 diff --git a/tests/data/data.json b/tests/data/data.json index 5f47e15485..3a0c1c099c 100644 --- a/tests/data/data.json +++ b/tests/data/data.json @@ -3523,5 +3523,17 @@ "operation": "update", "user_name": "updated_user", "date": "2021-01-21T09:51:52.879533+00:00" + }, + "user1": { + "$schema": "https://ils.rero.ch/schemas/users/user-v0.0.1.json", + "first_name": "John", + "last_name": "Smith", + "birth_date": "1974-03-21", + "username": "smith_john", + "street": "1600 Donald Trump Avenue NW", + "postal_code": "20500", + "city": "Washington, D.C.", + "phone": "+012024561414", + "keep_history": true } } diff --git a/tests/fixtures/circulation.py b/tests/fixtures/circulation.py index 2918c483ce..4f1f37672d 100644 --- a/tests/fixtures/circulation.py +++ b/tests/fixtures/circulation.py @@ -1317,3 +1317,10 @@ def ill_request_sion(app, loc_public_sion, patron_sion, reindex=True) flush_index(ILLRequestsSearch.Meta.index) return illr + + +# ------------ users ---------- +@pytest.fixture(scope="module") +def user_data_tmp(data): + """Load user data.""" + return deepcopy(data.get('user1')) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 0f99a3e67a..11ed13b578 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -207,6 +207,14 @@ def item_schema(monkeypatch): ) return get_schema(monkeypatch, schema_in_bytes) +@pytest.fixture() +def user_schema(monkeypatch): + """User Jsonschema for records.""" + schema_in_bytes = resource_string( + 'rero_ils.modules.users.jsonschemas', + 'users/user-v0.0.1.json' + ) + return get_schema(monkeypatch, schema_in_bytes) @pytest.fixture() def holding_schema(monkeypatch): diff --git a/tests/unit/test_users_jsonschema.py b/tests/unit/test_users_jsonschema.py new file mode 100644 index 0000000000..87f068b3ec --- /dev/null +++ b/tests/unit/test_users_jsonschema.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""User JSON schema tests.""" + +import pytest +from jsonschema import validate +from jsonschema.exceptions import ValidationError + + +def test_required(user_schema, user_data_tmp): + """Test required for user jsonschemas.""" + validate(user_data_tmp, user_schema) + + with pytest.raises(ValidationError): + validate({}, user_schema) + + +def test_user_all_jsonschema_keys_values( + user_schema, user_data_tmp): + """Test all keys and values for user jsonschema.""" + record = user_data_tmp + validate(record, user_schema) + validator = [ + {'key': 'first_name', 'value': 25}, + {'key': 'last_name', 'value': 25}, + {'key': 'birth_date', 'value': 25}, + {'key': 'username', 'value': 25}, + {'key': 'street', 'value': 25}, + {'key': 'postal_code', 'value': 25}, + {'key': 'city', 'value': 25}, + {'key': 'phone', 'value': 25}, + {'key': 'keep_history', 'value': 25}, + {'key': 'user_id', 'value': '25'} + ] + for element in validator: + with pytest.raises(ValidationError): + record[element['key']] = element['value'] + validate(record, user_schema) From fb8f162259fa2601f04c1fbd5862cecd8f91e1d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Thu, 11 Feb 2021 16:17:38 +0100 Subject: [PATCH 2/2] users: add new fields, move fields from the patron MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replaces the phone field by home, business, mobile, other phone fields in the user data model. * Adds country and gender fields in the user data model. * Adds 3 invenio-userprofiles configuration to specify the list of countries, the default country and the read only fields. * Makes the barcode repetitive. * Removes all user fields from the patron data model. * Adds the second address, the local code, the source fields in the patron data model. * Closes #1724, #1634, #1615, #1490, #1467, #1318, #1384, #1670. Co-Authored-by: Johnny Mariéthoz --- data/templates.json | 6 +- data/users.json | 214 +++++++++++------- poetry.lock | 7 +- pyproject.toml | 2 +- rero_ils/config.py | 33 ++- rero_ils/dojson/utils.py | 2 +- rero_ils/modules/items/api/circulation.py | 2 +- rero_ils/modules/items/api/record.py | 4 +- rero_ils/modules/items/cli.py | 4 +- rero_ils/modules/loans/api.py | 2 +- rero_ils/modules/loans/cli.py | 5 +- rero_ils/modules/patrons/api.py | 117 +++------- rero_ils/modules/patrons/cli.py | 2 +- .../jsonschemas/patrons/patron-v0.0.1.json | 203 ++++++++--------- rero_ils/modules/patrons/listener.py | 14 +- .../mappings/v7/patrons/patron-v0.0.1.json | 35 ++- rero_ils/modules/patrons/serializers.py | 46 ++++ rero_ils/modules/patrons/tasks.py | 2 +- .../rero_ils/_patron_profile_head.html | 2 +- .../rero_ils/_patron_profile_personal.html | 6 +- .../templates/rero_ils/patron_profile.html | 3 +- rero_ils/modules/selfcheck/api.py | 2 +- rero_ils/modules/users/api.py | 49 ++-- .../users/jsonschemas/users/user-v0.0.1.json | 86 ++++++- rero_ils/modules/utils.py | 23 +- .../mappings/v7/vendors/vendor-v0.0.1.json | 4 +- .../templates/rero_ils/page_settings.html | 6 +- rero_ils/theme/views.py | 9 + rero_ils/utils.py | 19 +- tests/api/documents/test_marcxml_rest_api.py | 2 +- tests/api/items/test_items_rest.py | 4 +- tests/api/loans/test_loans_rest.py | 2 +- tests/api/patrons/test_patrons_blocked.py | 4 +- tests/api/patrons/test_patrons_rest.py | 39 ++-- tests/api/patrons/test_patrons_views.py | 2 +- tests/api/selfcheck/test_selfcheck.py | 23 +- tests/api/test_availability.py | 16 +- tests/api/test_permissions_librarian.py | 8 +- tests/api/test_permissions_patron.py | 4 +- tests/api/test_permissions_sys_librarian.py | 4 +- tests/api/test_search.py | 2 +- tests/api/test_user_authentication.py | 16 +- .../{test_users_api.py => test_users_rest.py} | 10 +- tests/data/data.json | 46 ++-- tests/e2e/cypress/cypress/fixtures/users.json | 6 +- tests/e2e/cypress/cypress/support/api.js | 6 +- tests/fixtures/sip2.py | 1 + tests/ui/circulation/test_actions_checkout.py | 4 +- tests/ui/circulation/test_inhouse_cipo.py | 2 +- tests/ui/patrons/test_patrons_api.py | 21 +- tests/unit/conftest.py | 4 +- tests/unit/test_cli.py | 2 +- tests/unit/test_patrons_jsonschema.py | 18 +- tests/unit/test_users_jsonschema.py | 7 +- tests/utils.py | 4 +- 55 files changed, 704 insertions(+), 462 deletions(-) create mode 100644 rero_ils/modules/patrons/serializers.py rename tests/api/users/{test_users_api.py => test_users_rest.py} (94%) diff --git a/data/templates.json b/data/templates.json index 242580d636..76296ae157 100644 --- a/data/templates.json +++ b/data/templates.json @@ -282,7 +282,7 @@ "patron_type": { "$ref": "https://ils.rero.ch/api/patron_types/1" }, - "phone": "", + "home_phone": "", "postal_code": "", "roles": [ "patron" @@ -861,7 +861,7 @@ "patron_type": { "$ref": "https://ils.rero.ch/api/patron_types/4" }, - "phone": "", + "home_phone": "", "postal_code": "", "roles": [ "patron" @@ -1440,7 +1440,7 @@ "patron_type": { "$ref": "https://ils.rero.ch/api/patron_types/5" }, - "phone": "", + "home_phone": "", "postal_code": "", "roles": [ "patron" diff --git a/data/users.json b/data/users.json index 523b97c49d..8367dd24a6 100644 --- a/data/users.json +++ b/data/users.json @@ -16,7 +16,7 @@ "$ref": "https://ils.rero.ch/api/libraries/4" } ], - "phone": "+39324993597", + "home_phone": "+39324993597", "postal_code": "11100", "street": "Viale Rue Gran Paradiso, 44", "notes": [ @@ -45,7 +45,7 @@ "$ref": "https://ils.rero.ch/api/libraries/4" } ], - "phone": "+41261234567", + "home_phone": "+41261234567", "postal_code": "1700", "street": "Avenue de la Gare, 80" }, @@ -65,7 +65,7 @@ "$ref": "https://ils.rero.ch/api/libraries/3" } ], - "phone": "+41911234567", + "home_phone": "+41911234567", "postal_code": "6900", "street": "Viale Carlo Cattaneo, 52" }, @@ -80,14 +80,16 @@ "patron" ], "last_name": "Casalini", - "phone": "+39324993585", + "home_phone": "+39324993585", "postal_code": "11100", "street": "Via Croix Noire 3", "patron": { "expiration_date": "2023-10-07", "blocked": false, "keep_history": false, - "barcode": "2050124311", + "barcode": [ + "2050124311" + ], "type": { "$ref": "https://ils.rero.ch/api/patron_types/1" }, @@ -120,7 +122,9 @@ "street": "Panoramica Collinare, 47", "patron": { "expiration_date": "2023-10-07", - "barcode": "2030124287", + "barcode": [ + "2030124287" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/2" @@ -145,7 +149,7 @@ "$ref": "https://ils.rero.ch/api/libraries/4" } ], - "phone": "+39244857275", + "home_phone": "+39244857275", "postal_code": "11010", "street": "Frazione Plan" }, @@ -164,7 +168,9 @@ "street": "Rue du Nord 7", "patron": { "expiration_date": "2023-10-07", - "barcode": "reroils1", + "barcode": [ + "reroils1" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/3" @@ -184,12 +190,14 @@ "patron" ], "last_name": "John", - "phone": "+1408492280015", + "home_phone": "+1408492280015", "postal_code": "95054", "street": "520 Scott Blvd", "patron": { "expiration_date": "2023-10-07", - "barcode": "2000000001", + "barcode": [ + "2000000001" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/3" @@ -209,12 +217,14 @@ "patron" ], "last_name": "Premand", - "phone": "+41795762233", + "home_phone": "+41795762233", "postal_code": "1892", "street": "Route du Village 6", "patron": { "expiration_date": "2023-10-07", - "barcode": "10000001", + "barcode": [ + "10000001" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/1" @@ -234,12 +244,14 @@ "patron" ], "last_name": "Broglio", - "phone": "+41918264239", + "home_phone": "+41918264239", "postal_code": "6500", "street": "Piazza Collegiata 12", "patron": { "expiration_date": "2023-10-07", - "barcode": "2050124312", + "barcode": [ + "2050124312" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/1" @@ -259,12 +271,14 @@ "patron" ], "last_name": "Carron", - "phone": "+41792001020", + "home_phone": "+41792001020", "postal_code": "1920", "street": "Gare 45", "patron": { "expiration_date": "2023-10-07", - "barcode": "kad001", + "barcode": [ + "kad001" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/1" @@ -284,12 +298,14 @@ "patron" ], "last_name": "Rard", - "phone": "+41792500512", + "home_phone": "+41792500512", "postal_code": "1926", "street": "Vignettes 25", "patron": { "expiration_date": "2023-10-07", - "barcode": "kad002", + "barcode": [ + "kad002" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/1" @@ -315,7 +331,7 @@ "$ref": "https://ils.rero.ch/api/libraries/5" } ], - "phone": "+01411234567", + "home_phone": "+01411234567", "postal_code": "0000", "street": "High Street" }, @@ -334,7 +350,9 @@ "street": "Magic Street 3", "patron": { "expiration_date": "2023-10-07", - "barcode": "3050124311", + "barcode": [ + "3050124311" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/4" @@ -358,7 +376,9 @@ "street": "Diagon Alley 72", "patron": { "expiration_date": "2023-10-07", - "barcode": "3050124312", + "barcode": [ + "3050124312" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/4" @@ -384,7 +404,7 @@ "$ref": "https://ils.rero.ch/api/libraries/6" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "7878", "street": "Imagination Street 48" }, @@ -404,7 +424,7 @@ "$ref": "https://ils.rero.ch/api/libraries/6" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -424,7 +444,7 @@ "$ref": "https://ils.rero.ch/api/libraries/7" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -444,7 +464,7 @@ "$ref": "https://ils.rero.ch/api/libraries/8" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -464,7 +484,7 @@ "$ref": "https://ils.rero.ch/api/libraries/9" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -484,7 +504,7 @@ "$ref": "https://ils.rero.ch/api/libraries/10" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -504,7 +524,7 @@ "$ref": "https://ils.rero.ch/api/libraries/11" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -524,7 +544,7 @@ "$ref": "https://ils.rero.ch/api/libraries/12" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -544,7 +564,7 @@ "$ref": "https://ils.rero.ch/api/libraries/13" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -564,7 +584,7 @@ "$ref": "https://ils.rero.ch/api/libraries/14" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -584,7 +604,7 @@ "$ref": "https://ils.rero.ch/api/libraries/15" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -604,7 +624,7 @@ "$ref": "https://ils.rero.ch/api/libraries/16" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -624,7 +644,7 @@ "$ref": "https://ils.rero.ch/api/libraries/17" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -644,7 +664,7 @@ "$ref": "https://ils.rero.ch/api/libraries/18" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -664,7 +684,7 @@ "$ref": "https://ils.rero.ch/api/libraries/19" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -684,7 +704,7 @@ "$ref": "https://ils.rero.ch/api/libraries/20" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -704,7 +724,7 @@ "$ref": "https://ils.rero.ch/api/libraries/21" } ], - "phone": "+765554433", + "home_phone": "+765554433", "postal_code": "no indication", "street": "no indication" }, @@ -719,12 +739,14 @@ "patron" ], "last_name": "Rh\u00e9aume", - "phone": "+41316126321", + "home_phone": "+41316126321", "postal_code": "1292", "street": "Faubourg du Lac 6", "patron": { "expiration_date": "2023-10-07", - "barcode": "920001", + "barcode": [ + "920001" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -744,12 +766,14 @@ "patron" ], "last_name": "Rocher", - "phone": "+41913843044", + "home_phone": "+41913843044", "postal_code": "1489", "street": "Rue du Tr\u00e9sor 9", "patron": { "expiration_date": "2023-10-07", - "barcode": "920002", + "barcode": [ + "920002" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -769,12 +793,14 @@ "patron" ], "last_name": "Chalifour", - "phone": "+41628609930", + "home_phone": "+41628609930", "postal_code": "2953", "street": "Rue de l'Industrie 46", "patron": { "expiration_date": "2023-10-07", - "barcode": "920003", + "barcode": [ + "920003" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -794,12 +820,14 @@ "patron" ], "last_name": "Lamarre", - "phone": "+41613956062", + "home_phone": "+41613956062", "postal_code": "2000", "street": "Avenue Max-Huber 2", "patron": { "expiration_date": "2023-10-07", - "barcode": "920004", + "barcode": [ + "920004" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -819,12 +847,14 @@ "patron" ], "last_name": "Boileau", - "phone": "+41815333591", + "home_phone": "+41815333591", "postal_code": "3960", "street": "Rue de Montsalvens 3", "patron": { "expiration_date": "2023-10-07", - "barcode": "920005", + "barcode": [ + "920005" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -844,12 +874,14 @@ "patron" ], "last_name": "Doiron", - "phone": "+41316126321", + "home_phone": "+41316126321", "postal_code": "1630", "street": "Rue du Vieux-Pont 3", "patron": { "expiration_date": "2023-10-07", - "barcode": "920006", + "barcode": [ + "920006" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -869,12 +901,14 @@ "patron" ], "last_name": "Beich", - "phone": "+41562747247", + "home_phone": "+41562747247", "postal_code": "9604", "street": "Seefeldstrasse 37", "patron": { "expiration_date": "2023-10-07", - "barcode": "920007", + "barcode": [ + "920007" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -894,12 +928,14 @@ "patron" ], "last_name": "Fischer", - "phone": "+41449166744", + "home_phone": "+41449166744", "postal_code": "8620", "street": "In Stierwisen 98", "patron": { "expiration_date": "2023-10-07", - "barcode": "920008", + "barcode": [ + "920008" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -919,12 +955,14 @@ "patron" ], "last_name": "Freeh", - "phone": "+41523734083", + "home_phone": "+41523734083", "postal_code": "8155", "street": "Im Wingert 117", "patron": { "expiration_date": "2023-10-07", - "barcode": "920009", + "barcode": [ + "920009" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -944,12 +982,14 @@ "patron" ], "last_name": "Schuhmacher", - "phone": "+41318033434", + "home_phone": "+41318033434", "postal_code": "9525", "street": "Werkstrasse 136", "patron": { "expiration_date": "2023-10-07", - "barcode": "920010", + "barcode": [ + "920010" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -969,12 +1009,14 @@ "patron" ], "last_name": "Trommler", - "phone": "+41278023434", + "home_phone": "+41278023434", "postal_code": "3605", "street": "L\u00fctzelfl\u00fchstrasse 120", "patron": { "expiration_date": "2023-10-07", - "barcode": "920011", + "barcode": [ + "920011" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -994,12 +1036,14 @@ "patron" ], "last_name": "Ziegler", - "phone": "+41222747247", + "home_phone": "+41222747247", "postal_code": "5637", "street": "\u00dcerklisweg 55", "patron": { "expiration_date": "2023-10-07", - "barcode": "920012", + "barcode": [ + "920012" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1019,12 +1063,14 @@ "patron" ], "last_name": "Marcelo", - "phone": "+41322747447", + "home_phone": "+41322747447", "postal_code": "6675", "street": "Via Verbano 109", "patron": { "expiration_date": "2023-10-07", - "barcode": "920013", + "barcode": [ + "920013" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1043,12 +1089,14 @@ "patron" ], "last_name": "Folliero", - "phone": "+41328619930", + "home_phone": "+41328619930", "postal_code": "7106", "street": "Quadra 93", "patron": { "expiration_date": "2023-10-07", - "barcode": "920014", + "barcode": [ + "920014" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1068,12 +1116,14 @@ "patron" ], "last_name": "Bellucci", - "phone": "+41313966062", + "home_phone": "+41313966062", "postal_code": "6987", "street": "Via Camischolas sura 63", "patron": { "expiration_date": "2023-10-07", - "barcode": "920015", + "barcode": [ + "920015" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1093,12 +1143,14 @@ "patron" ], "last_name": "Colombo", - "phone": "+41274865229", + "home_phone": "+41274865229", "postal_code": "6500", "street": "Lungolago 118", "patron": { "expiration_date": "2023-10-07", - "barcode": "920016", + "barcode": [ + "920016" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1118,12 +1170,14 @@ "patron" ], "last_name": "de Baskerville", - "phone": "+41324993585", + "home_phone": "+41324993585", "postal_code": "6073", "street": "Ranftweg 1", "patron": { "expiration_date": "2023-10-07", - "barcode": "999999", + "barcode": [ + "999999" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/5" @@ -1150,7 +1204,7 @@ } ], "password": "123456", - "phone": "+39324993599", + "home_phone": "+39324993599", "postal_code": "11101", "street": "Central Park" }, @@ -1171,7 +1225,7 @@ } ], "password": "123456", - "phone": "+39324993596", + "home_phone": "+39324993596", "postal_code": "11102", "street": "Illogical street, 44" }, @@ -1187,12 +1241,14 @@ ], "last_name": "Kirk", "password": "123456", - "phone": "+41324883598", + "home_phone": "+41324883598", "postal_code": "4242", "street": "Galaxy street", "patron": { "expiration_date": "2023-10-07", - "barcode": "cypress-1", + "barcode": [ + "cypress-1" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/7" @@ -1213,12 +1269,14 @@ ], "last_name": "Uhura", "password": "123456", - "phone": "+41324883123", + "home_phone": "+41324883123", "postal_code": "4242", "street": "Milky Way street", "patron": { "expiration_date": "2023-10-07", - "barcode": "cypress-2", + "barcode": [ + "cypress-2" + ], "keep_history": false, "type": { "$ref": "https://ils.rero.ch/api/patron_types/7" diff --git a/poetry.lock b/poetry.lock index 20c40a550f..3dff2fe92a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1038,7 +1038,7 @@ optional = true version = ">=1.2.5,<1.3.0" [package.dependencies.invenio-db] -extras = ["postgresql", "versioning"] +extras = ["versioning", "postgresql"] optional = true version = ">=1.0.8,<1.1.0" @@ -1845,10 +1845,9 @@ sqlite = ["invenio-db (>=1.0.5)"] tests = ["pytest-invenio (>=1.4.0)"] [package.source] -reference = "d7fafc33068df24a7a9ef34836587f15c45ca96d" +reference = "49d77006fc4485f704dab6270fcd7115f6291eb7" type = "git" url = "https://github.com/rero/invenio-userprofiles.git" - [[package]] category = "main" description = "IPython: Productive Interactive Computing" @@ -3334,7 +3333,7 @@ testing = ["pytest (>=3.5,<3.7.3 || >3.7.3)", "pytest-checkdocs (>=1.2.3)", "pyt sip2 = ["invenio-sip2"] [metadata] -content-hash = "6dcb39a59a6b2fc23bacdab85f297565cfff29a3e275da7acdd081fe94034e19" +content-hash = "c5acff9e5250e3265fad4a23093b8ae36014509dc48486c56549af08e12ac508" lock-version = "1.0" python-versions = ">= 3.6, <3.8" diff --git a/pyproject.toml b/pyproject.toml index e2285308a8..b925e824a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,7 @@ redisbeat = "*" jsonpickle = ">=1.4.1" ciso8601 = "*" # TODO: to be removed when the thumbnail will be refactored -invenio-userprofiles = {git = "https://github.com/rero/invenio-userprofiles.git", tag = "invenio-3.4"} +invenio-userprofiles = {git = "https://github.com/rero/invenio-userprofiles.git", rev = "v1.2.1-rero1.0"} ## Additionnal constraints on python modules flask-wiki = {git = "https://github.com/rero/flask-wiki.git"} diff --git a/rero_ils/config.py b/rero_ils/config.py index 5dec88095b..cfb6a135a5 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -99,6 +99,8 @@ from .modules.permissions import record_permission_factory from .modules.templates.api import Template from .modules.templates.permissions import TemplatePermission +from .modules.users.api import get_profile_countries, \ + get_readonly_profile_fields from .modules.vendors.api import Vendor from .modules.vendors.permissions import VendorPermission from .permissions import librarian_delete_permission_factory, \ @@ -228,6 +230,9 @@ def _(x): ACCOUNTS_USERINFO_HEADERS = False # Disable User Profiles USERPROFILES = True +USERPROFILES_COUNTRIES = get_profile_countries +USERPROFILES_DEFAULT_COUNTRY = 'sz' +USERPROFILES_READONLY_FIELDS = get_readonly_profile_fields # Custom login view ACCOUNTS_REST_AUTH_VIEWS = { @@ -802,15 +807,19 @@ def _(x): }, record_serializers_aliases={ 'json': 'application/json', + 'rero+json': 'application/rero+json', }, search_serializers={ 'application/json': ( 'rero_ils.modules.serializers:json_v1_search' + ), + 'application/rero+json': ( + 'rero_ils.modules.patrons.serializers:json_patron_search' ) }, list_route='/patrons/', record_loaders={ - 'application/json': lambda: Patron(request.get_json()), + 'application/json': lambda: Patron.load(request.get_json()), }, record_class='rero_ils.modules.patrons.api:Patron', item_route=('/patrons/. """API for manipulating patrons.""" +from copy import deepcopy from datetime import datetime from functools import partial @@ -24,7 +25,6 @@ from flask_babelex import gettext as _ from flask_login import current_user from invenio_circulation.proxies import current_circulation -from invenio_db import db from werkzeug.local import LocalProxy from .models import PatronIdentifier, PatronMetadata @@ -36,8 +36,9 @@ from ..organisations.api import Organisation from ..patron_transactions.api import PatronTransaction from ..providers import Provider +from ..users.api import User from ..utils import extracted_data_from_ref, get_patron_from_arguments, \ - get_ref_for_pid, trim_barcode_for_record + get_ref_for_pid, trim_patron_barcode_for_record from ...utils import create_user_from_data _datastore = LocalProxy(lambda: current_app.extensions['security'].datastore) @@ -106,12 +107,6 @@ class Patron(IlsRecord): provider = PatronProvider model_cls = PatronMetadata - # field list to be in sync - profile_fields = [ - 'first_name', 'last_name', 'street', 'postal_code', - 'city', 'birth_date', 'username', 'phone', 'keep_history' - ] - available_roles = [ROLE_SYSTEM_LIBRARIAN, ROLE_LIBRARIAN, ROLE_PATRON] def _validate(self, **kwargs): @@ -200,30 +195,16 @@ def create(cls, data, id_=None, delete_pid=False, :param email_notification - send a reset password link to the user """ # remove spaces - data = trim_barcode_for_record(data=data) - # synchronize the rero id user profile data - user = cls._get_user_by_user_id(data.get('user_id')) - data = cls.merge_data_from_profile(data, user.profile) - try: - record = super().create( + data = trim_patron_barcode_for_record(data=data) + record = super().create( data, id_, delete_pid, dbcommit, reindex, **kwargs) - record._update_roles() - except Exception as err: - db.session.rollback() - raise err - # TODO: send reset password instruction when a librarian create a user + record._update_roles() return record def update(self, data, dbcommit=False, reindex=False): """Update data for record.""" # remove spaces - data = trim_barcode_for_record(data=data) - data = dict(self, **data) - - # synchronize the rero id user profile data - user = self._get_user_by_user_id(data.get('user_id')) - data = self.merge_data_from_profile(data, user.profile) - + data = trim_patron_barcode_for_record(data=data) super().update(data, dbcommit, reindex) self._update_roles() return self @@ -235,54 +216,29 @@ def delete(self, force=False, delindex=False): return self @classmethod - def merge_data_from_profile(cls, data, profile): - """Get the profile informations and inject it. - - TODO: move this to the indexing time. - """ - # retrieve the user - for field in cls.profile_fields: - # date field requires conversion - if field == 'birth_date': - data[field] = getattr( - profile, field).strftime('%Y-%m-%d') - elif field == 'keep_history': - if 'patron' in data.get('roles', []): - new_keep_history = getattr(profile, field) - data.setdefault('patron', {})['keep_history'] = new_keep_history - else: - value = getattr(profile, field) - if value not in [None, '']: - data[field] = value - # update the email - if profile.user.email != data.get('email'): - # the email is not defined or removed in the user profile - if not profile.user.email: - try: - del data['email'] - except KeyError: - pass - else: - # the email has been updated in the user profile - data['email'] = profile.user.email - return data + def load(cls, data): + """Load the data and remove the user data.""" + return cls(cls.removeUserData(data)) @classmethod - def update_from_profile(cls, data, profile): - """Update the current record with the user profile data. + def removeUserData(cls, data): + """Remove the user data.""" + data = deepcopy(data) + profile_fields = User.profile_fields + ['username', 'email'] + for field in profile_fields: + try: + del data[field] + except KeyError: + pass + return data - :param profile - the rero user profile - """ - # retrieve the user - patron = Patron.get_patron_by_user(profile.user) - if patron: - cls.merge_data_from_profile(dict(patron), profile) - super().update(dict(patron), True, True) - # TODO: do it at the profile changes - # anonymize user loans if keep_history is changed - # if old_keep_history and not new_keep_history: - # from ..loans.api import anonymize_loans - # anonymize_loans(patron_pid=patron.pid, dbcommit=True, reindex=True) + def dumps(self, **kwargs): + """Return pure Python dictionary with record metadata.""" + dump = super().dumps(**kwargs) + user = User.get_by_id(self['user_id']) + user_info = user.dumpsMetadata() + dump.update(user_info) + return dump @classmethod def _get_user_by_user_id(cls, user_id): @@ -476,20 +432,6 @@ def remove_role(self, role_name): _datastore.remove_role_from_user(self.user, role) _datastore.commit() - @property - def initial(self): - """Return the initials of the patron first name.""" - initial = '' - firsts = self['first_name'].split(' ') - for first in firsts: - initial += first[0] - lasts = self['last_name'].split(' ') - for last in lasts: - if last[0].isupper(): - initial += last[0] - - return initial - @property def patron(self): """Patron property shorcut.""" @@ -498,9 +440,10 @@ def patron(self): @property def formatted_name(self): """Return the best possible human readable patron name.""" + profile = self.user.profile name_parts = [ - self.get('last_name', '').strip(), - self.get('first_name', '').strip() + profile.last_name.strip(), + profile.first_name.strip() ] name_parts = [part for part in name_parts if part] # remove empty part return ', '.join(name_parts) diff --git a/rero_ils/modules/patrons/cli.py b/rero_ils/modules/patrons/cli.py index 7166b7196c..11aaa8694d 100644 --- a/rero_ils/modules/patrons/cli.py +++ b/rero_ils/modules/patrons/cli.py @@ -84,7 +84,7 @@ def import_users(infile, append, verbose, password, lazy, dont_stop_on_error, ), fg='yellow') if password: patron_data.pop('password', None) - # do nothing if the patron alredy exists + # do nothing if the patron already exists patron = Patron.get_patron_by_username(username) if patron: click.secho('{count: <8} Patron already exist: {username}'.format( 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 011bb09a45..71759c4533 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 @@ -6,10 +6,13 @@ "additionalProperties": false, "propertiesOrder": [ "user_id", + "second_address", "roles", "patron", "libraries", - "notes" + "notes", + "source", + "local_code" ], "required": [ "$schema", @@ -29,8 +32,20 @@ "title": "Patron ID", "type": "string" }, + "source": { + "title": "Source", + "description": "Source if the record has been loaded in a batch.", + "type": "string", + "minLength": 2 + }, + "local_code": { + "title": "Local code", + "description": "Code used to classify users, for instance for statistics.", + "type": "string", + "minLength": 2 + }, "user_id": { - "title": "Personal Informations", + "title": "Personal informations", "description": "", "type": "number", "form": { @@ -42,106 +57,68 @@ } } }, - "first_name": { - "title": "First name", - "type": "string", - "minLength": 2, - "form": { - "focus": true - } - }, - "last_name": { - "title": "Last name", - "type": "string", - "minLength": 2 - }, - "birth_date": { - "title": "Date of birth", - "type": "string", - "format": "date", - "pattern": "^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$", - "form": { - "validation": { - "messages": { - "patternMessage": "Should be in the following format: 2022-12-31 (YYYY-MM-DD)." + "second_address": { + "title": "Secound address", + "type": "object", + "additionalProperties": false, + "propertiesOrder": [ + "street", + "postal_code", + "city", + "country" + ], + "properties": { + "street": { + "title": "Street", + "description": "Street and number of the address.", + "type": "string", + "minLength": 1, + "form": { + "templateOptions": { + "itemCssClass": "col-lg-12" + } } }, - "placeholder": "Example: 1985-12-29" - } - }, - "email": { - "title": "Email", - "type": "string", - "format": "email", - "pattern": "^.*@.*\\..+$", - "minLength": 6, - "form": { - "expressionProperties": { - "templateOptions.required": "field.parent.model.roles.some(v => (v === 'librarian' || v === 'system_librarian')) || (field.parent.model.patron.communication_channel === 'email' && !field.parent.model.patron.additional_communication_email)" - }, - "validation": { - "validators": { - "valueAlreadyExists": { - "term": "email", - "remoteRecordType": "patrons/count" + "postal_code": { + "title": "Postal code", + "type": "string", + "minLength": 1, + "form": { + "templateOptions": { + "itemCssClass": "col-lg-6" } - }, - "messages": { - "patternMessage": "The email is not valid.", - "alreadyTakenMessage": "This email is already taken." } - } - } - }, - "username": { - "title": "Username", - "description": "Login username for the web interface.", - "type": "string", - "pattern": "^[a-zA-Z][a-zA-Z0-9-_]{2}[a-zA-Z0-9-_]*$", - "minLength": 3, - "form": { - "validation": { - "validators": { - "valueAlreadyExists": { - "term": "username", - "remoteRecordType": "patrons/count" + }, + "city": { + "title": "City", + "type": "string", + "minLength": 1, + "form": { + "templateOptions": { + "itemCssClass": "col-lg-6" } - }, - "messages": { - "patternMessage": "Username must start with a letter, be at least three characters long and only contain alphanumeric characters, dashes and underscores.", - "alreadyTakenMessage": "This username is already taken." } + }, + "country": { + "allOf": [ + { + "$ref": "https://ils.rero.ch/schemas/common/countries-v0.0.1.json#/country" + }, + { + "form": { + "hideExpression": "!(field.parent.model && field.parent.model.city)" + } + } + ] } - } - }, - "street": { - "title": "Street", - "description": "Street and number of the address.", - "type": "string", - "minLength": 1 - }, - "postal_code": { - "title": "Postal code", - "type": "string", - "minLength": 1 - }, - "city": { - "title": "City", - "type": "string", - "minLength": 1 - }, - "phone": { - "title": "Phone number", - "description": "Phone number with the international prefix, without spaces.", - "type": "string", - "pattern": "^\\+[0-9]*$", + }, "form": { - "validation": { - "messages": { - "patternMessage": "Phone number with the international prefix, without spaces, ie +41221234567." - } - }, - "placeholder": "Example: +41791231212" + "templateOptions": { + "wrappers": [ + "card" + ], + "containerCssClass": "row" + } } }, "patron": { @@ -163,7 +140,6 @@ "communication_language", "expiration_date", "libraries", - "keep_history", "blocked", "blocked_note" ], @@ -188,16 +164,25 @@ } }, "barcode": { - "title": "Patron's barcode or card number", - "type": "string", - "minLength": 6, - "form": { - "validation": { - "validators": { - "valueAlreadyExists": {} - }, - "messages": { - "alreadyTakenMessage": "The barcode is already taken." + "title": "Patron's barcodes or cards number", + "type": "array", + "minItems": 1, + "maxItems": 2, + "uniqueItems": true, + "items": { + "title": "Patron's barcode or card number", + "type": "string", + "minLength": 6, + "form": { + "validation": { + "validators": { + "valueAlreadyExists": { + "term": "barcode" + } + }, + "messages": { + "alreadyTakenMessage": "The barcode is already taken." + } } } } @@ -469,13 +454,19 @@ } }, "form": { - "fieldMap": "roles" + "fieldMap": "roles", + "templateOptions": { + "wrappers": [ + "card" + ] + } } }, "notes": { "title": "Notes", "description": "The public note is visible for the patron in his/her account.", "type": "array", + "minItems": 0, "items": { "type": "object", "additionalProperties": false, diff --git a/rero_ils/modules/patrons/listener.py b/rero_ils/modules/patrons/listener.py index 58f02dcb03..b24d4a999d 100644 --- a/rero_ils/modules/patrons/listener.py +++ b/rero_ils/modules/patrons/listener.py @@ -43,7 +43,6 @@ def enrich_patron_data(sender, json=None, record=None, index=None, 'pid': org_pid } - def create_subscription_patron_transaction(sender, record=None, **kwargs): """This method check the patron to know if a subscription is requested. @@ -76,4 +75,15 @@ def update_from_profile(sender, profile=None, **kwargs): :param profile - the rero user profile """ - Patron.update_from_profile(profile) + patron = Patron.get_patron_by_user(profile.user) + if patron: + old_keep_history = patron.get('patron', {}).get('keep_history') + patron.reindex() + from ..loans.api import anonymize_loans + new_keep_history = profile.keep_history + if old_keep_history and not new_keep_history: + anonymize_loans( + patron_data=patron, + patron_pid=patron.get('pid'), + dbcommit=True, + reindex=True) diff --git a/rero_ils/modules/patrons/mappings/v7/patrons/patron-v0.0.1.json b/rero_ils/modules/patrons/mappings/v7/patrons/patron-v0.0.1.json index 2faacd1860..8e03ade1a4 100644 --- a/rero_ils/modules/patrons/mappings/v7/patrons/patron-v0.0.1.json +++ b/rero_ils/modules/patrons/mappings/v7/patrons/patron-v0.0.1.json @@ -61,7 +61,40 @@ "city": { "type": "text" }, - "phone": { + "facet_city": { + "type": "keyword", + "copy_to": "facet_city" + }, + "country": { + "type": "text" + }, + "second_address": { + "properties": { + "street": { + "type": "text" + }, + "postal_code": { + "type": "keyword" + }, + "city": { + "type": "text", + "copy_to": "facet_city" + }, + "country": { + "type": "text" + } + } + }, + "home_phone": { + "type": "keyword" + }, + "business_phone": { + "type": "keyword" + }, + "mobile_phone": { + "type": "keyword" + }, + "other_phone": { "type": "keyword" }, "barcode": { diff --git a/rero_ils/modules/patrons/serializers.py b/rero_ils/modules/patrons/serializers.py new file mode 100644 index 0000000000..2d21530555 --- /dev/null +++ b/rero_ils/modules/patrons/serializers.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# 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 . + +"""Patrons serialization.""" + +from invenio_records_rest.serializers.response import search_responsify + +from ..patron_types.api import PatronType +from ..serializers import JSONSerializer, RecordSchemaJSONV1 + + +class PatronJSONSerializer(JSONSerializer): + """Mixin serializing records as JSON.""" + + def post_process_serialize_search(self, results, pid_fetcher): + """Post process the search results.""" + # Add patron type name + for type_term in results.get('aggregations', {}).get( + 'patron_type', {}).get('buckets', []): + pid = type_term.get('key') + name = PatronType.get_record_by_pid(pid).get('name') + type_term['key'] = pid + type_term['name'] = name + + return super().post_process_serialize_search(results, pid_fetcher) + + +json_patron = PatronJSONSerializer(RecordSchemaJSONV1) +"""JSON v1 serializer.""" + +json_patron_search = search_responsify( + json_patron, 'application/rero+json') diff --git a/rero_ils/modules/patrons/tasks.py b/rero_ils/modules/patrons/tasks.py index 58fac6fd24..ed2db2ab46 100644 --- a/rero_ils/modules/patrons/tasks.py +++ b/rero_ils/modules/patrons/tasks.py @@ -55,7 +55,7 @@ def is_obsolete(subscription, end_date=None): # NOTE : this update will trigger the listener # `create_subscription_patron_transaction`. This listener will # create a new subscription if needed - patron.update(patron.dumps(), dbcommit=True, reindex=True) + patron.update(Patron.removeUserData(patron.dumps()), dbcommit=True, reindex=True) def check_patron_types_and_add_subscriptions(): diff --git a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_head.html b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_head.html index 3669f0336f..7bc2b2dba3 100644 --- a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_head.html +++ b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_head.html @@ -21,6 +21,6 @@
-

{{ record.first_name }} {{ record.last_name }}

+

{{ record_dumps.first_name }} {{ record_dumps.last_name }}

diff --git a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_personal.html b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_personal.html index d12228bde2..180c54cde0 100644 --- a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_personal.html +++ b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_personal.html @@ -96,7 +96,7 @@
- {{ record.patron.barcode }} + {{ record.patron.barcode[0] }}
{% endif %} @@ -117,7 +117,7 @@ {% endif %} - {% if record.patron.barcode %} + {% if record.patron %}
{{_('Keep history')}}: @@ -125,7 +125,7 @@
- {% if record.patron.keep_history %} + {% if record_dumps.keep_history %} {{_('The loan history is saved for a maximum of six months. It is visible to you and the library staff.')}} {%- else %} {{_('The loan history is not saved.')}} 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 16629508a2..2aa73d37a2 100644 --- a/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html +++ b/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html @@ -19,6 +19,7 @@ {%- extends 'rero_ils/page_settings.html' %} {% from "rero_ils/_macro_profile.html" import build_fees %} +{% set record_dumps = record.dumps() %} {%- block settings_body %} {% include('rero_ils/_patron_profile_head.html') %} @@ -59,7 +60,7 @@ {{ fees.open.total_amount | format_currency(fees.open.currency) }} - {% if record.patron.keep_history %} + {% if record_dumps.keep_history %}
  • diff --git a/rero_ils/modules/selfcheck/api.py b/rero_ils/modules/selfcheck/api.py index 795b0ad6de..009f1a99ed 100644 --- a/rero_ils/modules/selfcheck/api.py +++ b/rero_ils/modules/selfcheck/api.py @@ -77,7 +77,7 @@ def authorize_patron(barcode, password): """ patron = Patron.get_patron_by_barcode(barcode) if patron and patron.is_patron: - return authorize_selfckeck_user(patron.get('email'), password) + return authorize_selfckeck_user(patron.user.email, password) return False diff --git a/rero_ils/modules/users/api.py b/rero_ils/modules/users/api.py index 33c3892ff0..111de4e9cf 100644 --- a/rero_ils/modules/users/api.py +++ b/rero_ils/modules/users/api.py @@ -20,11 +20,14 @@ from datetime import datetime from flask import current_app, url_for +from flask_babelex import lazy_gettext as _ +from flask_login import current_user from flask_security.confirmable import confirm_user from flask_security.recoverable import send_reset_password_instructions from invenio_accounts.ext import hash_password from invenio_accounts.models import User as BaseUser from invenio_db import db +from invenio_jsonschemas import current_jsonschemas from invenio_records.validators import PartialDraft4Validator from invenio_userprofiles.models import UserProfile from jsonschema import FormatChecker @@ -35,13 +38,27 @@ _records_state = LocalProxy(lambda: current_app.extensions['invenio-records']) +def get_profile_countries(): + """Get country list from the jsonschema.""" + schema = current_jsonschemas.get_schema('common/countries-v0.0.1.json') + options = schema['country']['form']['options'] + return [(option.get('value'), _((option.get('label')))) for option in options] + + +def get_readonly_profile_fields(): + """Disalow to edit some fields for patrons.""" + if current_user.has_role('patron'): + return ['first_name', 'last_name', 'birth_date'] + return ['keep_history'] + class User(object): """User API.""" profile_fields = [ - 'first_name', 'last_name', 'street', 'postal_code', - 'city', 'birth_date', 'username', 'phone', 'keep_history' + 'first_name', 'last_name', 'street', 'postal_code', 'gender', + 'city', 'birth_date', 'username', 'home_phone', 'business_phone', + 'mobile_phone', 'other_phone', 'keep_history', 'country' ] def __init__(self, user): @@ -59,10 +76,10 @@ def create(cls, data, **kwargs): email = data.pop('email', None) roles = data.pop('roles', None) cls._validate(data=data) + password = data.pop('password', + data.get('birth_date', '123456')) user = BaseUser( - password=hash_password( - data.get('password', - data.get('birth_date', '123456'))), + password=hash_password(password), profile=data, active=True) db.session.add(user) profile = user.profile @@ -148,18 +165,24 @@ def dumps(self): 'id': self.user.id, 'links': {'self': url_for( 'api_users.users_item', _external=True, id=self.user.id)}, - 'metadata': {} + 'metadata': self.dumpsMetadata() } + return data + + def dumpsMetadata(self): + """Dumps the profile, email, roles metadata.""" + metadata = {} if self.user.profile: for field in self.profile_fields: value = getattr(self.user.profile, field) - if field == 'birth_date': - value = datetime.strftime(value, '%Y-%m-%d') - data['metadata'][field] = value - data['metadata']['email'] = self.user.email - data['metadata']['roles'] = [r.name for r in self.user.roles] - data = remove_empties_from_dict(data) - return data + if value: + if field == 'birth_date': + value = datetime.strftime(value, '%Y-%m-%d') + metadata[field] = value + if self.user.email: + metadata['email'] = self.user.email + metadata['roles'] = [r.name for r in self.user.roles] + return remove_empties_from_dict(metadata) @classmethod def get_by_username(cls, username): diff --git a/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json b/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json index 8ebc3ca46c..5638f102c5 100644 --- a/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json +++ b/rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json @@ -13,8 +13,13 @@ "street", "postal_code", "city", + "country", "birth_date", - "phone", + "gender", + "home_phone", + "business_phone", + "mobile_phone", + "other_phone", "keep_history" ], "required": [ @@ -56,7 +61,7 @@ "formlyConfig": { "validation": { "messages": { - "pattern": "Should be in the following format: 2022-12-31 (YYYY-MM-DD)." + "patternMessage": "Should be in the following format: 2022-12-31 (YYYY-MM-DD)." } }, "templateOptions": { @@ -65,17 +70,25 @@ } } }, + "gender": { + "title": "Gender", + "enum": [ + "male", + "female", + "other" + ] + }, "username": { "title": "Username", "description": "Login username for the web interface.", "type": "string", - "pattern": "^[a-zA-Z][a-zA-Z0-9-_]{2}[a-zA-Z0-9-_]*$", + "pattern": "^[a-zA-Z0-9][a-zA-Z0-9-_]{2}[a-zA-Z0-9-_]*$", "minLength": 3, "widget": { "formlyConfig": { "validation": { "messages": { - "patternMessage": "Username must start with a letter, be at least three characters long and only contain alphanumeric characters, dashes and underscores.", + "patternMessage": "Username must start with a letter or a number, be at least three characters long and only contain alphanumeric characters, dashes and underscores.", "uniqueUsernameMessage": "This username is already taken." } } @@ -98,16 +111,73 @@ "type": "string", "minLength": 1 }, - "phone": { - "title": "Phone number", - "description": "Phone number with the international prefix, without spaces.", + "country": { + "$ref": "https://ils.rero.ch/schemas/common/countries-v0.0.1.json#/country" + }, + "home_phone": { + "title": "Home phone number", + "description": "Home phone number with the international prefix, without spaces.", + "type": "string", + "pattern": "^\\+[0-9]*$", + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "patternMessage": "Phone number with the international prefix, without spaces, ie +41221234567." + } + }, + "templateOptions": { + "placeholder": "Example: +41791231212" + } + } + } + }, + "business_phone": { + "title": "Business phone number", + "description": "Business phone number with the international prefix, without spaces.", + "type": "string", + "pattern": "^\\+[0-9]*$", + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "patternMessage": "Phone number with the international prefix, without spaces, ie +41221234567." + } + }, + "templateOptions": { + "placeholder": "Example: +41791231212" + } + } + } + }, + "mobile_phone": { + "title": "Mobile phone number", + "description": "Mobile phone number with the international prefix, without spaces.", + "type": "string", + "pattern": "^\\+[0-9]*$", + "widget": { + "formlyConfig": { + "validation": { + "messages": { + "patternMessage": "Phone number with the international prefix, without spaces, ie +41221234567." + } + }, + "templateOptions": { + "placeholder": "Example: +41791231212" + } + } + } + }, + "other_phone": { + "title": "Other phone number", + "description": "Other phone number with the international prefix, without spaces.", "type": "string", "pattern": "^\\+[0-9]*$", "widget": { "formlyConfig": { "validation": { "messages": { - "pattern": "Phone number with the international prefix, without spaces, ie +41221234567." + "patternMessage": "Phone number with the international prefix, without spaces, ie +41221234567." } }, "templateOptions": { diff --git a/rero_ils/modules/utils.py b/rero_ils/modules/utils.py index ac309f8055..d3b6ee6a27 100644 --- a/rero_ils/modules/utils.py +++ b/rero_ils/modules/utils.py @@ -277,19 +277,30 @@ def add_years(initial_date, years): date(initial_date.year, 1, 1)) -def trim_barcode_for_record(data=None): +def trim_patron_barcode_for_record(data=None): """Trim the barcode for a patron or an item record. :param data: the patron or item record :return: data with trimmed barcode """ if data and data.get('barcode'): - data['barcode'] = data.get('barcode').strip() + data['barcode'] = [d.strip() for d in data.get('barcode')] if data and data.get('patron', {}).get('barcode'): - data['patron']['barcode'] = data.get( - 'patron', {}).get('barcode').strip() + data['patron']['barcode'] = [ + d.strip() for d in data['patron']['barcode']] return data +def trim_item_barcode_for_record(data=None): + """Trim the barcode for a patron or an item record. + + :param data: the patron or item record + :return: data with trimmed barcode + """ + if data and data.get('barcode'): + data['barcode'] = data.get('barcode').strip() + if data and data.get('patron', {}).get('barcode'): + data['patron']['barcode'][0] = data['patron']['barcode'][0].strip() + return data def generate_item_barcode(data=None): """Generate a barcode for an item record that does not have one. @@ -303,8 +314,8 @@ def generate_item_barcode(data=None): data['barcode'] = 'f-{}'.format( datetime.now().strftime('%Y%m%d%I%M%S%f')) if data.get('patron') and not data.get('patron', {}).get('barcode'): - data['patron']['barcode'] = 'f-{}'.format( - datetime.now().strftime('%Y%m%d%I%M%S%f')) + data['patron']['barcode'] = [ + 'f-{}'.format(datetime.now().strftime('%Y%m%d%I%M%S%f'))] return data diff --git a/rero_ils/modules/vendors/mappings/v7/vendors/vendor-v0.0.1.json b/rero_ils/modules/vendors/mappings/v7/vendors/vendor-v0.0.1.json index 2c5993e52a..16bc31d8d0 100644 --- a/rero_ils/modules/vendors/mappings/v7/vendors/vendor-v0.0.1.json +++ b/rero_ils/modules/vendors/mappings/v7/vendors/vendor-v0.0.1.json @@ -58,7 +58,7 @@ } } }, - "phone": { + "home_phone": { "type": "keyword" }, "postal_code": { @@ -89,7 +89,7 @@ } } }, - "phone": { + "home_phone": { "type": "keyword" }, "postal_code": { diff --git a/rero_ils/theme/templates/rero_ils/page_settings.html b/rero_ils/theme/templates/rero_ils/page_settings.html index 656c0e6726..9b236bb9f0 100644 --- a/rero_ils/theme/templates/rero_ils/page_settings.html +++ b/rero_ils/theme/templates/rero_ils/page_settings.html @@ -20,9 +20,8 @@ {%- extends config.THEME_BASE_TEMPLATE %} {%- block body scoped %} -
    -
    +
    {%- block settings_menu scoped %}
      {%- for item in current_menu.submenu('settings').children if item.visible %} @@ -35,7 +34,7 @@
    {%- endblock %}
    -
    +
    {%- block settings_content scoped %}
    @@ -60,5 +59,4 @@ {%- endblock %}
    -
    {%- endblock %} diff --git a/rero_ils/theme/views.py b/rero_ils/theme/views.py index 47fec0119e..a6294e7f7f 100644 --- a/rero_ils/theme/views.py +++ b/rero_ils/theme/views.py @@ -427,6 +427,15 @@ def prepare_jsonschema(schema): schema.pop('$schema', None) if 'pid' in schema.get('required', []): schema['required'].remove('pid') + default_country = current_app.config.get('USERPROFILES_DEFAULT_COUNTRY') + # users + field = schema.get('properties', {}).get('country') + # patrons: allOf does not works to remove property + if not field: + field = schema.get('properties', {}).get('second_address', {}).get( + 'properties', {}).get('country') + if field and default_country: + field['default'] = default_country return schema diff --git a/rero_ils/utils.py b/rero_ils/utils.py index 550ecfb106..28b54780b3 100644 --- a/rero_ils/utils.py +++ b/rero_ils/utils.py @@ -17,9 +17,14 @@ """Utilities functions for rero-ils.""" +from copy import deepcopy +from datetime import datetime from flask import current_app from flask_security.confirmable import confirm_user +from invenio_accounts.ext import hash_password +from invenio_accounts.models import User +from invenio_db import db from invenio_i18n.ext import current_i18n @@ -65,15 +70,11 @@ def create_user_from_data(data): :param data: A dict containing a mix of patron and user data. :returns: The modified dict. """ - from datetime import datetime - - from invenio_accounts.ext import hash_password - from invenio_accounts.models import User - from invenio_db import db - + data = deepcopy(data) profile_fields = [ - 'first_name', 'last_name', 'street', 'postal_code', - 'city', 'birth_date', 'username', 'phone', 'keep_history' + 'first_name', 'last_name', 'street', 'postal_code', 'gender', + 'city', 'birth_date', 'username', 'home_phone', 'business_phone', + 'mobile_phone', 'other_phone', 'keep_history', 'country' ] with db.session.begin_nested(): # create the user @@ -83,7 +84,7 @@ def create_user_from_data(data): db.session.add(user) # set the user fields if data.get('email') is not None: - user.email = data.get('email') + user.email = data.pop('email') profile = user.profile # set the profile for field in profile_fields: diff --git a/tests/api/documents/test_marcxml_rest_api.py b/tests/api/documents/test_marcxml_rest_api.py index 832dfab734..a6e1d5ed70 100644 --- a/tests/api/documents/test_marcxml_rest_api.py +++ b/tests/api/documents/test_marcxml_rest_api.py @@ -66,7 +66,7 @@ def test_marcxml_documents_create_with_a_token( runner = CliRunner() res = runner.invoke( tokens_create, - ['-n', 'test', '-u', librarian_martigny.get('email'), + ['-n', 'test', '-u', librarian_martigny.dumps().get('email'), '-t', 'my_token'], obj=script_info ) diff --git a/tests/api/items/test_items_rest.py b/tests/api/items/test_items_rest.py index 68d71c2f50..5ba06772d5 100644 --- a/tests/api/items/test_items_rest.py +++ b/tests/api/items/test_items_rest.py @@ -353,7 +353,7 @@ def test_items_receive(client, librarian_martigny, item_pid = item.pid patron_pid = patron_martigny.pid assert not item.patron_has_an_active_loan_on_item( - patron_martigny.get('patron', {}).get('barcode')) + patron_martigny.get('patron', {}).get('barcode')[0]) location = loc_public_martigny # checkout res, data = postdata( @@ -372,7 +372,7 @@ def test_items_receive(client, librarian_martigny, assert item_data.get('status') == ItemStatus.ON_LOAN assert actions.get(LoanAction.CHECKOUT) assert item.patron_has_an_active_loan_on_item( - patron_martigny.get('patron', {}).get('barcode')) + patron_martigny.get('patron', {}).get('barcode')[0]) loan_pid = actions[LoanAction.CHECKOUT].get('pid') # checkin diff --git a/tests/api/loans/test_loans_rest.py b/tests/api/loans/test_loans_rest.py index d78d2a189d..6331a1e6fd 100644 --- a/tests/api/loans/test_loans_rest.py +++ b/tests/api/loans/test_loans_rest.py @@ -104,7 +104,7 @@ def test_due_soon_loans(client, librarian_martigny, assert not get_last_transaction_loc_for_item(item_pid) assert not item.patron_has_an_active_loan_on_item( - patron_martigny.get('patron', {}).get('barcode')) + patron_martigny.get('patron', {}).get('barcode')[0]) assert item.can_delete assert item.available diff --git a/tests/api/patrons/test_patrons_blocked.py b/tests/api/patrons/test_patrons_blocked.py index 73af4074fb..041660cbd5 100644 --- a/tests/api/patrons/test_patrons_blocked.py +++ b/tests/api/patrons/test_patrons_blocked.py @@ -88,7 +88,7 @@ def test_blocked_patron_cannot_request(client, item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron3_martigny_blocked.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 @@ -102,7 +102,7 @@ def test_blocked_patron_cannot_request(client, item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 diff --git a/tests/api/patrons/test_patrons_rest.py b/tests/api/patrons/test_patrons_rest.py index 9e6d5e424a..c633ba253b 100644 --- a/tests/api/patrons/test_patrons_rest.py +++ b/tests/api/patrons/test_patrons_rest.py @@ -256,7 +256,7 @@ def test_patrons_get(client, librarian_martigny): result = data['hits']['hits'][0]['metadata'] # organisation has been added during the indexing del(result['organisation']) - assert result == patron.replace_refs() + assert result == patron.replace_refs().dumps() @mock.patch('invenio_records_rest.views.verify_record_permission', @@ -309,7 +309,7 @@ def test_patrons_post_put_delete(app, client, lib_martigny, # Update record/PUT data = patron_data - data['patron']['barcode'] = 'barcode_test' + data['patron']['barcode'] = ['barcode_test'] res = client.put( item_url, data=json.dumps(data), @@ -320,19 +320,19 @@ def test_patrons_post_put_delete(app, client, lib_martigny, # Check that the returned record matches the given data data = get_json(res) - assert data['metadata']['patron']['barcode'] == 'barcode_test' + assert data['metadata']['patron']['barcode'][0] == 'barcode_test' res = client.get(item_url) assert res.status_code == 200 data = get_json(res) - assert data['metadata']['patron']['barcode'] == 'barcode_test' + assert data['metadata']['patron']['barcode'][0] == 'barcode_test' res = client.get(list_url) assert res.status_code == 200 data = get_json(res)['hits']['hits'][0] - assert data['metadata']['patron']['barcode'] == 'barcode_test' + assert data['metadata']['patron']['barcode'][0] == 'barcode_test' # Delete record/DELETE res = client.delete(item_url) @@ -354,16 +354,15 @@ def test_patrons_post_without_email(app, client, lib_martigny, patron_data = deepcopy(patron_martigny_data_tmp) patron_data['email'] = 'post_without_email@test.ch' patron_data['username'] = 'post_without_email' + del patron_data['pid'] + del patron_data['email'] + patron_data['patron']['communication_channel'] = 'mail' patron_data = create_user_from_data(patron_data) pids = Patron.count() assert len(mailbox) == 0 # Create record / POST - del patron_data['pid'] - del patron_data['email'] - patron_data['patron']['communication_channel'] = 'mail' - res, _ = postdata( client, 'invenio_records_rest.ptrn_list', @@ -403,7 +402,7 @@ def test_patron_secure_api(client, json_header, assert res.status_code == 403 -def test_patron_secure_api_create(client, patron_type_children_martigny, +def test_patron_secure_api_create(app, client, patron_type_children_martigny, patron_martigny_data, librarian_martigny, librarian_sion): @@ -413,10 +412,13 @@ def test_patron_secure_api_create(client, patron_type_children_martigny, post_entrypoint = 'invenio_records_rest.ptrn_list' del patron_martigny_data['pid'] + patron_martigny_data['email'] = 'secure_api@test.ch' + patron_martigny_data['username'] = 'secure_api' + data = create_user_from_data(patron_martigny_data) res, _ = postdata( client, post_entrypoint, - patron_martigny_data + data ) assert res.status_code == 201 @@ -426,9 +428,12 @@ def test_patron_secure_api_create(client, patron_type_children_martigny, res, _ = postdata( client, post_entrypoint, - patron_martigny_data + data ) assert res.status_code == 403 + ds = app.extensions['invenio-accounts'].datastore + ds.delete_user(ds.find_user(id=data['user_id'])) + def test_patron_secure_api_delete(app, client, librarian_martigny, @@ -463,20 +468,20 @@ def test_patron_secure_api_delete(app, client, librarian_martigny, # clean up ds = app.extensions['invenio-accounts'].datastore - ds.delete_user(ds.find_user(id=data['user_id'])) + ds.delete_user(ds.find_user(id=patron['user_id'])) def test_patrons_dirty_barcode( client, patron_martigny, librarian_martigny): """Test patron update with dirty barcode.""" - barcode = patron_martigny.get('patron', {}).get('barcode') - patron_martigny['patron']['barcode'] = ' {barcode} '.format( + barcode = patron_martigny.get('patron', {}).get('barcode')[0] + patron_martigny['patron']['barcode'] = [' {barcode} '.format( barcode=barcode - ) + )] patron_martigny.update( patron_martigny, dbcommit=True, reindex=True) patron = Patron.get_record_by_pid(patron_martigny.pid) - assert patron.patron.get('barcode') == barcode + assert patron.patron.get('barcode') == [barcode] # Ensure that users with no patron role will not have a barcode librarian_martigny.update( diff --git a/tests/api/patrons/test_patrons_views.py b/tests/api/patrons/test_patrons_views.py index dd8618e8fe..a369b95336 100644 --- a/tests/api/patrons/test_patrons_views.py +++ b/tests/api/patrons/test_patrons_views.py @@ -98,4 +98,4 @@ def test_patron_utils(client, librarian_martigny, from rero_ils.modules.patrons.views import get_patron_from_barcode assert get_patron_from_barcode( - patron.patron.get('barcode')) == patron + patron.patron.get('barcode')[0]) == patron diff --git a/tests/api/selfcheck/test_selfcheck.py b/tests/api/selfcheck/test_selfcheck.py index 634b5649d6..e0d21f1013 100644 --- a/tests/api/selfcheck/test_selfcheck.py +++ b/tests/api/selfcheck/test_selfcheck.py @@ -56,8 +56,9 @@ def test_selfcheck_login(sip2_librarian_martigny): # test success login response = selfcheck_login( - sip2_librarian_martigny.get('email'), - sip2_librarian_martigny.get('birth_date')) + sip2_librarian_martigny.user.email, + sip2_librarian_martigny.dumps().get('birth_date') + ) assert response assert response.get('authenticated') @@ -67,7 +68,7 @@ def test_authorize_patron(sip2_patron_martigny): # try to authorize with wrong password response = authorize_patron(sip2_patron_martigny.get( - 'patron', {}).get('barcode'), 'invalid_password') + 'patron', {}).get('barcode')[0], 'invalid_password') assert not response # try to authorize with wrong barcode @@ -76,8 +77,8 @@ def test_authorize_patron(sip2_patron_martigny): # authorize success response = authorize_patron( - sip2_patron_martigny.get('patron', {}).get('barcode'), - sip2_patron_martigny.get('birth_date')) + sip2_patron_martigny.get('patron', {}).get('barcode')[0], + sip2_patron_martigny.dumps().get('birth_date')) assert response @@ -85,7 +86,7 @@ def test_validate_patron(sip2_patron_martigny): """Test validate patron.""" # test valid patron barcode assert validate_patron_account( - sip2_patron_martigny.get('patron', {}).get('barcode')) + sip2_patron_martigny.get('patron', {}).get('barcode')[0]) # test invalid patron barcode assert not validate_patron_account('invalid_barcode') @@ -101,7 +102,7 @@ def test_system_status(sip2_librarian_martigny): def test_enable_patron(sip2_patron_martigny): """Test enable patron.""" response = enable_patron( - sip2_patron_martigny.get('patron', {}).get('barcode')) + sip2_patron_martigny.get('patron', {}).get('barcode')[0]) assert response['institution_id'] == sip2_patron_martigny\ .library_pid assert response['patron_id'] @@ -159,7 +160,7 @@ def test_patron_information(client, sip2_librarian_martigny, assert res.status_code == 200 # get patron information response = patron_information(sip2_patron_martigny.get( - 'patron', {}).get('barcode')) + 'patron', {}).get('barcode')[0]) assert response # checkin @@ -216,7 +217,7 @@ def test_item_information(client, sip2_librarian_martigny, assert number_of_reminders_sent(loan) == 1 patron_barcode = sip2_patron_martigny\ - .get('patron', {}).get('barcode') + .get('patron', {}).get('barcode')[0] item_pid = item_lib_martigny.pid # get item information @@ -256,7 +257,7 @@ def test_selfcheck_checkout(client, sip2_librarian_martigny, circulation_policies): """Test selfcheck checkout.""" patron_barcode = sip2_patron_martigny \ - .get('patron', {}).get('barcode') + .get('patron', {}).get('barcode')[0] item_barcode = item_lib_martigny.get('barcode') # selfcheck checkout @@ -290,7 +291,7 @@ def test_selfcheck_checkin(client, sip2_librarian_martigny, document, circulation_policies): """Test selfcheck checkin.""" patron_barcode = sip2_patron_martigny \ - .get('patron', {}).get('barcode') + .get('patron', {}).get('barcode')[0] item_barcode = item_lib_martigny.get('barcode') # librarian checkout diff --git a/tests/api/test_availability.py b/tests/api/test_availability.py index 2ac3d753bf..53db7666d5 100644 --- a/tests/api/test_availability.py +++ b/tests/api/test_availability.py @@ -48,7 +48,7 @@ def test_item_can_request( item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 401 @@ -64,7 +64,7 @@ def test_item_can_request( item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 @@ -78,7 +78,7 @@ def test_item_can_request( item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=system_librarian_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 @@ -92,7 +92,7 @@ def test_item_can_request( item_pid='no_item', library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 404 @@ -104,7 +104,7 @@ def test_item_can_request( item_pid=item_lib_martigny.pid, library_pid='no_library', patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 404 @@ -129,7 +129,7 @@ def test_item_can_request( item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 @@ -323,7 +323,7 @@ def test_item_holding_document_availability( item_pid=item_lib_martigny.pid, library_pid=lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 @@ -438,7 +438,7 @@ def test_availability_cipo_allow_request( 'api_item.can_request', item_pid=item_lib_martigny.pid, patron_barcode=patron_martigny.get( - 'patron', {}).get('barcode') + 'patron', {}).get('barcode')[0] ) ) assert res.status_code == 200 diff --git a/tests/api/test_permissions_librarian.py b/tests/api/test_permissions_librarian.py index 6bed512b70..857f03861c 100644 --- a/tests/api/test_permissions_librarian.py +++ b/tests/api/test_permissions_librarian.py @@ -65,7 +65,7 @@ def test_librarian_permissions( "communication_language": "ita" }, "libraries": [{"$ref": "https://ils.rero.ch/api/libraries/lib1"}], - "phone": "+41324993111" + "home_phone": "+41324993111" } record = create_user_from_data(record) # can retrieve all type of users. @@ -103,7 +103,7 @@ def test_librarian_permissions( counter += 1 data = record['data'] data['roles'] = [record['role']] - data['patron']['barcode'] = 'barcode' + str(counter) + data['patron']['barcode'] = ['barcode' + str(counter)] res, _ = postdata( client, @@ -152,7 +152,7 @@ def test_librarian_permissions( counter += 1 data = record['data'] data['roles'] = [record['role']] - data['patron']['barcode'] = 'barcode' + str(counter) + data['patron']['barcode'] = ['barcode' + str(counter)] res, _ = postdata( client, post_entrypoint, @@ -161,7 +161,7 @@ def test_librarian_permissions( assert res.status_code == 403 system_librarian['roles'] = ['system_librarian'] - system_librarian['patron']['barcode'] = 'barcode' + system_librarian['patron']['barcode'] = ['barcode'] res, _ = postdata( client, diff --git a/tests/api/test_permissions_patron.py b/tests/api/test_permissions_patron.py index 5e0cae344e..00e613d2c2 100644 --- a/tests/api/test_permissions_patron.py +++ b/tests/api/test_permissions_patron.py @@ -53,7 +53,7 @@ def test_patron_permissions( "communication_channel": "email", "communication_language": "ita" }, - "phone": "+41324993111" + "home_phone": "+41324993111" } # can not retrieve any type of users. @@ -79,7 +79,7 @@ def test_patron_permissions( counter += 1 data = record['data'] data['roles'] = [record['role']] - data['patron']['barcode'] = 'barcode' + str(counter) + data['patron']['barcode'] = ['barcode' + str(counter)] data['email'] = str(counter) + '@domain.com' res, _ = postdata( client, diff --git a/tests/api/test_permissions_sys_librarian.py b/tests/api/test_permissions_sys_librarian.py index 579e98c080..05a9f59b88 100644 --- a/tests/api/test_permissions_sys_librarian.py +++ b/tests/api/test_permissions_sys_librarian.py @@ -44,7 +44,7 @@ def test_system_librarian_permissions( "postal_code": "1920", "city": "Martigny", "birth_date": "1967-06-07", - "phone": "+41324993111" + "home_phone": "+41324993111" } record = create_user_from_data(record) @@ -101,7 +101,7 @@ def test_system_librarian_permissions( data = record['user'] data.update(record['patch']) if data.get('patron'): - data['patron']['barcode'] = 'barcode' + str(counter) + data['patron']['barcode'] = ['barcode' + str(counter)] res, _ = postdata( client, 'invenio_records_rest.ptrn_list', diff --git a/tests/api/test_search.py b/tests/api/test_search.py index eb5606d825..41fd449d8e 100644 --- a/tests/api/test_search.py +++ b/tests/api/test_search.py @@ -293,7 +293,7 @@ def test_patrons_search( ): """Test document boosting.""" login_user_via_session(client, librarian_martigny.user) - birthdate = librarian_martigny.get('birth_date') + birthdate = librarian_martigny.dumps()['birth_date'] # complete birthdate list_url = url_for( 'invenio_records_rest.ptrn_list', diff --git a/tests/api/test_user_authentication.py b/tests/api/test_user_authentication.py index d0eeb2feeb..1d5138110c 100644 --- a/tests/api/test_user_authentication.py +++ b/tests/api/test_user_authentication.py @@ -26,7 +26,7 @@ def test_login(client, patron_sion): """Test login with several scenarios.""" - + patron_sion = patron_sion.dumps() # user does not exists res, _ = postdata( client, @@ -88,6 +88,7 @@ def test_login(client, patron_sion): def test_login_without_email(client, patron_sion_without_email1): """Test login with several scenarios.""" + patron_sion_without_email1 = patron_sion_without_email1.dumps() # login by username without email res, _ = postdata( client, @@ -108,7 +109,12 @@ def test_change_password(client, patron_martigny, librarian_sion, librarian_martigny): """Test login with several scenarios.""" - + p_martigny = patron_martigny + patron_martigny = patron_martigny.dumps() + l_sion = librarian_sion + librarian_sion = librarian_sion.dumps() + l_martigny = librarian_martigny + librarian_martigny = librarian_martigny.dumps() # try to change password with an anonymous user res, _ = postdata( client, @@ -122,7 +128,7 @@ def test_change_password(client, patron_martigny, assert res.status_code == 401 # with a logged but the password is too short - login_user_via_session(client, patron_martigny.user) + login_user_via_session(client, p_martigny.user) res, _ = postdata( client, 'invenio_accounts_rest_auth.change_password', @@ -149,7 +155,7 @@ def test_change_password(client, patron_martigny, assert data.get('message') == 'You successfully changed your password.' # with a librarian of a different organisation - login_user_via_session(client, librarian_sion.user) + login_user_via_session(client, l_sion.user) res, _ = postdata( client, 'invenio_accounts_rest_auth.change_password', @@ -162,7 +168,7 @@ def test_change_password(client, patron_martigny, assert res.status_code == 401 # with a librarian of the same organisation - login_user_via_session(client, librarian_martigny.user) + login_user_via_session(client, l_martigny.user) res, _ = postdata( client, 'invenio_accounts_rest_auth.change_password', diff --git a/tests/api/users/test_users_api.py b/tests/api/users/test_users_rest.py similarity index 94% rename from tests/api/users/test_users_api.py rename to tests/api/users/test_users_rest.py index b9a25aaf64..364186b478 100644 --- a/tests/api/users/test_users_api.py +++ b/tests/api/users/test_users_rest.py @@ -26,7 +26,7 @@ from utils import get_json, postdata -def test_users_api( +def test_users_post_put( client, user_data_tmp, librarian_martigny, json_header): """Test users REST api for retrieve, create and update.""" first_name = user_data_tmp.get('first_name') @@ -54,6 +54,7 @@ def test_users_api( user_data_tmp.pop('toto') user_data_tmp['first_name'] = 1 + user_data_tmp['password'] = '123456' res, data = postdata( client, 'api_users.users_list', @@ -113,6 +114,11 @@ def test_users_api( def test_users_search_api(client, librarian_martigny, patron_martigny): """Test users search REST API.""" + l_martigny = librarian_martigny + librarian_martigny = librarian_martigny.dumps() + p_martigny = patron_martigny + patron_martigny = patron_martigny.dumps() + res = client.get( url_for( 'api_users.users_list', @@ -121,7 +127,7 @@ def test_users_search_api(client, librarian_martigny, patron_martigny): ) assert res.status_code == 401 - login_user_via_session(client, librarian_martigny.user) + login_user_via_session(client, l_martigny.user) # empty query => no result res = client.get( url_for( diff --git a/tests/data/data.json b/tests/data/data.json index 3a0c1c099c..e9b1cb0313 100644 --- a/tests/data/data.json +++ b/tests/data/data.json @@ -2847,13 +2847,15 @@ ], "email": "simolibri07@gmail.com", "username": "simonetta", - "phone": "+41324993585", + "home_phone": "+41324993585", "roles": [ "system_librarian" ], "patron": { "expiration_date": "2023-10-07", - "barcode": "sys_ptrn1", + "barcode": [ + "sys_ptrn1" + ], "type": { "$ref": "https://ils.rero.ch/api/patron_types/ptty1" }, @@ -2877,7 +2879,7 @@ ], "username": "marie", "email": "mpedronni@gmail.com", - "phone": "+41324993578", + "home_phone": "+41324993578", "roles": [ "librarian" ] @@ -2898,7 +2900,7 @@ ], "username": "marcel", "email": "msaudan@gmail.com", - "phone": "+41324993541", + "home_phone": "+41324993541", "roles": [ "librarian" ] @@ -2919,7 +2921,7 @@ ], "username": "elena", "email": "elena_rodriguez@gmail.com", - "phone": "+41324993515", + "home_phone": "+41324993515", "roles": [ "librarian" ] @@ -2940,7 +2942,7 @@ ], "username": "virgile", "email": "virgile_charbonnet@gmail.com", - "phone": "+41324111515", + "home_phone": "+41324111515", "roles": [ "librarian" ] @@ -2956,7 +2958,7 @@ "birth_date": "1947-06-07", "username": "louis", "email": "lroduit@gmail.com", - "phone": "+41324993156", + "home_phone": "+41324993156", "roles": [ "patron" ], @@ -2964,7 +2966,9 @@ "expiration_date": "2023-10-07", "blocked": false, "blocked_note": "Check that even if blocking block exists, user is safe.", - "barcode": "4098124352", + "barcode": [ + "4098124352" + ], "libraries": [ { "$ref": "https://ils.rero.ch/api/libraries/lib1" @@ -2999,13 +3003,15 @@ "birth_date": "1967-07-07", "username": "louise", "email": "lmoret@gmail.com", - "phone": "+41324993151", + "home_phone": "+41324993151", "roles": [ "patron" ], "patron": { "expiration_date": "2023-10-07", - "barcode": "40981212452", + "barcode": [ + "40981212452" + ], "type": { "$ref": "https://ils.rero.ch/api/patron_types/ptty2" }, @@ -3065,13 +3071,15 @@ "birth_date": "1967-06-07", "username": "jules", "email": "reroilstest+jules@gmail.com", - "phone": "+41324993673", + "home_phone": "+41324993673", "roles": [ "patron" ], "patron": { "expiration_date": "2023-10-07", - "barcode": "4090124789243", + "barcode": [ + "4090124789243" + ], "type": { "$ref": "https://ils.rero.ch/api/patron_types/ptty4" }, @@ -3090,7 +3098,7 @@ "birth_date": "1958-01-12", "username": "marvin", "email": "mlorgne@gmail.com", - "phone": "+41324993142", + "home_phone": "+41324993142", "roles": [ "patron" ], @@ -3098,7 +3106,9 @@ "expiration_date": "2023-10-07", "blocked": true, "blocked_note": "Lost card", - "barcode": "42421313000", + "barcode": [ + "42421313000" + ], "libraries": [ { "$ref": "https://ils.rero.ch/api/libraries/lib1" @@ -3125,7 +3135,7 @@ "birth_date": "1928-01-01", "email": "sharifo@gmail.com", "username": "omar", - "phone": "+41324991231", + "home_phone": "+41324991231", "libraries": [ { "$ref": "https://ils.rero.ch/api/libraries/lib1" @@ -3136,7 +3146,9 @@ ], "patron": { "expiration_date": "2023-10-07", - "barcode": "5127891213", + "barcode": [ + "5127891213" + ], "type": { "$ref": "https://ils.rero.ch/api/patron_types/ptty2" }, @@ -3533,7 +3545,7 @@ "street": "1600 Donald Trump Avenue NW", "postal_code": "20500", "city": "Washington, D.C.", - "phone": "+012024561414", + "home_phone": "+012024561414", "keep_history": true } } diff --git a/tests/e2e/cypress/cypress/fixtures/users.json b/tests/e2e/cypress/cypress/fixtures/users.json index b238563a21..1886f3010c 100644 --- a/tests/e2e/cypress/cypress/fixtures/users.json +++ b/tests/e2e/cypress/cypress/fixtures/users.json @@ -2,7 +2,7 @@ "patrons": { "james": { "patron":{ - "barcode": "cypress-1" + "barcode": ["cypress-1"] }, "email": "reroilstest+james@gmail.com", "first_name": "James", @@ -11,7 +11,7 @@ }, "nyota": { "patron": { - "barcode":"cypress-2", + "barcode": ["cypress-2"], "communication_channel":"email", "communication_language":"eng", "expiration_date":"2023-10-07", @@ -22,7 +22,7 @@ "email":"reroilstest+nyota@gmail.com", "first_name":"Nyota", "last_name":"Uhura", - "phone":"+41324883123", + "home_phone":"+41324883123", "pid":"53", "postal_code":"4242", "roles":["patron"], diff --git a/tests/e2e/cypress/cypress/support/api.js b/tests/e2e/cypress/cypress/support/api.js index 9f2e768844..15477cb4b8 100644 --- a/tests/e2e/cypress/cypress/support/api.js +++ b/tests/e2e/cypress/cypress/support/api.js @@ -152,7 +152,7 @@ Cypress.Commands.add('apiDeleteResources', (resourceName, query) => { "type": { "$ref":('https://ils.rero.ch/api/patron_types/' + patronTypePid) }, - "barcode": patron.patron.barcode, + "barcode": [patron.patron.barcode], "communication_channel": patron.patron.communication_channel, "communication_language": patron.patron.communication_language }, @@ -186,7 +186,7 @@ Cypress.Commands.add('apiDeleteResources', (resourceName, query) => { followRedirect: false, body: { "patron": { - "barcode": patron.patron.barcode, + "barcode": [patron.patron.barcode], "communication_channel": patron.patron.communication_channel, "communication_language": patron.patron.communication_language, "expiration_date": patron.patron.expiration_date, @@ -200,7 +200,7 @@ Cypress.Commands.add('apiDeleteResources', (resourceName, query) => { "email": patron.email, "first_name": patron.first_name, "last_name": patron.last_name, - "phone": patron.phone, + "home_phone": patron.home_phone, "pid": patron.pid, "postal_code": patron.postal_code, "roles": patron.roles, diff --git a/tests/fixtures/sip2.py b/tests/fixtures/sip2.py index 005fb0515f..109cf98792 100644 --- a/tests/fixtures/sip2.py +++ b/tests/fixtures/sip2.py @@ -50,6 +50,7 @@ def sip2_patron_martigny_data(data): def sip2_patron_martigny( app, roles, + lib_martigny, patron_type_children_martigny, sip2_patron_martigny_data): """Create Martigny patron without sending reset password instruction.""" diff --git a/tests/ui/circulation/test_actions_checkout.py b/tests/ui/circulation/test_actions_checkout.py index 914eeceec8..39a529fb72 100644 --- a/tests/ui/circulation/test_actions_checkout.py +++ b/tests/ui/circulation/test_actions_checkout.py @@ -50,7 +50,7 @@ def test_checkout_on_item_on_shelf( assert created_item.number_of_requests() == 0 assert created_item.status == ItemStatus.ON_SHELF assert not created_item.is_requested_by_patron( - patron_martigny.get('patron', {}).get('barcode')) + patron_martigny.get('patron', {}).get('barcode')[0]) # the following tests the circulation action CHECKOUT_1_1 # an ON_SHELF item @@ -95,7 +95,7 @@ def test_checkout_on_item_on_shelf( # checkout patron = patron of first PENDING loan # CAN be CHECKOUT assert pending_item.is_requested_by_patron( - patron.patron.get('barcode')) + patron.patron.get('barcode')[0]) # Checkout it! CHECKOUT patron == 1st PENDING LOAN patron assert patron.get('pid') == loan.get('patron_pid') params['patron_pid'] = patron_martigny.pid diff --git a/tests/ui/circulation/test_inhouse_cipo.py b/tests/ui/circulation/test_inhouse_cipo.py index bca2d14c74..68c2fb031a 100644 --- a/tests/ui/circulation/test_inhouse_cipo.py +++ b/tests/ui/circulation/test_inhouse_cipo.py @@ -48,7 +48,7 @@ def test_less_than_one_day_checkout( assert created_item.number_of_requests() == 0 assert created_item.status == ItemStatus.ON_SHELF assert not created_item.is_requested_by_patron( - patron2_martigny.get('patron', {}).get('barcode')) + patron2_martigny.get('patron', {}).get('barcode')[0]) # Ensure than the transaction date used will be an open_day. owner_lib = Library.get_record_by_pid(created_item.library_pid) diff --git a/tests/ui/patrons/test_patrons_api.py b/tests/ui/patrons/test_patrons_api.py index bd40a1baf5..44d05b4650 100644 --- a/tests/ui/patrons/test_patrons_api.py +++ b/tests/ui/patrons/test_patrons_api.py @@ -43,7 +43,7 @@ def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, assert len(mailbox) == 0 assert User.query.count() == 0 assert UserProfile.query.count() == 0 - + l_martigny_data_tmp = librarian_martigny_data_tmp librarian_martigny_data_tmp = create_user_from_data(librarian_martigny_data_tmp) # wrong_librarian_martigny_data_tmp = deepcopy(librarian_martigny_data_tmp) # wrong_librarian_martigny_data_tmp.pop('first_name') @@ -66,7 +66,7 @@ def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, wrong_librarian_martigny_data_tmp = deepcopy(librarian_martigny_data_tmp) wrong_librarian_martigny_data_tmp.setdefault('patron', { 'expiration_date': '2023-10-07', - 'barcode': '2050124311', + 'barcode': ['2050124311'], 'type': { '$ref': 'https://ils.rero.ch/api/patron_types/ptty2' }, @@ -103,12 +103,12 @@ def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, assert user.active for field in [ 'first_name', 'last_name', 'street', 'postal_code', 'city', 'username', - 'phone' + 'home_phone' ]: - assert getattr(user.profile, field) == librarian_martigny_data_tmp.get( + assert getattr(user.profile, field) == l_martigny_data_tmp.get( field) user.profile.birth_date == datetime.strptime( - librarian_martigny_data_tmp.get('birth_date'), '%Y-%m-%d') + l_martigny_data_tmp.get('birth_date'), '%Y-%m-%d') user_roles = [r.name for r in user.roles] assert set(user_roles) == set(ptrn.get('roles')) # TODO: make these checks during the librarian POST creation @@ -143,7 +143,7 @@ def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, 'roles': Patron.available_roles, 'patron': { 'expiration_date': '2023-10-07', - 'barcode': '2050124311', + 'barcode': ['2050124311'], 'type': { '$ref': 'https://ils.rero.ch/api/patron_types/ptty2' }, @@ -281,10 +281,10 @@ def test_patron_organisation_pid(org_martigny, patron_martigny, def test_get_patron(patron_martigny): """Test patron retrieval.""" patron = patron_martigny - assert Patron.get_patron_by_email(patron.get('email')) == patron + assert Patron.get_patron_by_email(patron.dumps().get('email')) == patron assert not Patron.get_patron_by_email('not exists') assert Patron.get_patron_by_barcode( - patron.patron.get('barcode')) == patron + patron.patron.get('barcode')[0]) == patron assert not Patron.get_patron_by_barcode('not exists') assert Patron.get_patron_by_user(patron.user) == patron @@ -295,20 +295,19 @@ class user: def test_user_librarian_can_delete(librarian_martigny): """Test can delete a librarian.""" - print('bla', User.query.all()) assert librarian_martigny.get_links_to_me() == {} assert librarian_martigny.can_delete def test_get_patron_blocked_field(patron_martigny): """Test patron blocked field retrieval.""" - patron = Patron.get_patron_by_email(patron_martigny.get('email')) + patron = Patron.get_patron_by_email(patron_martigny.dumps().get('email')) assert patron.patron.get('blocked') is False def test_get_patron_blocked_field_absent(patron2_martigny): """Test patron blocked field retrieval.""" - patron = Patron.get_patron_by_email(patron2_martigny.get('email')) + patron = Patron.get_patron_by_email(patron2_martigny.dumps().get('email')) assert 'blocked' not in patron diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 11ed13b578..53a5aed405 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -25,6 +25,8 @@ from pkg_resources import resource_string from utils import get_schema +from rero_ils.modules.patrons.api import Patron + @pytest.fixture(scope='module') def create_app(): @@ -172,7 +174,7 @@ def patron_schema(monkeypatch): @pytest.fixture(scope="function") def patron_martigny_data_tmp_with_id(patron_martigny_data_tmp): """Load Martigny patron data scope function with a mocked user_id.""" - patron = deepcopy(patron_martigny_data_tmp) + patron = Patron.removeUserData(deepcopy(patron_martigny_data_tmp)) # mock the user_id which is add by the Patron API. patron['user_id'] = 100 return patron diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index ddd96a8972..268174b6ab 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -46,7 +46,7 @@ def test_cli_access_token(app, script_info, patron_martigny): runner = CliRunner() res = runner.invoke( tokens_create, - ['-n', 'test', '-u', patron_martigny.get('email'), + ['-n', 'test', '-u', patron_martigny.dumps().get('email'), '-t', 'my_token'], obj=script_info ) diff --git a/tests/unit/test_patrons_jsonschema.py b/tests/unit/test_patrons_jsonschema.py index 80416786aa..72925f64cf 100644 --- a/tests/unit/test_patrons_jsonschema.py +++ b/tests/unit/test_patrons_jsonschema.py @@ -92,7 +92,7 @@ def test_barcode(patron_schema, patron_martigny_data_tmp_with_id): validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp_with_id['patron']['barcode'] = 25 + patron_martigny_data_tmp_with_id['patron']['barcode'][0] = 25 validate(patron_martigny_data_tmp_with_id, patron_schema) @@ -105,26 +105,12 @@ def test_birth_date(patron_schema, patron_martigny_data_tmp_with_id): validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_email(patron_schema, patron_martigny_data_tmp_with_id): - """Test email for patron jsonschemas.""" - validate(patron_martigny_data_tmp_with_id, patron_schema) - - # email is optional - del patron_martigny_data_tmp_with_id['email'] - validate(patron_martigny_data_tmp_with_id, patron_schema) - - # email with a bad format - with pytest.raises(ValidationError): - patron_martigny_data_tmp_with_id['email'] = 25 - validate(patron_martigny_data_tmp_with_id, patron_schema) - - def test_phone(patron_schema, patron_martigny_data_tmp_with_id): """Test phone for patron jsonschemas.""" validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp_with_id['phone'] = 25 + patron_martigny_data_tmp_with_id['home_phone'] = 25 validate(patron_martigny_data_tmp_with_id, patron_schema) diff --git a/tests/unit/test_users_jsonschema.py b/tests/unit/test_users_jsonschema.py index 87f068b3ec..9ce0f7745b 100644 --- a/tests/unit/test_users_jsonschema.py +++ b/tests/unit/test_users_jsonschema.py @@ -39,11 +39,16 @@ def test_user_all_jsonschema_keys_values( {'key': 'first_name', 'value': 25}, {'key': 'last_name', 'value': 25}, {'key': 'birth_date', 'value': 25}, + {'key': 'gender', 'value': 25}, {'key': 'username', 'value': 25}, {'key': 'street', 'value': 25}, {'key': 'postal_code', 'value': 25}, {'key': 'city', 'value': 25}, - {'key': 'phone', 'value': 25}, + {'key': 'country', 'value': 25}, + {'key': 'mobile_phone', 'value': 25}, + {'key': 'business_phone', 'value': 25}, + {'key': 'mobile_phone', 'value': 25}, + {'key': 'other_phone', 'value': 25}, {'key': 'keep_history', 'value': 25}, {'key': 'user_id', 'value': '25'} ] diff --git a/tests/utils.py b/tests/utils.py index 4e43f390dc..a22ff5c20d 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -63,7 +63,7 @@ def login_user(client, user): def login_user_for_view(client, user): """Sign in user for view.""" invenio_user = user.user - invenio_user.password_plaintext = user.get('birth_date') + invenio_user.password_plaintext = user.dumps().get('birth_date') login_user_via_view(client, user=invenio_user) @@ -325,7 +325,7 @@ def item_record_to_a_specific_loan_state( loan = Loan.get_record_by_pid(actions[LoanAction.REQUEST].get('pid')) assert item.number_of_requests() >= 1 assert item.is_requested_by_patron(patron.get( - 'patron', {}).get('barcode')) + 'patron', {}).get('barcode')[0]) if loan_state in [ LoanState.ITEM_AT_DESK, LoanState.ITEM_IN_TRANSIT_FOR_PICKUP,