From b29a85d0e56467af1697f1d9fe1f81dc45d2beea Mon Sep 17 00:00:00 2001 From: Carlin MacKenzie Date: Wed, 6 Nov 2024 15:20:25 +0100 Subject: [PATCH 1/3] bug: add handler for handling changed domain remove setting of role.id as the added flush() will add the ID from DB --- invenio_accounts/datastore.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/invenio_accounts/datastore.py b/invenio_accounts/datastore.py index a235b8f7..a7a554db 100644 --- a/invenio_accounts/datastore.py +++ b/invenio_accounts/datastore.py @@ -75,11 +75,16 @@ def commit(self): def mark_changed(self, sid, uid=None, rid=None, model=None): """Save a user to the changed history.""" + # needed so that we have the id from the DB on the model + self.db.session.flush() + if model: if isinstance(model, User): current_db_change_history.add_updated_user(sid, model.id) elif isinstance(model, Role): current_db_change_history.add_updated_role(sid, model.id) + elif isinstance(model, Domain): + current_db_change_history.add_updated_domain(sid, model.id) elif uid: # Deprecated - use model param instead (still used in e.g. # UserFixture pytest-invenio) @@ -91,20 +96,12 @@ def mark_changed(self, sid, uid=None, rid=None, model=None): def update_role(self, role): """Updates roles.""" role = self.db.session.merge(role) - # This works because role defines it's own id - for users - # the same doesn't work because id is assigned on commit which - # hasn't happened yet. self.mark_changed(id(self.db.session), model=role) return role def create_role(self, **kwargs): """Creates and returns a new role from the given parameters.""" role = super().create_role(**kwargs) - # This works because role defines it's own id - for users - # the same doesn't work because id is assigned on commit which - # hasn't happened yet. - if role.id is None: - role.id = role.name self.mark_changed(id(self.db.session), model=role) return role From 72e7cf1bd3ab29b27c9bdb27bceb9552050ac992 Mon Sep 17 00:00:00 2001 From: Carlin MacKenzie Date: Thu, 7 Nov 2024 10:02:36 +0100 Subject: [PATCH 2/3] domains: extend find_domain to accept ID --- invenio_accounts/datastore.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/invenio_accounts/datastore.py b/invenio_accounts/datastore.py index a7a554db..2d8e7c65 100644 --- a/invenio_accounts/datastore.py +++ b/invenio_accounts/datastore.py @@ -106,14 +106,23 @@ def create_role(self, **kwargs): return role def find_role_by_id(self, role_id): - """Fetches roles searching by id.""" + """Fetches roles searching by ID.""" return db.session.query(self.role_model).filter_by(id=role_id).one_or_none() - def find_domain(self, domain): - """Find a domain.""" + def find_domain(self, domain_or_id): + """Find a domain by value or ID.""" + if isinstance(domain_or_id, str): + if domain_or_id.isdigit(): + clause = Domain.id == int(domain_or_id) + else: + clause = Domain.domain == domain_or_id + elif isinstance(domain_or_id, int): + clause = Domain.id == domain_or_id + else: + raise ValueError("Expected string or int, received:", type(domain_or_id)) return ( db.session.query(Domain) - .filter_by(domain=domain) + .filter(clause) .options(joinedload(Domain.category_name)) .one_or_none() ) From 4644611cc8ce2681a29bd2d5237242f143a9115c Mon Sep 17 00:00:00 2001 From: Carlin MacKenzie Date: Fri, 8 Nov 2024 16:11:52 +0100 Subject: [PATCH 3/3] domains: only flush if data not persisted --- invenio_accounts/datastore.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/invenio_accounts/datastore.py b/invenio_accounts/datastore.py index 2d8e7c65..98808247 100644 --- a/invenio_accounts/datastore.py +++ b/invenio_accounts/datastore.py @@ -14,6 +14,7 @@ from flask import current_app from flask_security import SQLAlchemyUserDatastore, user_confirmed from invenio_db import db +from sqlalchemy import inspect from sqlalchemy.orm import joinedload from .models import Domain, Role, User @@ -75,10 +76,11 @@ def commit(self): def mark_changed(self, sid, uid=None, rid=None, model=None): """Save a user to the changed history.""" - # needed so that we have the id from the DB on the model - self.db.session.flush() - if model: + # add the ID to the model from the DB if needed + if not inspect(model).persistent: + self.db.session.flush() + if isinstance(model, User): current_db_change_history.add_updated_user(sid, model.id) elif isinstance(model, Role):