diff --git a/rero_ils/config.py b/rero_ils/config.py index f7e541b15c..f57ac10679 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -2080,6 +2080,10 @@ def _(x): #: Allow users to login without first confirming their email address. SECURITY_LOGIN_WITHOUT_CONFIRMATION = False +#: TODO: remove this when the email is sent only if the user has an email +# address +SECURITY_SEND_PASSWORD_CHANGE_EMAIL = False + # Misc INDEXER_REPLACE_REFS = True INDEXER_RECORD_TO_INDEX = 'rero_ils.modules.indexer_utils.record_to_index' diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 59d75351cb..5f8035ad88 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -32,7 +32,6 @@ from invenio_userprofiles.models import UserProfile from sqlalchemy.orm.exc import NoResultFound from werkzeug.local import LocalProxy -from werkzeug.utils import cached_property from .models import PatronIdentifier, PatronMetadata from .utils import get_patron_from_arguments @@ -173,12 +172,7 @@ def create(cls, data, id_=None, delete_pid=False, data = trim_barcode_for_record(data=data) # synchronize the rero id user profile data user = cls.sync_user_and_profile(data) - try: - # for a fresh created user - if user: - # link by id - data.setdefault('user_id', user.id) record = super(Patron, cls).create( data, id_, delete_pid, dbcommit, reindex, **kwargs) record._update_roles() @@ -197,7 +191,8 @@ def update(self, data, dbcommit=False, reindex=False): # remove spaces data = trim_barcode_for_record(data=data) # synchronize the rero id user profile data - self.sync_user_and_profile(dict(self, **data)) + data = dict(self, **data) + self.sync_user_and_profile(data) super(Patron, self).update(data, dbcommit, reindex) self._update_roles() return self @@ -226,6 +221,17 @@ def update_from_profile(cls, profile): value = getattr(profile, field) if value not in [None, '']: patron[field] = value + # update the email + if profile.user.email != patron.get('email'): + # the email is not defined or removed in the user profile + if not profile.user.email: + try: + del patron['email'] + except KeyError: + pass + else: + # the email has been updated in the user profile + patron['email'] = profile.user.email super(Patron, patron).update(dict(patron), True, True) @classmethod @@ -238,8 +244,6 @@ def _get_user_by_data(cls, data): :return: a patron object or None. """ user = None - if data.get('user_id'): - user = User.query.filter_by(id=data.get('user_id')).first() if not user and data.get('username'): try: user = UserProfile.get_by_username(data.get('username')).user @@ -247,6 +251,8 @@ def _get_user_by_data(cls, data): user = None if not user and data.get('email'): user = User.query.filter_by(email=data.get('email')).first() + if not user and not data.get('user_id'): + user = User.query.filter_by(id=data.get('user_id')).first() return user @classmethod @@ -255,9 +261,10 @@ def sync_user_and_profile(cls, data): :param data - dict representing the patron data """ + created = False # start a session to be able to rollback if the data are not valid + user = cls._get_user_by_data(data) with db.session.begin_nested(): - user = cls._get_user_by_data(data) # need to create the user if not user: birth_date = data.get('birth_date') @@ -270,24 +277,31 @@ def sync_user_and_profile(cls, data): password=hash_password(birth_date), profile=dict(), active=True) db.session.add(user) + created = True else: if user.email != data.get('email'): user.email = data.get('email') # update all common fields + if user.profile is None: + user.profile = UserProfile(user_id=user.id) + profile = user.profile for field in cls.profile_fields: # date field need conversion if field == 'birth_date': setattr( - user.profile, field, + profile, field, datetime.strptime(data.get(field), '%Y-%m-%d')) else: - setattr(user.profile, field, data.get(field, '')) + setattr(profile, field, data.get(field, '')) db.session.merge(user) - if not data.get('user_id'): + data['user_id'] = user.id + if created: # the fresh created user return user - @cached_property + # TODO: use cached property one we found how to invalidate the cache when + # the user change + @property def user(self): """Invenio user of a patron.""" user = _datastore.find_user(id=self.get('user_id')) 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 015e903cea..0946b8be0d 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 @@ -80,7 +80,7 @@ "minLength": 6, "form": { "expressionProperties": { - "templateOptions.required": "field.parent.model.roles.some(v => (v === 'librarian' || v === 'system_librarian'))" + "templateOptions.required": "field.parent.model.roles.some(v => (v === 'librarian' || v === 'system_librarian')) || field.parent.model.patron.communication_channel === 'email'" }, "validation": { "validators": { diff --git a/tests/ui/patrons/test_patrons_api.py b/tests/ui/patrons/test_patrons_api.py index 2b840e7f96..75272ae2ae 100644 --- a/tests/ui/patrons/test_patrons_api.py +++ b/tests/ui/patrons/test_patrons_api.py @@ -25,6 +25,7 @@ import pytest from invenio_accounts.models import User +from invenio_accounts.testutils import create_test_user from invenio_userprofiles import UserProfile from jsonschema.exceptions import ValidationError @@ -167,18 +168,19 @@ def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, def test_patron_create_without_email(app, roles, patron_type_children_martigny, patron_martigny_data_tmp, mailbox): """Test Patron creation without an email.""" - del patron_martigny_data_tmp['email'] + patron_martigny_data_tmp = deepcopy(patron_martigny_data_tmp) # no data has been created mailbox.clear() - # assert User.query.count() == 0 - # assert UserProfile.query.count() == 0 + # create a patron without email + del patron_martigny_data_tmp['email'] ptrn = Patron.create( patron_martigny_data_tmp, dbcommit=True, delete_pid=True ) + # user has been created user = User.query.filter_by(id=ptrn.get('user_id')).first() assert user assert not user.email @@ -186,26 +188,31 @@ def test_patron_create_without_email(app, roles, patron_type_children_martigny, assert user.active assert len(mailbox) == 0 + # add an email of a non existing user patron_martigny_data_tmp['email'] = 'test@test.ch' ptrn.replace( data=patron_martigny_data_tmp, dbcommit=True ) + # the user remains the same assert user == ptrn.user assert user.email == patron_martigny_data_tmp['email'] assert user.active assert len(mailbox) == 0 + # update with a new email in the system patron_martigny_data_tmp['email'] = 'test@test1.ch' ptrn.replace( data=patron_martigny_data_tmp, dbcommit=True ) + # the user remains the same assert user == ptrn.user assert user.email == patron_martigny_data_tmp['email'] assert user.active assert len(mailbox) == 0 + # remove the email del patron_martigny_data_tmp['email'] ptrn.replace( data=patron_martigny_data_tmp, @@ -216,6 +223,26 @@ def test_patron_create_without_email(app, roles, patron_type_children_martigny, assert user.active assert len(mailbox) == 0 + # create a new invenio user in the system + rero_id_user = create_test_user(email='reroid@test.com', active=True) + + # update the patron with the email of the freshed create invenio user + patron_martigny_data_tmp['email'] = 'reroid@test.com' + patron_martigny_data_tmp['username'] = 'reroid' + ptrn.replace( + data=patron_martigny_data_tmp, + dbcommit=True + ) + # the user linked with the patron has been changed + assert rero_id_user == ptrn.user + # the username is updated on both user profile and patron + assert rero_id_user.profile.username == ptrn.get('username') == 'reroid' + + # clean up created users + ds = app.extensions['invenio-accounts'].datastore + ds.delete_user(user) + ds.delete_user(rero_id_user) + def test_patron_organisation_pid(org_martigny, patron_martigny_no_email, librarian_martigny_no_email):