Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
"""Add user.shortcuts_preferences column."""

from alembic import op
from sqlalchemy import Column
from sqlalchemy.dialects.postgresql import JSONB

revision = "7c1a3e2b9f20"
down_revision = "d4dd768e1439"


def upgrade():
op.add_column("user", Column("shortcuts_preferences", JSONB, nullable=True))


def downgrade():
op.drop_column("user", "shortcuts_preferences")
4 changes: 4 additions & 0 deletions h/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import TYPE_CHECKING

import sqlalchemy as sa
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.ext.hybrid import Comparator, hybrid_property
from sqlalchemy.orm import Mapped, mapped_column
Expand Down Expand Up @@ -231,6 +232,9 @@ def __table_args__(cls): # noqa: N805
# Has the user opted-in for news etc.
comms_opt_in = sa.Column(sa.Boolean, nullable=True)

# A JSON blob with user shortcuts preferences.
shortcuts_preferences = sa.Column(JSONB, nullable=True)

identities = sa.orm.relationship(
"UserIdentity", backref="user", cascade="all, delete-orphan"
)
Expand Down
57 changes: 56 additions & 1 deletion h/services/user.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
from collections.abc import Mapping

import sqlalchemy as sa

from h.models import User, UserIdentity
from h.util.db import on_transaction_end
from h.util.user import split_user

UPDATE_PREFS_ALLOWED_KEYS = {"show_sidebar_tutorial"}
UPDATE_PREFS_ALLOWED_KEYS = {
"show_sidebar_tutorial",
"shortcuts_preferences",
}
REPEATABLE_SHORTCUT_GROUPS = []
ALLOWED_SHORTCUT_ACTIONS = {
"applyUpdates",
"openKeyboardShortcuts",
"openSearch",
"annotateSelection",
"highlightSelection",
"toggleHighlights",
"showSelection",
"hideAdder",
}


class UserNotActivated(Exception): # noqa: N818
Expand Down Expand Up @@ -171,7 +187,46 @@ def update_preferences(user, **kwargs):
if "show_sidebar_tutorial" in kwargs: # pragma: no cover
user.sidebar_tutorial_dismissed = not kwargs["show_sidebar_tutorial"]

if "shortcuts_preferences" in kwargs:
updated = kwargs["shortcuts_preferences"]
_validate_shortcuts_preferences(updated)
user.shortcuts_preferences = updated


def user_service_factory(_context, request):
"""Return a UserService instance for the passed context and request."""
return UserService(default_authority=request.default_authority, session=request.db)


def _validate_shortcuts_preferences(preferences):
if not isinstance(preferences, Mapping):
message = "shortcuts_preferences must be a mapping"
raise TypeError(message)

# Check for invalid keys
invalid_keys = set(preferences.keys()) - ALLOWED_SHORTCUT_ACTIONS
if invalid_keys:
keys = ", ".join(sorted(invalid_keys))
message = f"shortcuts_preferences with keys {keys} are not allowed"
raise TypeError(message)

# Check for duplicate shortcut values
actions_by_value = {}
for action, value in preferences.items():
actions_by_value.setdefault(value, set()).add(action)

for actions in actions_by_value.values():
if len(actions) <= 1:
continue
if _allow_duplicate_shortcuts(actions):
continue
actions_list = ", ".join(sorted(actions))
message = (
"shortcuts_preferences has duplicate shortcut values for actions "
f"{actions_list}"
)
raise TypeError(message)


def _allow_duplicate_shortcuts(actions):
return any(actions <= group for group in REPEATABLE_SHORTCUT_GROUPS)
2 changes: 2 additions & 0 deletions h/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ def _user_preferences(user):
preferences = {}
if user and not user.sidebar_tutorial_dismissed:
preferences["show_sidebar_tutorial"] = True
if user and user.shortcuts_preferences is not None:
preferences["shortcuts_preferences"] = user.shortcuts_preferences
return preferences


Expand Down
3 changes: 3 additions & 0 deletions tests/unit/h/models/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ def test_privacy_accepted_defaults_to_None(self):
# nullable
assert User().privacy_accepted is None

def test_shortcuts_preferences_defaults_to_None(self):
assert User().shortcuts_preferences is None

def test_repr(self, user):
assert repr(user) == f"User(id={user.id})"

Expand Down
77 changes: 77 additions & 0 deletions tests/unit/h/services/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,83 @@ def test_update_preferences_raises_for_unsupported_keys(self, svc, factories):

assert "keys baz, foo are not allowed" in str(exc.value)

def test_update_preferences_rejects_shortcuts_preferences_non_mapping(
self, svc, factories
):
user = factories.User.build()

with pytest.raises(TypeError) as exc:
svc.update_preferences(user, shortcuts_preferences="qux")

assert "shortcuts_preferences must be a mapping" in str(exc.value)

def test_update_preferences_rejects_shortcuts_preferences_invalid_keys(
self, svc, factories
):
user = factories.User.build()

with pytest.raises(TypeError) as exc:
svc.update_preferences(user, shortcuts_preferences={"notAllowed": "k"})

assert "keys notAllowed are not allowed" in str(exc.value)

def test_update_preferences_rejects_shortcut_value_duplicates(self, svc, factories):
user = factories.User.build()

with pytest.raises(TypeError) as exc:
svc.update_preferences(
user,
shortcuts_preferences={
"applyUpdates": "l",
"openKeyboardShortcuts": "l",
},
)

assert "duplicate shortcut values" in str(exc.value)

def test_update_preferences_allows_repeatable_shortcut_duplicates(
self, svc, factories, monkeypatch
):
user = factories.User.build()

monkeypatch.setattr(
"h.services.user.REPEATABLE_SHORTCUT_GROUPS",
[{"applyUpdates", "openKeyboardShortcuts"}],
)

svc.update_preferences(
user,
shortcuts_preferences={
"applyUpdates": "l",
"openKeyboardShortcuts": "l",
},
)

assert user.shortcuts_preferences == {
"applyUpdates": "l",
"openKeyboardShortcuts": "l",
}

def test_update_preferences_accepts_valid_shortcuts_preferences(
self, svc, factories
):
user = factories.User.build()

svc.update_preferences(
user,
shortcuts_preferences={
"openSearch": "k",
"applyUpdates": "l",
"openKeyboardShortcuts": "?",
},
)

assert user.shortcuts_preferences == {
"openSearch": "k",
"applyUpdates": "l",
"openKeyboardShortcuts": "?",
}

def test_sets_up_cache_clearing_on_transaction_end(self, patch, db_session):
decorator = patch("h.services.user.on_transaction_end")

Expand Down
16 changes: 16 additions & 0 deletions tests/unit/h/session_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ def test_authenticated_sidebar_tutorial(self, authenticated_request, dismissed):
else:
assert preferences["show_sidebar_tutorial"] is True

def test_authenticated_includes_shortcuts_preferences(self, authenticated_request):
shortcuts_preferences = {"applyUpdates": "l"}
authenticated_request.user.shortcuts_preferences = shortcuts_preferences

preferences = session.model(authenticated_request)["preferences"]

assert preferences["shortcuts_preferences"] == shortcuts_preferences


class TestProfile:
def test_userid_unauthenticated(self, unauthenticated_request):
Expand Down Expand Up @@ -166,6 +174,14 @@ def test_authenticated_sidebar_tutorial(self, authenticated_request, dismissed):
else:
assert preferences["show_sidebar_tutorial"] is True

def test_authenticated_includes_shortcuts_preferences(self, authenticated_request):
shortcuts_preferences = {"applyUpdates": "l"}
authenticated_request.user.shortcuts_preferences = shortcuts_preferences

preferences = session.profile(authenticated_request)["preferences"]

assert preferences["shortcuts_preferences"] == shortcuts_preferences

def test_anonymous_authority(self, unauthenticated_request, authority):
assert session.profile(unauthenticated_request)["authority"] == authority

Expand Down
Loading