model: Update role_id column

 * fix role instantiation
 * closes inveniosoftware/invenio-app-rdm#2186

Co-authored-by: jrcastro2 <[email protected]>

invenio_access/__init__.py

@@ -42,9 +42,12 @@
 >>> from invenio_db import InvenioDB
 >>> from invenio_accounts import InvenioAccounts
 >>> from invenio_access import InvenioAccess
+>>> from invenio_i18n import InvenioI18N
+
 >>> ext_db = InvenioDB(app)
 >>> ext_accounts = InvenioAccounts(app)
 >>> ext_access = InvenioAccess(app)
+>>> ext_i18n = InvenioI18N(app)
 
 The following examples needs to run in a Flask application context, so
 let's push one:

invenio_access/alembic/842a62b56e60_change_fk_accountsrole_to_string_downgrade.py

@@ -0,0 +1,40 @@
+# This file is part of Invenio.
+# Copyright (C) 2023 CERN.
+#
+# Invenio is free software; you can redistribute it and/or modify it
+# under the terms of the MIT License; see LICENSE file for more details.
+
+"""Change FK AccountsRole to string (downgrade recipe)."""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "842a62b56e60"
+down_revision = "04480be1593e"
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    """Upgrade database."""
+    pass
+
+
+def downgrade():
+    """Downgrade database."""
+    op.alter_column(
+        "access_actionsroles",
+        "role_id",
+        existing_type=sa.String(80),
+        type_=sa.Integer,
+        postgresql_using="role_id::integer",
+    )
+    op.create_foreign_key(
+        op.f("fk_access_actionsroles_role_id_accounts_role"),
+        "access_actionsroles",
+        "accounts_role",
+        ["role_id"],
+        ["id"],
+        ondelete="CASCADE",
+    )

invenio_access/alembic/f9843093f686_change_fk_accountsrole_to_string_upgrade.py

@@ -0,0 +1,50 @@
+#
+# This file is part of Invenio.
+# Copyright (C) 2023 CERN.
+#
+# Invenio is free software; you can redistribute it and/or modify it
+# under the terms of the MIT License; see LICENSE file for more details.
+
+"""Change FK AccountsRole to string (upgrade recipe).
+
+This recipe only contains the upgrade because as it directly depends on invenio-accounts recipe. That recipe is in
+charge of deleting all the constraints on the role_id, including foreign keys using the role_id declared in this module.
+Therefore, when in order to downgrade we need to split the recipes to be able to first execute the recipe in
+invenio-accounts (f2522cdd5fcd) and after that we can execute the downgrade recipe (842a62b56e60).
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "f9843093f686"
+down_revision = (
+    "f2522cdd5fcd",
+    "842a62b56e60",
+)  # Depends on invenio-accounts revision id (f2522cdd5fcd)
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    """Upgrade database."""
+    op.alter_column(
+        "access_actionsroles",
+        "role_id",
+        existing_type=sa.Integer,
+        type_=sa.String(80),
+        postgresql_using="role_id::integer",
+    )
+    op.create_foreign_key(
+        op.f("fk_access_actionsroles_role_id_accounts_role"),
+        "access_actionsroles",
+        "accounts_role",
+        ["role_id"],
+        ["id"],
+        ondelete="CASCADE",
+    )
+
+
+def downgrade():
+    """Downgrade database."""
+    pass

invenio_access/models.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # This file is part of Invenio.
-# Copyright (C) 2015-2018 CERN.
+# Copyright (C) 2015-2023 CERN.
 #
 # Invenio is free software; you can redistribute it and/or modify it
 # under the terms of the MIT License; see LICENSE file for more details.
@@ -156,7 +156,7 @@ class ActionRoles(ActionNeedMixin, db.Model):
     )
 
     role_id = db.Column(
-        db.Integer(),
+        db.String(80),
         db.ForeignKey(Role.id, ondelete="CASCADE"),
         nullable=False,
         index=True,

run-tests.sh

@@ -18,12 +18,35 @@ set -o nounset
 function cleanup() {
     eval "$(docker-services-cli down --env)"
 }
-trap cleanup EXIT
+
+# Check for arguments
+# Note: "-k" would clash with "pytest"
+keep_services=0
+pytest_args=()
+for arg in $@; do
+	# from the CLI args, filter out some known values and forward the rest to "pytest"
+	# note: we don't use "getopts" here b/c of some limitations (e.g. long options),
+	#       which means that we can't combine short options (e.g. "./run-tests -Kk pattern")
+	case ${arg} in
+		-K|--keep-services)
+			keep_services=1
+			;;
+		*)
+			pytest_args+=( ${arg} )
+			;;
+	esac
+done
+
+if [[ ${keep_services} -eq 0 ]]; then
+	trap cleanup EXIT
+fi
 
 python -m check_manifest
 python -m setup extract_messages --dry-run
 python -m sphinx.cmd.build -qnNW docs docs/_build/html
 eval "$(docker-services-cli up --db ${DB:-postgresql} --cache ${CACHE:-redis} --env)"
-python -m pytest
+# Note: expansion of pytest_args looks like below to not cause an unbound
+# variable error when 1) "nounset" and 2) the array is empty.
+python -m pytest ${pytest_args[@]+"${pytest_args[@]}"}
 tests_exit_code=$?
 exit "$tests_exit_code"

setup.cfg

@@ -28,7 +28,7 @@ python_requires = >=3.7
 zip_safe = False
 install_requires =
     invenio-admin>=1.2.0
-    invenio-accounts>=1.4.13
+    invenio-accounts>=2.2.0
     invenio-base>=1.2.11
     invenio-i18n>=2.0.0
 
@@ -37,7 +37,7 @@ tests =
     pytest-black>=0.3.0,<0.3.10
     cachelib>=0.1
     pytest-invenio>=1.4.1
-    invenio-db[mysql, postgresql]>=1.0.14
+    invenio-db[mysql, postgresql]>=1.1.2
     redis>=2.10.5
     sphinx>=4.5
 # Kept for backwards compatibility

tests/test_loaders.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # This file is part of Invenio.
-# Copyright (C) 2017-2018 CERN.
+# Copyright (C) 2017-2023 CERN.
 #
 # Invenio is free software; you can redistribute it and/or modify it
 # under the terms of the MIT License; see LICENSE file for more details.
@@ -26,40 +26,37 @@
 def test_load_permissions_on_request_loaded(app):
     """Checks that the needs are loaded during request in the user Identity."""
     InvenioAccess(app)
-    with app.test_request_context():
-        with app.test_client() as client:
-            client.get("/")
-            assert g.identity.provides == {any_user}
+    with app.test_client() as client:
+        client.get("/")
+        assert g.identity.provides == {any_user}
 
 
 def test_load_permissions_on_identity_loaded(app):
     """Check that the needs are loaded properly in the user Identity."""
     InvenioAccess(app)
 
-    with app.test_request_context():
-        identity_changed.send(
-            current_app._get_current_object(), identity=AnonymousIdentity()
-        )
-        assert g.identity.provides == {any_user}
+    identity_changed.send(
+        current_app._get_current_object(), identity=AnonymousIdentity()
+    )
+    assert g.identity.provides == {any_user}
 
-    with app.test_request_context():
-        user = testutils.create_test_user("[email protected]")
-        login_user(user)
-        assert g.identity.provides == {any_user, authenticated_user, UserNeed(user.id)}
-        logout_user()
-        # FIXME: The user is still authenticatd when the identity loader
-        # is called during logout. We could filter on AnonymousIdentity, but
-        # This would be inconsistent as the UserNeed(user.id) is still there.
-        # This will pass even if it is unexpected:
-        # assert g.identity.provides == {
-        #     any_user, authenticated_user, UserNeed(user.id)
-        # }
-        # Forcing the identity to reload again cleans the mess. In practice
-        # this won't be needed as the identity is reloaded between requests.
-        identity_changed.send(
-            current_app._get_current_object(), identity=AnonymousIdentity()
-        )
-        assert g.identity.provides == {any_user}
+    user = testutils.create_test_user("[email protected]")
+    login_user(user)
+    assert g.identity.provides == {any_user, authenticated_user, UserNeed(user.id)}
+    logout_user()
+    # FIXME: The user is still authenticatd when the identity loader
+    # is called during logout. We could filter on AnonymousIdentity, but
+    # This would be inconsistent as the UserNeed(user.id) is still there.
+    # This will pass even if it is unexpected:
+    # assert g.identity.provides == {
+    #     any_user, authenticated_user, UserNeed(user.id)
+    # }
+    # Forcing the identity to reload again cleans the mess. In practice
+    # this won't be needed as the identity is reloaded between requests.
+    identity_changed.send(
+        current_app._get_current_object(), identity=AnonymousIdentity()
+    )
+    assert g.identity.provides == {any_user}
 
 
 def test_disabled_loader(app):
@@ -69,8 +66,7 @@ def test_disabled_loader(app):
 
     assert load_permissions_on_identity_loaded not in identity_loaded.receivers.values()
 
-    with app.test_request_context():
-        identity_changed.send(
-            current_app._get_current_object(), identity=AnonymousIdentity()
-        )
-        assert g.identity.provides == set()
+    identity_changed.send(
+        current_app._get_current_object(), identity=AnonymousIdentity()
+    )
+    assert g.identity.provides == set()

tests/test_permissions.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # This file is part of Invenio.
-# Copyright (C) 2015-2019 CERN.
+# Copyright (C) 2015-2023 CERN.
 #
 # Invenio is free software; you can redistribute it and/or modify it
 # under the terms of the MIT License; see LICENSE file for more details.
@@ -84,7 +84,7 @@ def create_roles(*names):
     """Helper to create roles."""
     roles = []
     for name in names:
-        role = Role(name=name)
+        role = Role(id=name, name=name)
         db.session.add(role)
         roles.append(role)
     db.session.commit()