From 7492a700a6fb342444128fb2ccc3c6e7d5fdfac0 Mon Sep 17 00:00:00 2001 From: Peter Weber Date: Wed, 23 Oct 2024 09:39:35 +0200 Subject: [PATCH] logging: add user information to sentry * Closes: #2734. Co-Authored-by: Peter Weber --- docs/conf.py | 2 +- rero_ils/config.py | 5 +++++ rero_ils/modules/patrons/api.py | 8 ++----- rero_ils/modules/patrons/utils.py | 6 +++--- rero_ils/modules/utils.py | 36 ++++++++++++++++++++----------- tests/ui/conftest.py | 27 +++++++++++++++++++---- tests/ui/test_views.py | 23 ++++++++++++++------ 7 files changed, 75 insertions(+), 32 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 4cf0bfdea2..35289690ae 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -24,7 +24,7 @@ # -- General configuration ------------------------------------------------ # If your documentation needs a minimal Sphinx version, state it here. -#needs_sphinx = '1.0' +# needs_sphinx = '1.0' # Do not warn on external images. suppress_warnings = ['image.nonlocal_uri'] diff --git a/rero_ils/config.py b/rero_ils/config.py index d5ed19b187..3833b7e5b6 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -3458,6 +3458,11 @@ def _(x): LOGGING_SENTRY_LEVEL = "ERROR" #: Sentry: use celery or not LOGGING_SENTRY_CELERY = True +# Invenio logging +LOGGING_SENTRY_INIT_KWARGS = { + # instuct sentry to send user data attached to the event + "send_default_pii": True +} ROLLOVER_LOGGING_CONFIG = { "version": 1, diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 6f61fa18cb..1f9352849b 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -53,6 +53,7 @@ get_patron_from_arguments, get_ref_for_pid, sorted_pids, + user_formatted_name, ) from .extensions import UserDataExtension @@ -768,12 +769,7 @@ def is_expired(self): @property def formatted_name(self): """Return the best possible human-readable patron name.""" - profile = self.user.user_profile - name_parts = [ - profile.get("last_name", "").strip(), - profile.get("first_name", "").strip(), - ] - return ", ".join(filter(None, name_parts)) + return user_formatted_name(self.user) @property def patron_type_pid(self): diff --git a/rero_ils/modules/patrons/utils.py b/rero_ils/modules/patrons/utils.py index 54f07261fe..9b728471dc 100644 --- a/rero_ils/modules/patrons/utils.py +++ b/rero_ils/modules/patrons/utils.py @@ -77,7 +77,7 @@ def create_user_from_data(data, send_email=False): """Create a user and set the profile fields from a data. :param data: A dict containing a mix of patron and user data. - :param send_email - send the reset password email to the user + :param send_email: send the reset password email to the user :returns: The modified dict. """ user = User.get_by_username(data.get("username")) @@ -94,8 +94,8 @@ def create_user_from_data(data, send_email=False): def create_patron_from_data(data, dbcommit=True, reindex=True, send_email=False): """Create a patron and a user from a data dict. - :param data - dictionary representing a library user - :param send_email - send the reset password email to the user + :param data: dictionary representing a library user + :param send_email: send the reset password email to the user :returns: - A `Patron` instance """ from .api import Patron diff --git a/rero_ils/modules/utils.py b/rero_ils/modules/utils.py index 75e0db8c27..d5c32c8c86 100644 --- a/rero_ils/modules/utils.py +++ b/rero_ils/modules/utils.py @@ -42,7 +42,6 @@ from flask import current_app, session from flask_babel import gettext as _ from flask_babel import ngettext -from flask_login import current_user from invenio_accounts.models import Role from invenio_cache import current_cache from invenio_pidstore.models import PersistentIdentifier @@ -1085,22 +1084,35 @@ def close(self): self.__del__() +def user_formatted_name(user): + """Formatted name from user profile. + + :param user: user to use. + :returns: formatted name. + """ + profile = user.user_profile + name_parts = [ + profile.get("last_name", "").strip(), + profile.get("first_name", "").strip(), + ] + return ", ".join(filter(None, name_parts)) + + def set_user_name(sender, user): """Set the username in the current flask session.""" - from .patrons.api import current_librarian, current_patrons - - user_name = None remove_user_name(sender, user) - if current_librarian: - user_name = current_librarian.formatted_name - elif current_patrons: - user_name = current_patrons[0].formatted_name + user_email = None + with contextlib.suppress(AttributeError): + user_email = user.email + + # set session user name + if full_name := user_formatted_name(user): + session["user_name"] = full_name + elif user_email: + session["user_name"] = user_email else: - with contextlib.suppress(AttributeError): - user_name = current_user.email - if user_name: - session["user_name"] = user_name + session["user_name"] = user.username def remove_user_name(sender, user): diff --git a/tests/ui/conftest.py b/tests/ui/conftest.py index e7a61158e5..2627e1e56f 100644 --- a/tests/ui/conftest.py +++ b/tests/ui/conftest.py @@ -47,19 +47,38 @@ def user_with_profile(db, default_user_password): @pytest.fixture(scope="function") -def user_without_email(db, default_user_password): +def user_without_name_email(db, default_user_password): """Create a simple invenio user without email.""" with db.session.begin_nested(): profile = dict( birth_date="1990-01-01", - first_name="User", - last_name="With Profile", city="Nowhere", ) user = User( password=hash_password(default_user_password), user_profile=profile, - username="user_without_email", + username="user_without_name_email", + active=True, + ) + db.session.add(user) + db.session.commit() + user.password_plaintext = default_user_password + return user + + +@pytest.fixture(scope="function") +def user_without_name(db, default_user_password): + """Create a simple invenio user without nmae.""" + with db.session.begin_nested(): + profile = dict( + birth_date="1990-01-01", + city="Nowhere", + ) + user = User( + password=hash_password(default_user_password), + user_profile=profile, + username="user_without_name", + email="user_without_name@test.com", active=True, ) db.session.add(user) diff --git a/tests/ui/test_views.py b/tests/ui/test_views.py index 83ba840209..3ec8f7c8dd 100644 --- a/tests/ui/test_views.py +++ b/tests/ui/test_views.py @@ -24,6 +24,7 @@ from flask_login import login_user, logout_user from utils import postdata +from rero_ils.modules.utils import user_formatted_name from rero_ils.theme.views import nl2br @@ -135,20 +136,30 @@ def test_language(client, app): def test_set_user_name( - app, librarian_martigny, patron_martigny, user_with_profile, user_without_email + app, + librarian_martigny, + patron_martigny, + user_with_profile, + user_without_name, + user_without_name_email, ): """Test the user_name in the flask session.""" - # should be the email address + # should be the formatted name login_user(user=user_with_profile) assert "user_name" in session - assert session["user_name"] == user_with_profile.email + assert session["user_name"] == user_formatted_name(user_with_profile) # should be removed logout_user() assert "user_name" not in session - # should not be set - login_user(user=user_without_email) - assert "user_name" not in session + # should be the email + login_user(user=user_without_name) + assert session["user_name"] == user_without_name.email + logout_user() + + # should be the user.username + login_user(user=user_without_name_email) + assert session["user_name"] == user_without_name_email.username logout_user() # should be the formatted name