feat: add new pre-registration columns and update related user handling

Author: pcrespov

packages/postgres-database/src/simcore_postgres_database/migration/versions/ba9c4816a31b_new_pre_registration_columns.py

@@ -0,0 +1,107 @@
+"""new pre-registration columns
+
+Revision ID: ba9c4816a31b
+Revises: b39f2dc87ccd
+Create Date: 2025-05-19 15:21:40.182354+00:00
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "ba9c4816a31b"
+down_revision = "b39f2dc87ccd"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    # Create the enum type first before using it
+    account_request_status = sa.Enum(
+        "PENDING", "APPROVED", "REJECTED", name="accountrequeststatus"
+    )
+    account_request_status.create(op.get_bind(), checkfirst=True)
+
+    op.add_column(
+        "users_pre_registration_details",
+        sa.Column(
+            "id",
+            sa.BigInteger(),
+            sa.Identity(always=False, start=1, cycle=False),
+            nullable=False,
+        ),
+    )
+    op.add_column(
+        "users_pre_registration_details",
+        sa.Column(
+            "account_request_status",
+            account_request_status,  # Use the created enum type
+            server_default="PENDING",  # Simply use the string value as default
+            nullable=False,
+        ),
+    )
+    op.add_column(
+        "users_pre_registration_details",
+        sa.Column("product_name", sa.String(), nullable=True),
+    )
+    op.drop_constraint(
+        "users_pre_registration_details_pre_email_key",
+        "users_pre_registration_details",
+        type_="unique",
+    )
+    op.create_foreign_key(
+        "fk_users_pre_registration_details_product_name",
+        "users_pre_registration_details",
+        "products",
+        ["product_name"],
+        ["name"],
+        onupdate="CASCADE",
+        ondelete="SET NULL",
+    )
+    # Set primary key on id column
+    op.create_primary_key(
+        "users_pre_registration_details_pk",
+        "users_pre_registration_details",
+        ["id"],
+    )
+    # Add composite unique constraint on pre_email and product_name
+    op.create_unique_constraint(
+        "users_pre_registration_details_pre_email_product_name_key",
+        "users_pre_registration_details",
+        ["pre_email", "product_name"],
+    )
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    # Drop the composite unique constraint
+    op.drop_constraint(
+        "users_pre_registration_details_pre_email_product_name_key",
+        "users_pre_registration_details",
+        type_="unique",
+    )
+    op.drop_constraint(
+        "users_pre_registration_details_pk",
+        "users_pre_registration_details",
+        type_="primary",
+    )
+    op.drop_constraint(
+        "fk_users_pre_registration_details_product_name",
+        "users_pre_registration_details",
+        type_="foreignkey",
+    )
+    op.create_unique_constraint(
+        "users_pre_registration_details_pre_email_key",
+        "users_pre_registration_details",
+        ["pre_email"],
+    )
+    op.drop_column("users_pre_registration_details", "product_name")
+    op.drop_column("users_pre_registration_details", "account_request_status")
+    op.drop_column("users_pre_registration_details", "id")
+
+    # Drop the enum type in downgrade
+    sa.Enum(name="accountrequeststatus").drop(op.get_bind(), checkfirst=True)
+    # ### end Alembic commands ###

packages/postgres-database/src/simcore_postgres_database/models/users_details.py

@@ -44,7 +44,6 @@
         "pre_email",
         sa.String(),
         nullable=False,
-        unique=False,  # Same email could be used for different products
         doc="Email of the user on pre-registration (copied to users.email upon registration)",
     ),
     sa.Column(
@@ -68,7 +67,8 @@
         "account_request_status",
         sa.Enum(AccountRequestStatus),
         nullable=False,
-        server_default=sa.text("'PENDING'::account_request_status"),
+        server_default=AccountRequestStatus.PENDING.value,
+        doc="Status of approval of the account request",
     ),
     # Product the user is requesting access to
     sa.Column(
@@ -100,6 +100,13 @@
     column_created_by_user(users_table=users, required=False),
     column_created_datetime(timezone=False),
     column_modified_datetime(timezone=False),
+    # CONSTRAINTS:
+    # Composite unique constraint to ensure a user can only have one pre-registration per product
+    sa.UniqueConstraint(
+        "pre_email",
+        "product_name",
+        name="users_pre_registration_details_pre_email_product_name_key",
+    ),
 )
 
 register_modified_datetime_auto_update_trigger(users_pre_registration_details)

packages/postgres-database/src/simcore_postgres_database/utils_users.py

@@ -88,15 +88,15 @@ async def new_user(
                 users.c.status,
             ).where(users.c.id == user_id)
         )
-        row = await maybe_await(result.first())
-        from aiopg.sa.result import RowProxy
-
-        assert isinstance(row, RowProxy)  # nosec
-        return row
+        return await maybe_await(result.first())
 
     @staticmethod
-    async def join_and_update_from_pre_registration_details(
-        conn: DBConnection, new_user_id: int, new_user_email: str
+    async def link_and_update_user_from_pre_registration(
+        conn: DBConnection,
+        *,
+        new_user_id: int,
+        new_user_email: str,
+        update_user: bool = True,
     ) -> None:
         """After a user is created, it can be associated with information provided during invitation
 
@@ -113,11 +113,8 @@ async def join_and_update_from_pre_registration_details(
             .values(user_id=new_user_id)
         )
 
-        from aiopg.sa.result import ResultProxy
-
-        assert isinstance(result, ResultProxy)  # nosec
-
-        if result.rowcount:
+        if update_user:
+            # COPIES some pre-registration details to the users table
             pre_columns = (
                 users_pre_registration_details.c.pre_first_name,
                 users_pre_registration_details.c.pre_last_name,
@@ -141,7 +138,7 @@ async def join_and_update_from_pre_registration_details(
                     users_pre_registration_details.c.pre_email == new_user_email
                 )
             )
-            if details := await maybe_await(result.fetchone()):
+            if details := result.first():
                 await conn.execute(
                     users.update()
                     .where(users.c.id == new_user_id)

packages/postgres-database/tests/test_users_details.py

@@ -3,75 +3,104 @@
 # pylint: disable=unused-variable
 # pylint: disable=too-many-arguments
 
+from collections.abc import AsyncIterable
 from dataclasses import dataclass
+from typing import Any
 
 import pytest
 import sqlalchemy as sa
-from aiopg.sa.connection import SAConnection
 from aiopg.sa.result import RowProxy
 from faker import Faker
 from pytest_simcore.helpers.faker_factories import (
     random_pre_registration_details,
+    random_product,
     random_user,
 )
+from pytest_simcore.helpers.postgres_tools import (
+    insert_and_get_row_lifespan,
+)
+from simcore_postgres_database.models.products import products
 from simcore_postgres_database.models.users import UserRole, UserStatus, users
 from simcore_postgres_database.models.users_details import (
     users_pre_registration_details,
 )
+from simcore_postgres_database.utils_repos import (
+    pass_or_acquire_connection,
+    transaction_context,
+)
 from simcore_postgres_database.utils_users import UsersRepo
+from sqlalchemy.ext.asyncio import AsyncEngine
 
 
 @pytest.fixture
-async def po_user(
+async def product_name(
     faker: Faker,
-    connection: SAConnection,
-):
-    user_id = await connection.scalar(
-        users.insert()
-        .values(**random_user(faker, role=UserRole.PRODUCT_OWNER))
-        .returning(users.c.id)
-    )
-    assert user_id
+    asyncpg_engine: AsyncEngine,
+) -> AsyncIterable[str]:
+    async with insert_and_get_row_lifespan(  # pylint:disable=contextmanager-generator-missing-cleanup
+        asyncpg_engine,
+        table=products,
+        values=random_product(fake=faker, name="s4l"),
+        pk_col=products.c.name,
+    ) as row:
+        yield row["name"]
 
-    result = await connection.execute(sa.select(users).where(users.c.id == user_id))
-    yield await result.first()
 
-    users.delete().where(users.c.id == user_id)
+@pytest.fixture
+async def po_user(
+    faker: Faker,
+    asyncpg_engine: AsyncEngine,
+) -> AsyncIterable[dict[str, Any]]:
+    async with insert_and_get_row_lifespan(  # pylint:disable=contextmanager-generator-missing-cleanup
+        asyncpg_engine,
+        table=users,
+        values=random_user(faker, role=UserRole.PRODUCT_OWNER),
+        pk_col=users.c.id,
+    ) as row:
+        yield row
 
 
 @pytest.mark.acceptance_test(
     "pre-registration in https://github.com/ITISFoundation/osparc-simcore/issues/5138"
 )
 async def test_user_creation_workflow(
-    connection: SAConnection, faker: Faker, po_user: RowProxy
+    asyncpg_engine: AsyncEngine,
+    faker: Faker,
+    po_user: dict[str, Any],
+    product_name: str,
 ):
     # a PO creates an invitation
     fake_pre_registration_data = random_pre_registration_details(
-        faker, created_by=po_user.id
+        faker, created_by=po_user["id"], product_name=product_name
     )
 
-    pre_email = await connection.scalar(
-        sa.insert(users_pre_registration_details)
-        .values(**fake_pre_registration_data)
-        .returning(users_pre_registration_details.c.pre_email)
-    )
+    async with transaction_context(asyncpg_engine) as connection:
+        pre_email = await connection.scalar(
+            sa.insert(users_pre_registration_details)
+            .values(**fake_pre_registration_data)
+            .returning(users_pre_registration_details.c.pre_email)
+        )
     assert pre_email is not None
     assert pre_email == fake_pre_registration_data["pre_email"]
 
-    # user gets created
-    new_user = await UsersRepo.new_user(
-        connection,
-        email=pre_email,
-        password_hash="123456",  # noqa: S106
-        status=UserStatus.ACTIVE,
-        expires_at=None,
-    )
-    await UsersRepo.join_and_update_from_pre_registration_details(
-        connection, new_user.id, new_user.email
-    )
+    async with transaction_context(asyncpg_engine) as connection:
+        # user gets created
+        new_user = await UsersRepo.new_user(
+            connection,
+            email=pre_email,
+            password_hash="123456",  # noqa: S106
+            status=UserStatus.ACTIVE,
+            expires_at=None,
+        )
+        await UsersRepo.link_and_update_user_from_pre_registration(
+            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        )
 
-    invoice_data = await UsersRepo.get_billing_details(connection, user_id=new_user.id)
-    assert invoice_data is not None
+    async with pass_or_acquire_connection(asyncpg_engine) as connection:
+        invoice_data = await UsersRepo.get_billing_details(
+            connection, user_id=new_user.id
+        )
+        assert invoice_data is not None
 
     # drafts converting data models from https://github.com/ITISFoundation/osparc-simcore/pull/5402
     @dataclass
@@ -84,7 +113,11 @@ class UserAddress:
 
         @classmethod
         def create_from_db(cls, row: RowProxy):
-            parts = (row[c] for c in ("institution", "address") if row[c])
+            parts = (
+                getattr(row, col_name)
+                for col_name in ("institution", "address")
+                if getattr(row, col_name)
+            )
             return cls(
                 line1=". ".join(parts),
                 state=row.state,
@@ -110,28 +143,30 @@ def create_from_db(cls, row: RowProxy):
     assert user_address.country == fake_pre_registration_data["country"]
 
     # now let's update the user
-    result = await connection.execute(
-        users.update()
-        .values(first_name="My New Name")
-        .where(users.c.id == new_user.id)
-        .returning("*")
-    )
-    updated_user = await result.fetchone()
+    async with transaction_context(asyncpg_engine) as connection:
+        result = await connection.execute(
+            users.update()
+            .values(first_name="My New Name")
+            .where(users.c.id == new_user.id)
+            .returning("*")
+        )
+        updated_user = result.one()
 
     assert updated_user
     assert updated_user.first_name == "My New Name"
     assert updated_user.id == new_user.id
 
     for _ in range(2):
-        await UsersRepo.join_and_update_from_pre_registration_details(
-            connection, new_user.id, new_user.email
-        )
+        async with transaction_context(asyncpg_engine) as connection:
+            await UsersRepo.link_and_update_user_from_pre_registration(
+                connection, new_user_id=new_user.id, new_user_email=new_user.email
+            )
 
-        result = await connection.execute(
-            users.select().where(users.c.id == new_user.id)
-        )
-        current_user = await result.fetchone()
-        assert current_user
+            result = await connection.execute(
+                users.select().where(users.c.id == new_user.id)
+            )
+            current_user = result.one()
+            assert current_user
 
-        # overriden!
-        assert current_user.first_name != updated_user.first_name
+            # overriden!
+            assert current_user.first_name != updated_user.first_name