From ede2972029c475aab1cc5c4a0db96fdfd168af27 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 19:03:53 +0200 Subject: [PATCH 01/11] =?UTF-8?q?=E2=9C=A8=20Add=20AccountRequestStatus=20?= =?UTF-8?q?enum=20to=20manage=20account=20request=20states?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/common-library/src/common_library/users_enums.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/common-library/src/common_library/users_enums.py b/packages/common-library/src/common_library/users_enums.py index 7ebe4a617e9..4a0606bf46e 100644 --- a/packages/common-library/src/common_library/users_enums.py +++ b/packages/common-library/src/common_library/users_enums.py @@ -57,3 +57,11 @@ class UserStatus(str, Enum): BANNED = "BANNED" # This user is inactive because it was marked for deletion DELETED = "DELETED" + + +class AccountRequestStatus(str, Enum): + """Status of the request for an account""" + + PENDING = "PENDING" # Pending PO review to approve/reject the request + APPROVED = "APPROVED" # PO approved the request + REJECTED = "REJECTED" # PO rejected the request From 74be609f5b0c6ab64f6c4938486a774339eb91c2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 19:04:43 +0200 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=90=9B=20Fix=20pk=5Fvalue=20handlin?= =?UTF-8?q?g=20in=20insert=5Fand=5Fget=5Frow=20functions=20for=20better=20?= =?UTF-8?q?compatibility=20with=20SQLAlchemy=202.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../pytest_simcore/helpers/postgres_tools.py | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py b/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py index b3a01381b7a..1e854e8b687 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py @@ -89,13 +89,17 @@ async def _async_insert_and_get_row( table: sa.Table, values: dict[str, Any], pk_col: sa.Column, - pk_value: Any, -): + pk_value: Any | None = None, +) -> sa.engine.Row: result = await conn.execute(table.insert().values(**values).returning(pk_col)) row = result.one() - # NOTE: DO NO USE row[pk_col] since you will get a deprecation error (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) - assert getattr(row, pk_col.name) == pk_value + # Get the pk_value from the row if not provided + if pk_value is None: + pk_value = getattr(row, pk_col.name) + else: + # NOTE: DO NO USE row[pk_col] since you will get a deprecation error (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) + assert getattr(row, pk_col.name) == pk_value result = await conn.execute(sa.select(table).where(pk_col == pk_value)) return result.one() @@ -106,13 +110,17 @@ def _sync_insert_and_get_row( table: sa.Table, values: dict[str, Any], pk_col: sa.Column, - pk_value: Any, -): + pk_value: Any | None = None, +) -> sa.engine.Row: result = conn.execute(table.insert().values(**values).returning(pk_col)) row = result.one() - # NOTE: DO NO USE row[pk_col] since you will get a deprecation error (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) - assert getattr(row, pk_col.name) == pk_value + # Get the pk_value from the row if not provided + if pk_value is None: + pk_value = getattr(row, pk_col.name) + else: + # NOTE: DO NO USE row[pk_col] since you will get a deprecation error (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) + assert getattr(row, pk_col.name) == pk_value result = conn.execute(sa.select(table).where(pk_col == pk_value)) return result.one() @@ -125,13 +133,16 @@ async def insert_and_get_row_lifespan( table: sa.Table, values: dict[str, Any], pk_col: sa.Column, - pk_value: Any, + pk_value: Any | None = None, ) -> AsyncIterator[dict[str, Any]]: - # insert & get + # SETUP: insert & get async with sqlalchemy_async_engine.begin() as conn: row = await _async_insert_and_get_row( conn, table=table, values=values, pk_col=pk_col, pk_value=pk_value ) + # If pk_value was None, get it from the row for deletion later + if pk_value is None: + pk_value = getattr(row, pk_col.name) assert row @@ -139,7 +150,7 @@ async def insert_and_get_row_lifespan( # pylint: disable=protected-access yield row._asdict() - # delete row + # TEAD-DOWN: delete row async with sqlalchemy_async_engine.begin() as conn: await conn.execute(table.delete().where(pk_col == pk_value)) @@ -151,7 +162,7 @@ def sync_insert_and_get_row_lifespan( table: sa.Table, values: dict[str, Any], pk_col: sa.Column, - pk_value: Any, + pk_value: Any | None = None, ) -> Iterator[dict[str, Any]]: """sync version of insert_and_get_row_lifespan. @@ -159,11 +170,14 @@ def sync_insert_and_get_row_lifespan( database tables before the app starts since it does not require an `event_loop` fixture (which is funcition-scoped ) """ - # insert & get + # SETUP: insert & get with sqlalchemy_sync_engine.begin() as conn: row = _sync_insert_and_get_row( conn, table=table, values=values, pk_col=pk_col, pk_value=pk_value ) + # If pk_value was None, get it from the row for deletion later + if pk_value is None: + pk_value = getattr(row, pk_col.name) assert row @@ -171,6 +185,6 @@ def sync_insert_and_get_row_lifespan( # pylint: disable=protected-access yield row._asdict() - # delete row + # TEARDOWN: delete row with sqlalchemy_sync_engine.begin() as conn: conn.execute(table.delete().where(pk_col == pk_value)) From 6f32644de88df6b3eeabb5114d7e75051316870f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 19:04:56 +0200 Subject: [PATCH 03/11] =?UTF-8?q?=E2=9C=A8=20Add=20pre-registration=20colu?= =?UTF-8?q?mns=20and=20update=20user=20linking=20logic=20for=20account=20r?= =?UTF-8?q?equests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...9c4816a31b_new_pre_registration_columns.py | 140 +++++ .../models/users_details.py | 69 ++- .../simcore_postgres_database/utils_users.py | 28 +- .../tests/test_users_details.py | 536 +++++++++++++++--- .../pytest_simcore/helpers/faker_factories.py | 5 + 5 files changed, 674 insertions(+), 104 deletions(-) create mode 100644 packages/postgres-database/src/simcore_postgres_database/migration/versions/ba9c4816a31b_new_pre_registration_columns.py diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/ba9c4816a31b_new_pre_registration_columns.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/ba9c4816a31b_new_pre_registration_columns.py new file mode 100644 index 00000000000..6bf5169538c --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/ba9c4816a31b_new_pre_registration_columns.py @@ -0,0 +1,140 @@ +"""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( + "account_request_reviewed_by", + sa.Integer(), + nullable=True, + ), + ) + op.add_column( + "users_pre_registration_details", + sa.Column( + "account_request_reviewed_at", + sa.DateTime(timezone=True), + nullable=True, + ), + ) + 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", + ) + # Add foreign key for account_request_reviewed_by + op.create_foreign_key( + "fk_users_pre_registration_reviewed_by_user_id", + "users_pre_registration_details", + "users", + ["account_request_reviewed_by"], + ["id"], + 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_reviewed_by_user_id", + "users_pre_registration_details", + type_="foreignkey", + ) + 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_reviewed_at") + op.drop_column("users_pre_registration_details", "account_request_reviewed_by") + 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 ### diff --git a/packages/postgres-database/src/simcore_postgres_database/models/users_details.py b/packages/postgres-database/src/simcore_postgres_database/models/users_details.py index 555e623dbdc..cf58af8187e 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/users_details.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/users_details.py @@ -1,13 +1,16 @@ import sqlalchemy as sa +from common_library.users_enums import AccountRequestStatus from sqlalchemy.dialects import postgresql from ._common import ( RefActions, + column_created_by_user, column_created_datetime, column_modified_datetime, register_modified_datetime_auto_update_trigger, ) from .base import metadata +from .products import products # Import the products table from .users import users users_pre_registration_details = sa.Table( @@ -18,6 +21,13 @@ # a row can be added in this table during pre-registration i.e. even before the `users` row exists. # metadata, + sa.Column( + "id", + sa.BigInteger, + sa.Identity(start=1, cycle=False), + primary_key=True, + doc="Primary key for the pre-registration entry", + ), sa.Column( "user_id", sa.Integer, @@ -34,7 +44,6 @@ "pre_email", sa.String(), nullable=False, - unique=True, doc="Email of the user on pre-registration (copied to users.email upon registration)", ), sa.Column( @@ -53,6 +62,45 @@ doc="Phone provided on pre-registration" "NOTE: this is not copied upon registration since it needs to be confirmed", ), + # Account Request + sa.Column( + "account_request_status", + sa.Enum(AccountRequestStatus), + nullable=False, + server_default=AccountRequestStatus.PENDING.value, + doc="Status of review for the account request", + ), + sa.Column( + "account_request_reviewed_by", + sa.Integer, + sa.ForeignKey( + users.c.id, + onupdate=RefActions.CASCADE, + ondelete=RefActions.SET_NULL, + name="fk_users_pre_registration_reviewed_by_user_id", + ), + nullable=True, + doc="Tracks who approved or rejected the account request", + ), + sa.Column( + "account_request_reviewed_at", + sa.DateTime(timezone=True), + nullable=True, + doc="Timestamp when the account request was reviewed", + ), + # Product the user is requesting access to + sa.Column( + "product_name", + sa.String, + sa.ForeignKey( + products.c.name, + onupdate=RefActions.CASCADE, + ondelete=RefActions.SET_NULL, + name="fk_users_pre_registration_details_product_name", + ), + nullable=True, + doc="Product that the user is requesting an account for", + ), # Billable address columns: sa.Column("institution", sa.String(), doc="the name of a company or university"), sa.Column("address", sa.String()), @@ -67,19 +115,16 @@ doc="Extra information provided in the form but still not defined as a column.", ), # Other related users - sa.Column( - "created_by", - sa.Integer, - sa.ForeignKey( - users.c.id, - onupdate=RefActions.CASCADE, - ondelete=RefActions.SET_NULL, - ), - nullable=True, - doc="PO user that issued this pre-registration", - ), + 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) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_users.py b/packages/postgres-database/src/simcore_postgres_database/utils_users.py index 29544e35b9c..587f90ee504 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_users.py +++ b/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,13 +138,14 @@ 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 pre_registration_details_data := result.first(): + # NOTE: could have many products! which to use? await conn.execute( users.update() .where(users.c.id == new_user_id) .values( - first_name=details.pre_first_name, # type: ignore[union-attr] - last_name=details.pre_last_name, # type: ignore[union-attr] + first_name=pre_registration_details_data.pre_first_name, # type: ignore[union-attr] + last_name=pre_registration_details_data.pre_last_name, # type: ignore[union-attr] ) ) diff --git a/packages/postgres-database/tests/test_users_details.py b/packages/postgres-database/tests/test_users_details.py index f99ab6396f0..e4b6bfeb70f 100644 --- a/packages/postgres-database/tests/test_users_details.py +++ b/packages/postgres-database/tests/test_users_details.py @@ -3,135 +3,517 @@ # pylint: disable=unused-variable # pylint: disable=too-many-arguments +from collections.abc import AsyncIterable, AsyncIterator +from contextlib import AsyncExitStack from dataclasses import dataclass +from typing import Any, Protocol, Self import pytest import sqlalchemy as sa -from aiopg.sa.connection import SAConnection -from aiopg.sa.result import RowProxy +from common_library.groups_enums import GroupType +from common_library.users_enums import AccountRequestStatus from faker import Faker from pytest_simcore.helpers.faker_factories import ( + random_group, 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.groups import groups +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 + + +class CreateProductCallable(Protocol): + """Callable that creates a product and returns its row.""" + + async def __call__(self, name: str) -> dict[str, Any]: ... @pytest.fixture -async def po_user( +async def product_factory( faker: Faker, - connection: SAConnection, -): - user_id = await connection.scalar( - users.insert() - .values(**random_user(faker, role=UserRole.PRODUCT_OWNER)) - .returning(users.c.id) + asyncpg_engine: AsyncEngine, +) -> AsyncIterator[CreateProductCallable]: + """Fixture that yields a factory function to create products. + + All products created with this factory will be automatically cleaned up when the test ends. + """ + async with AsyncExitStack() as exit_stack: + + async def _create_product(name: str) -> dict[str, Any]: + # 1. create a product group + product_group_row = await exit_stack.enter_async_context( + insert_and_get_row_lifespan( + asyncpg_engine, + table=groups, + values=random_group(fake=faker, type=GroupType.STANDARD.name), + pk_col=groups.c.gid, + ) + ) + + # 2. create the product using that group + product_name = name or faker.pystr(min_chars=3, max_chars=10) + return await exit_stack.enter_async_context( + insert_and_get_row_lifespan( + asyncpg_engine, + table=products, + values=random_product( + fake=faker, + name=product_name, + group_id=int(product_group_row["gid"]), + ), + pk_col=products.c.name, + ) + ) + + yield _create_product + + +@pytest.fixture +async def product(product_factory: CreateProductCallable) -> dict[str, Any]: + """Returns a single product for backward compatibility.""" + return await product_factory("s4l") + + +@pytest.fixture +async def product_owner_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, + email="po-user@email.com", + name="po-user-fixture", + role=UserRole.PRODUCT_OWNER, + ), + pk_col=users.c.id, + ) as row: + yield row + + +@dataclass +class UserAddress: + """Model for user address information from database records.""" + + line1: str | None + state: str | None + postal_code: str | None + city: str | None + country: str + + @classmethod + def create_from_db(cls, row) -> Self: + parts = ( + getattr(row, col_name) + for col_name in ("institution", "address") + if getattr(row, col_name) + ) + return cls( + line1=". ".join(parts), + state=row.state, + postal_code=row.postal_code, + city=row.city, + country=row.country, + ) + + +@pytest.fixture +async def pre_registered_user( + asyncpg_engine: AsyncEngine, + faker: Faker, + product_owner_user: dict[str, Any], + product: dict[str, Any], +) -> tuple[str, dict[str, Any]]: + """Creates a pre-registered user and returns the email and registration data.""" + product_name = product["name"] + fake_pre_registration_data = random_pre_registration_details( + faker, + pre_email="pre-registered@user.com", + created_by=product_owner_user["id"], + product_name=product_name, ) - assert user_id - result = await connection.execute(sa.select(users).where(users.c.id == user_id)) - yield await result.first() + 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) + ) - users.delete().where(users.c.id == user_id) + assert pre_email == fake_pre_registration_data["pre_email"] + return pre_email, fake_pre_registration_data + + +async def test_user_requests_account_and_is_approved( + asyncpg_engine: AsyncEngine, + faker: Faker, + product_owner_user: dict[str, Any], + product: dict[str, Any], +): + product_name = product["name"] + + # 1. User request an account + interested_user_email = "interested@user.com" + + async with transaction_context(asyncpg_engine) as connection: + pre_email = await connection.scalar( + sa.insert(users_pre_registration_details) + .values( + **random_pre_registration_details( + faker, + pre_email=interested_user_email, + product_name=product_name, + ) + ) + .returning(users_pre_registration_details.c.pre_email) + ) + assert pre_email is not None + assert pre_email == interested_user_email + + # 2. PO approves the account request + async with transaction_context(asyncpg_engine) as connection: + await connection.execute( + users_pre_registration_details.update() + .where(users_pre_registration_details.c.pre_email == pre_email) + .values( + account_request_status=AccountRequestStatus.APPROVED, + account_request_reviewed_by=product_owner_user["id"], + account_request_reviewed_at=sa.func.now(), + ) + ) + + # 3. Verify approval was recorded + async with pass_or_acquire_connection(asyncpg_engine) as connection: + result = await connection.execute( + sa.select( + users_pre_registration_details.c.account_request_status, + users_pre_registration_details.c.account_request_reviewed_by, + users_pre_registration_details.c.account_request_reviewed_at, + ).where(users_pre_registration_details.c.pre_email == pre_email) + ) + approval_record = result.one() + assert approval_record.account_request_status == AccountRequestStatus.APPROVED + assert approval_record.account_request_reviewed_by == product_owner_user["id"] + assert approval_record.account_request_reviewed_at is not None @pytest.mark.acceptance_test( - "pre-registration in https://github.com/ITISFoundation/osparc-simcore/issues/5138" + "pre-registration link creation in https://github.com/ITISFoundation/osparc-simcore/issues/5138" ) -async def test_user_creation_workflow( - connection: SAConnection, faker: Faker, po_user: RowProxy +async def test_create_pre_registration_link( + asyncpg_engine: AsyncEngine, + faker: Faker, + product_owner_user: dict[str, Any], + product: dict[str, Any], ): - # a PO creates an invitation + """Test that a PO can create a pre-registration link for a user.""" + product_name = product["name"] + + # PO creates a pre-registration and sends an email with the invitation link fake_pre_registration_data = random_pre_registration_details( - faker, created_by=po_user.id + faker, + pre_email="interested@user.com", + created_by=product_owner_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 - ) - 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 - class UserAddress: - line1: str | None - state: str | None - postal_code: str | None - city: str | None - country: str - - @classmethod - def create_from_db(cls, row: RowProxy): - parts = (row[c] for c in ("institution", "address") if row[c]) - return cls( - line1=". ".join(parts), - state=row.state, - postal_code=row.postal_code, - city=row.city, - country=row.country, +@pytest.mark.acceptance_test( + "pre-registration user creation in https://github.com/ITISFoundation/osparc-simcore/issues/5138" +) +async def test_create_and_link_user_from_pre_registration( + asyncpg_engine: AsyncEngine, + pre_registered_user: tuple[str, dict[str, Any]], +): + """Test that a user can be created from a pre-registration link and is linked properly.""" + pre_email, _ = pre_registered_user + + # Invitation link is clicked and the user is created and linked to the pre-registration + 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 + ) + + # Verify the user was created and linked + async with pass_or_acquire_connection(asyncpg_engine) as connection: + result = await connection.execute( + sa.select(users_pre_registration_details.c.user_id).where( + users_pre_registration_details.c.pre_email == pre_email ) + ) + user_id = result.scalar() + assert user_id == new_user.id - user_address = UserAddress.create_from_db(invoice_data) - # Expects something like - # { - # "line1": "Jones, Jefferson and Rivera. 5938 Ramos Pike Suite 080, Lake Marytown, RI 65195", - # "state": "Virginia", - # "postal_code": "08756", - # "city": "Johnmouth", - # "country": "Trinidad and Tobago" - # } +@pytest.mark.acceptance_test( + "pre-registration billing info in https://github.com/ITISFoundation/osparc-simcore/issues/5138" +) +async def test_get_billing_details_from_pre_registration( + asyncpg_engine: AsyncEngine, + pre_registered_user: tuple[str, dict[str, Any]], +): + """Test that billing details can be retrieved from pre-registration data.""" + pre_email, fake_pre_registration_data = pre_registered_user + + # Create the user + async with transaction_context(asyncpg_engine) as connection: + 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 + ) + + # Get billing details + 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 + + # Test UserAddress model conversion + user_address = UserAddress.create_from_db(invoice_data) + # Verify address fields match the pre-registration data assert user_address.line1 assert user_address.state == fake_pre_registration_data["state"] assert user_address.postal_code == fake_pre_registration_data["postal_code"] 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() + +@pytest.mark.acceptance_test( + "pre-registration user update in https://github.com/ITISFoundation/osparc-simcore/issues/5138" +) +async def test_update_user_from_pre_registration( + asyncpg_engine: AsyncEngine, + pre_registered_user: tuple[str, dict[str, Any]], +): + """Test that pre-registration details override manual updates when re-linking.""" + pre_email, _ = pre_registered_user + + # Create the user and link to pre-registration + async with transaction_context(asyncpg_engine) as connection: + 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 + ) + + # Update the user manually + 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 + # Re-link the user to pre-registration, which should override manual updates + 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() + current_user = result.one() assert current_user - # overriden! + # Verify that the manual updates were overridden assert current_user.first_name != updated_user.first_name + + +async def test_user_preregisters_for_multiple_products_with_different_outcomes( + asyncpg_engine: AsyncEngine, + faker: Faker, + product_owner_user: dict[str, Any], + product_factory: CreateProductCallable, +): + """Test scenario where a user pre-registers for multiple products and gets different approval outcomes.""" + # Create two products + product1 = await product_factory("s4l") + product2 = await product_factory("tip") + + # User email for pre-registration + user_email = "multi-product-user@example.com" + + # User pre-registers for both products + async with transaction_context(asyncpg_engine) as connection: + # Pre-register for product1 + await connection.execute( + sa.insert(users_pre_registration_details).values( + **random_pre_registration_details( + faker, + pre_email=user_email, + product_name=product1["name"], + ) + ) + ) + + # Pre-register for product2 + await connection.execute( + sa.insert(users_pre_registration_details).values( + **random_pre_registration_details( + faker, + pre_email=user_email, + product_name=product2["name"], + ) + ) + ) + + # Verify both pre-registrations were created + async with pass_or_acquire_connection(asyncpg_engine) as connection: + result = await connection.execute( + sa.select( + users_pre_registration_details.c.pre_email, + users_pre_registration_details.c.product_name, + users_pre_registration_details.c.account_request_status, + ) + .where(users_pre_registration_details.c.pre_email == user_email) + .order_by(users_pre_registration_details.c.product_name) + ) + + registrations = result.fetchall() + assert len(registrations) == 2 + assert all( + reg.account_request_status == AccountRequestStatus.PENDING + for reg in registrations + ) + + # 2. PO approves and rejects the requests + async with transaction_context(asyncpg_engine) as connection: + # PO approves the request for product1 + await connection.execute( + users_pre_registration_details.update() + .where( + (users_pre_registration_details.c.pre_email == user_email) + & (users_pre_registration_details.c.product_name == product1["name"]) + ) + .values( + account_request_status=AccountRequestStatus.APPROVED, + account_request_reviewed_by=product_owner_user["id"], + account_request_reviewed_at=sa.func.now(), + ) + ) + + # PO rejects the request for product2 + await connection.execute( + users_pre_registration_details.update() + .where( + (users_pre_registration_details.c.pre_email == user_email) + & (users_pre_registration_details.c.product_name == product2["name"]) + ) + .values( + account_request_status=AccountRequestStatus.REJECTED, + account_request_reviewed_by=product_owner_user["id"], + account_request_reviewed_at=sa.func.now(), + ) + ) + + # Verify the status updates + async with pass_or_acquire_connection(asyncpg_engine) as connection: + result = await connection.execute( + sa.select( + users_pre_registration_details.c.product_name, + users_pre_registration_details.c.account_request_status, + users_pre_registration_details.c.account_request_reviewed_by, + users_pre_registration_details.c.account_request_reviewed_at, + ) + .where(users_pre_registration_details.c.pre_email == user_email) + .order_by(users_pre_registration_details.c.created) + ) + + registrations = result.fetchall() + assert len(registrations) == 2 + + # Check product1 was approved + assert registrations[0].product_name == product1["name"] + assert registrations[0].account_request_status == AccountRequestStatus.APPROVED + assert registrations[0].account_request_reviewed_by == product_owner_user["id"] + assert registrations[0].account_request_reviewed_at is not None + + # Check product2 was rejected + assert registrations[1].product_name == product2["name"] + assert registrations[1].account_request_status == AccountRequestStatus.REJECTED + assert registrations[1].account_request_reviewed_by == product_owner_user["id"] + assert registrations[1].account_request_reviewed_at is not None + + # 3.Now create a user account with the approved pre-registration + async with transaction_context(asyncpg_engine) as connection: + new_user = await UsersRepo.new_user( + connection, + email=user_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 + ) + + # Verify both pre-registrations are linked to the new user + async with pass_or_acquire_connection(asyncpg_engine) as connection: + result = await connection.execute( + sa.select( + users_pre_registration_details.c.product_name, + users_pre_registration_details.c.account_request_status, + users_pre_registration_details.c.user_id, + ) + .where(users_pre_registration_details.c.pre_email == user_email) + .order_by(users_pre_registration_details.c.product_name) + ) + + registrations = result.fetchall() + assert len(registrations) == 2 + + # Both registrations should be linked to the same user, regardless of approval status + assert all(reg.user_id == new_user.id for reg in registrations) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py index 6ff20f1b072..faa36f5d61e 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py @@ -85,8 +85,11 @@ def random_user( def random_pre_registration_details( fake: Faker = DEFAULT_FAKER, *, + # foreign keys user_id: int | None = None, created_by: int | None = None, + product_name: str | None = None, + account_request_reviewed_by: int | None = None, **overrides, ): from simcore_postgres_database.models.users_details import ( @@ -117,7 +120,9 @@ def random_pre_registration_details( "eula": True, "ipinfo": {"x-real-ip": "127.0.0.1"}, }, + "product_name": product_name, "created_by": created_by, # user id + "account_request_reviewed_by": account_request_reviewed_by, } assert set(data.keys()).issubset( From 914a90b5e95586402e095becd60f676382055cb7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 19:19:29 +0200 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=90=9B=20Refactor=20user=20fixture?= =?UTF-8?q?=20to=20use=20asyncpg=20and=20improve=20database=20connection?= =?UTF-8?q?=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../tests/test_utils_users.py | 55 ++++++++++++------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/packages/postgres-database/tests/test_utils_users.py b/packages/postgres-database/tests/test_utils_users.py index eb01f85c605..d4a7039f1f3 100644 --- a/packages/postgres-database/tests/test_utils_users.py +++ b/packages/postgres-database/tests/test_utils_users.py @@ -4,35 +4,48 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable +from collections.abc import AsyncIterable from typing import Any import pytest -from aiopg.sa.connection import SAConnection from faker import Faker -from pytest_simcore.helpers.faker_factories import random_user +from pytest_simcore.helpers.faker_factories import ( + random_user, +) +from pytest_simcore.helpers.postgres_tools import ( + insert_and_get_row_lifespan, +) from simcore_postgres_database.models.users import UserRole, users +from simcore_postgres_database.utils_repos import ( + pass_or_acquire_connection, +) from simcore_postgres_database.utils_users import UserNotFoundInRepoError, UsersRepo -from sqlalchemy.ext.asyncio import AsyncConnection +from sqlalchemy.ext.asyncio import AsyncEngine @pytest.fixture -async def user(connection: SAConnection, faker: Faker) -> dict[str, Any]: - data = random_user(role=faker.random_element(elements=UserRole)) - user_id = await connection.scalar( - users.insert().values(**data).returning(users.c.id) - ) - assert user_id - data["id"] = user_id - return data - - -async def test_users_repo_get( - connection_factory: SAConnection | AsyncConnection, user: dict[str, Any] -): +async def 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=faker.random_element(elements=UserRole), + ), + pk_col=users.c.id, + ) as row: + yield row + + +async def test_users_repo_get(asyncpg_engine: AsyncEngine, user: dict[str, Any]): repo = UsersRepo() - # NOTE: Temporary usage of connection_factory until asyncpg is used - assert await repo.get_email(connection_factory, user_id=user["id"]) == user["email"] - assert await repo.get_role(connection_factory, user_id=user["id"]) == user["role"] - with pytest.raises(UserNotFoundInRepoError): - await repo.get_role(connection_factory, user_id=55) + async with pass_or_acquire_connection(asyncpg_engine) as connection: + assert await repo.get_email(connection, user_id=user["id"]) == user["email"] + assert await repo.get_role(connection, user_id=user["id"]) == user["role"] + + with pytest.raises(UserNotFoundInRepoError): + await repo.get_role(connection, user_id=55) From d31185a1ddbeb9b1cd5b6e6c91e8fe7f0951c1c5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 20:02:43 +0200 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=90=9B=20Refactor=20database=20conn?= =?UTF-8?q?ection=20handling=20in=20authentication=20and=20change=20email?= =?UTF-8?q?=20processes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../login/_auth_service.py | 24 +++++++++++-------- .../login/_controller/rest/change.py | 5 ++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/login/_auth_service.py b/services/web/server/src/simcore_service_webserver/login/_auth_service.py index a936f7f62f2..d439bf94337 100644 --- a/services/web/server/src/simcore_service_webserver/login/_auth_service.py +++ b/services/web/server/src/simcore_service_webserver/login/_auth_service.py @@ -4,12 +4,13 @@ from aiohttp import web from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from simcore_postgres_database.models.users import UserStatus +from simcore_postgres_database.utils_repos import transaction_context from simcore_postgres_database.utils_users import UsersRepo +from simcore_service_webserver.db.plugin import get_asyncpg_engine -from ..db.plugin import get_database_engine -from ..groups.api import is_user_by_email_in_group +from ..groups import api as groups_service from ..products.models import Product -from ..security.api import check_password, encrypt_password +from ..security import api as security_service from . import _login_service from ._constants import MSG_UNKNOWN_EMAIL, MSG_WRONG_PASSWORD from ._login_repository_legacy import AsyncpgStorage, get_plugin_storage @@ -30,16 +31,16 @@ async def create_user( expires_at: datetime | None, ) -> dict[str, Any]: - async with get_database_engine(app).acquire() as conn: + async with transaction_context(get_asyncpg_engine(app)) as conn: user = await UsersRepo.new_user( conn, email=email, - password_hash=encrypt_password(password), + password_hash=security_service.encrypt_password(password), status=status_upon_creation, expires_at=expires_at, ) - await UsersRepo.join_and_update_from_pre_registration_details( - conn, user.id, user.email + await UsersRepo.link_and_update_user_from_pre_registration( + conn, new_user_id=user.id, new_user_email=user.email ) return dict(user.items()) @@ -57,7 +58,7 @@ async def check_authorized_user_credentials_or_raise( _login_service.validate_user_status(user=user, support_email=product.support_email) - if not check_password(password, user["password_hash"]): + if not security_service.check_password(password, user["password_hash"]): raise web.HTTPUnauthorized( reason=MSG_WRONG_PASSWORD, content_type=MIMETYPE_APPLICATION_JSON ) @@ -75,8 +76,11 @@ async def check_authorized_user_in_product_or_raise( product_group_id = product.group_id assert product_group_id is not None # nosec - if product_group_id is not None and not await is_user_by_email_in_group( - app, user_email=email, group_id=product_group_id + if ( + product_group_id is not None + and not await groups_service.is_user_by_email_in_group( + app, user_email=email, group_id=product_group_id + ) ): raise web.HTTPUnauthorized( reason=MSG_UNKNOWN_EMAIL, content_type=MIMETYPE_APPLICATION_JSON diff --git a/services/web/server/src/simcore_service_webserver/login/_controller/rest/change.py b/services/web/server/src/simcore_service_webserver/login/_controller/rest/change.py index 212c394b515..6d4767f8565 100644 --- a/services/web/server/src/simcore_service_webserver/login/_controller/rest/change.py +++ b/services/web/server/src/simcore_service_webserver/login/_controller/rest/change.py @@ -8,10 +8,11 @@ from servicelib.logging_errors import create_troubleshotting_log_kwargs from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from servicelib.request_keys import RQT_USERID_KEY +from simcore_postgres_database.utils_repos import pass_or_acquire_connection from simcore_postgres_database.utils_users import UsersRepo +from simcore_service_webserver.db.plugin import get_asyncpg_engine from ...._meta import API_VTAG -from ....db.plugin import get_database_engine from ....products import products_web from ....products.models import Product from ....security.api import check_password, encrypt_password @@ -238,7 +239,7 @@ async def initiate_change_email(request: web.Request): if user["email"] == request_body.email: return flash_response("Email changed") - async with get_database_engine(request.app).acquire() as conn: + async with pass_or_acquire_connection(get_asyncpg_engine(request.app)) as conn: if await UsersRepo.is_email_used(conn, email=request_body.email): raise web.HTTPUnprocessableEntity(reason="This email cannot be used") From 95b8462cd22fa356cd55eeeaa330c3af5dd8049a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 20:24:40 +0200 Subject: [PATCH 06/11] drafting tests for users repository --- .../users/_users_repository.py | 26 +- .../users/_users_rest.py | 6 +- .../users/_users_service.py | 19 +- .../unit/with_dbs/03/test_users_repository.py | 290 ++++++++++++++++++ 4 files changed, 327 insertions(+), 14 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_repository.py diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index a5be330f25b..ae9f9db784f 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -319,6 +319,7 @@ async def search_users_and_get_profile( connection: AsyncConnection | None = None, *, email_like: str, + product_name: ProductName | None = None, ) -> list[Row]: users_alias = sa.alias(users, name="users_alias") @@ -352,12 +353,16 @@ async def search_users_and_get_profile( invited_by, ) + join_condition = users.c.id == users_pre_registration_details.c.user_id + if product_name: + join_condition = join_condition & ( + users_pre_registration_details.c.product_name == product_name + ) + left_outer_join = ( sa.select(*columns) .select_from( - users_pre_registration_details.outerjoin( - users, users.c.id == users_pre_registration_details.c.user_id - ) + users_pre_registration_details.outerjoin(users, join_condition) ) .where(users_pre_registration_details.c.pre_email.like(email_like)) ) @@ -366,7 +371,7 @@ async def search_users_and_get_profile( .select_from( users.outerjoin( users_pre_registration_details, - users.c.id == users_pre_registration_details.c.user_id, + join_condition, ) ) .where(users.c.email.like(email_like)) @@ -412,22 +417,27 @@ async def get_user_products( return [row async for row in result] -async def create_user_details( +async def create_user_pre_registration( engine: AsyncEngine, connection: AsyncConnection | None = None, *, email: str, created_by: UserID, + product_name: ProductName, **other_values, -) -> None: +) -> int: async with transaction_context(engine, connection) as conn: - await conn.execute( - sa.insert(users_pre_registration_details).values( + result = await conn.execute( + sa.insert(users_pre_registration_details) + .values( created_by=created_by, pre_email=email, + product_name=product_name, **other_values, ) + .returning(users_pre_registration_details.c.id) ) + return result.scalar_one() async def get_user_billing_details( diff --git a/services/web/server/src/simcore_service_webserver/users/_users_rest.py b/services/web/server/src/simcore_service_webserver/users/_users_rest.py index 43cbb4f8422..c52f1c43e2d 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_rest.py @@ -1,5 +1,6 @@ import logging from contextlib import suppress +from itertools import product from aiohttp import web from models_library.api_schemas_webserver.users import ( @@ -195,7 +196,10 @@ async def pre_register_user_for_admin(request: web.Request) -> web.Response: pre_user_profile = await parse_request_body_as(PreRegisteredUserGet, request) user_profile = await _users_service.pre_register_user( - request.app, profile=pre_user_profile, creator_user_id=req_ctx.user_id + request.app, + profile=pre_user_profile, + creator_user_id=req_ctx.user_id, + product_name=req_ctx.product_name, ) return envelope_json_response( user_profile.model_dump(**_RESPONSE_MODEL_MINIMAL_POLICY) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_service.py b/services/web/server/src/simcore_service_webserver/users/_users_service.py index 29361eb8f09..1e4d820a443 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_service.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_service.py @@ -41,12 +41,14 @@ async def pre_register_user( app: web.Application, + *, profile: PreRegisteredUserGet, creator_user_id: UserID, + product_name: ProductName, ) -> UserForAdminGet: found = await search_users_as_admin( - app, email_glob=profile.email, include_products=False + app, email_glob=profile.email, product_name=product_name, include_products=False ) if found: raise AlreadyPreRegisteredError(num_found=len(found), email=profile.email) @@ -71,15 +73,16 @@ async def pre_register_user( if key in details: details[f"pre_{key}"] = details.pop(key) - await _users_repository.create_user_details( + await _users_repository.create_user_pre_registration( get_asyncpg_engine(app), email=profile.email, created_by=creator_user_id, + product_name=product_name, **details, ) found = await search_users_as_admin( - app, email_glob=profile.email, include_products=False + app, email_glob=profile.email, product_name=product_name, include_products=False ) assert len(found) == 1 # nosec @@ -130,7 +133,11 @@ async def get_user_id_from_gid(app: web.Application, primary_gid: GroupID) -> Us async def search_users_as_admin( - app: web.Application, email_glob: str, *, include_products: bool = False + app: web.Application, + *, + email_glob: str, + product_name: ProductName | None = None, + include_products: bool = False, ) -> list[UserForAdminGet]: """ WARNING: this information is reserved for admin users. Note that the returned model include UserForAdminGet @@ -148,7 +155,9 @@ def _glob_to_sql_like(glob_pattern: str) -> str: return sql_like_pattern.replace("*", "%").replace("?", "_") rows = await _users_repository.search_users_and_get_profile( - get_asyncpg_engine(app), email_like=_glob_to_sql_like(email_glob) + get_asyncpg_engine(app), + email_like=_glob_to_sql_like(email_glob), + product_name=product_name, ) async def _list_products_or_none(user_id): diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_repository.py b/services/web/server/tests/unit/with_dbs/03/test_users_repository.py new file mode 100644 index 00000000000..cc319f5fc52 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/test_users_repository.py @@ -0,0 +1,290 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +import asyncio +from collections.abc import AsyncIterable, Callable +from typing import Any + +import pytest +import sqlalchemy as sa +from aiohttp import web +from aiohttp.test_utils import TestServer +from faker import Faker +from models_library.api_schemas_webserver.users import UserForAdminGet +from models_library.products import ProductName +from pytest_simcore.helpers.typing_env import EnvVarsDict +from servicelib.aiohttp.application import create_safe_application +from simcore_postgres_database.models.users_details import ( + users_pre_registration_details, +) +from simcore_service_webserver.application_settings import setup_settings +from simcore_service_webserver.db.plugin import get_asyncpg_engine, setup_db +from simcore_service_webserver.users import _users_service +from simcore_service_webserver.users._users_repository import ( + create_user_pre_registration, +) +from sqlalchemy.ext.asyncio import AsyncEngine + + +@pytest.fixture +def web_server( + event_loop: asyncio.AbstractEventLoop, + app_environment: EnvVarsDict, # configs + postgres_db: sa.engine.Engine, # db-ready + webserver_test_server_port: int, + aiohttp_server: Callable, +) -> TestServer: + """Creates an app that setups the database only""" + app = web.Application() + + app = create_safe_application() + setup_settings(app) + setup_db(app) + + return event_loop.run_until_complete( + aiohttp_server(app, port=webserver_test_server_port) + ) + + +@pytest.fixture +def app(web_server: TestServer) -> web.Application: + """Setup and started app with db""" + _app = web_server.app + assert get_asyncpg_engine(_app) + return _app + + +@pytest.fixture +def asyncpg_engine( + app: web.Application, +) -> AsyncEngine: + """Returns the asyncpg engine ready to be used against postgres""" + return get_asyncpg_engine(app) + + +@pytest.fixture +async def product_owner_user( + faker: Faker, + asyncpg_engine: AsyncEngine, +) -> AsyncIterable[dict[str, Any]]: + """A PO user in the database""" + + from pytest_simcore.helpers.faker_factories import ( + random_user, + ) + from pytest_simcore.helpers.postgres_tools import ( + insert_and_get_row_lifespan, + ) + from simcore_postgres_database.models.users import UserRole, users + + async with insert_and_get_row_lifespan( # pylint:disable=contextmanager-generator-missing-cleanup + asyncpg_engine, + table=users, + values=random_user( + faker, + email="po-user@email.com", + name="po-user-fixture", + role=UserRole.PRODUCT_OWNER, + ), + pk_col=users.c.id, + ) as row: + yield row + + +async def test_create_user_pre_registration( + app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] +): + # Arrange + asyncpg_engine = get_asyncpg_engine(app) + + test_email = "test.user@example.com" + created_by_user_id = product_owner_user["id"] + institution = "Test Institution" + other_values: dict[str, Any] = { + "institution": institution, + "pre_first_name": "Test", + "pre_last_name": "User", + } + + # Act + pre_registration_id = await create_user_pre_registration( + asyncpg_engine, + email=test_email, + created_by=created_by_user_id, + product_name=product_name, + **other_values, + ) + + # Assert + async with asyncpg_engine.connect() as conn: + # Query to check if the record was inserted + result = await conn.execute( + sa.select(users_pre_registration_details).where( + (users_pre_registration_details.c.pre_email == test_email) + & (users_pre_registration_details.c.product_name == product_name) + ) + ) + record = result.first() + + # Clean up - delete the test record + await conn.execute( + sa.delete(users_pre_registration_details).where( + users_pre_registration_details.c.id == pre_registration_id + ) + ) + await conn.commit() + + # Verify the record was created with correct values + assert record is not None + assert record.pre_email == test_email + assert record.created_by == created_by_user_id + assert record.product_name == product_name + assert record.institution == institution + + +async def test_search_users_as_admin_real_user( + app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] +): + """Test searching for a real user as admin""" + # Arrange + user_email = product_owner_user["email"] + + # Act + found_users = await _users_service.search_users_as_admin( + app, email_glob=user_email, product_name=product_name, include_products=False + ) + + # Assert + assert len(found_users) == 1 + found_user = found_users[0] + assert found_user.email == user_email + assert found_user.registered is True + assert ( + found_user.products is None + ) # This test user does not have a product associated with it + + # Verify the UserForAdminGet model is populated correctly + assert isinstance(found_user, UserForAdminGet) + # If first_name is populated in the fixture, it should be in the result + if product_owner_user.get("first_name"): + assert found_user.first_name == product_owner_user["first_name"] + + +async def test_search_users_as_admin_pre_registered_user( + app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] +): + """Test searching for a pre-registered user as admin""" + # Arrange + asyncpg_engine = get_asyncpg_engine(app) + pre_registered_email = "pre-registered@example.com" + created_by_user_id = product_owner_user["id"] + + pre_registration_details = { + "pre_first_name": "Pre-Registered", + "pre_last_name": "User", + "institution": "Test University", + "address": "123 Test Street", + "city": "Test City", + "state": "Test State", + "postal_code": "12345", + "country": "US", + } + + # Create a pre-registered user + pre_registration_id = await create_user_pre_registration( + asyncpg_engine, + email=pre_registered_email, + created_by=created_by_user_id, + product_name=product_name, + **pre_registration_details, + ) + + try: + # Act + found_users = await _users_service.search_users_as_admin( + app, email_glob=pre_registered_email, product_name=product_name + ) + + # Assert + assert len(found_users) == 1 + pre_registered_user = found_users[0] + assert pre_registered_user.email == pre_registered_email + assert ( + pre_registered_user.registered is False + ) # Pre-registered users are not yet registered + assert ( + pre_registered_user.institution == pre_registration_details["institution"] + ) + assert ( + pre_registered_user.first_name == pre_registration_details["pre_first_name"] + ) + assert ( + pre_registered_user.last_name == pre_registration_details["pre_last_name"] + ) + assert pre_registered_user.address == pre_registration_details["address"] + + # Verify the invited_by field is populated (should be the name of the product owner) + assert pre_registered_user.invited_by is not None + + finally: + # Clean up - delete the test record + async with asyncpg_engine.connect() as conn: + await conn.execute( + sa.delete(users_pre_registration_details).where( + users_pre_registration_details.c.id == pre_registration_id + ) + ) + await conn.commit() + + +async def test_search_users_as_admin_wildcard( + app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] +): + """Test searching for users with wildcards""" + # Arrange + asyncpg_engine = get_asyncpg_engine(app) + email_domain = "@example.com" + + # Create multiple pre-registered users with the same domain + emails = [f"user1{email_domain}", f"user2{email_domain}", f"different@other.com"] + created_by_user_id = product_owner_user["id"] + + try: + # Create pre-registered users + for email in emails: + await create_user_pre_registration( + asyncpg_engine, + email=email, + created_by=created_by_user_id, + product_name=product_name, + institution="Test Institution", + ) + + # Act - search with wildcard for the domain + found_users = await _users_service.search_users_as_admin( + app, email_glob=f"*{email_domain}", product_name=product_name + ) + + # Assert + assert len(found_users) == 2 # Should find the two users with @example.com + emails_found = [user.email for user in found_users] + assert f"user1{email_domain}" in emails_found + assert f"user2{email_domain}" in emails_found + assert "different@other.com" not in emails_found + + finally: + # Clean up - delete all test records + async with asyncpg_engine.connect() as conn: + for email in emails: + await conn.execute( + sa.delete(users_pre_registration_details).where( + (users_pre_registration_details.c.pre_email == email) + & ( + users_pre_registration_details.c.product_name + == product_name + ) + ) + ) + await conn.commit() From 865ebcf0a55b4222200a795c556e0b5e229589df Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 20 May 2025 20:50:58 +0200 Subject: [PATCH 07/11] moved functions --- .../unit/with_dbs/03/test_users_repository.py | 290 ------------------ .../tests/unit/with_dbs/03/users/conftest.py | 84 +++++ .../03/users/test_users_repository.py | 70 +++++ .../with_dbs/03/users/test_users_service.py | 189 ++++++++++++ 4 files changed, 343 insertions(+), 290 deletions(-) delete mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_repository.py create mode 100644 services/web/server/tests/unit/with_dbs/03/users/conftest.py create mode 100644 services/web/server/tests/unit/with_dbs/03/users/test_users_repository.py create mode 100644 services/web/server/tests/unit/with_dbs/03/users/test_users_service.py diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_repository.py b/services/web/server/tests/unit/with_dbs/03/test_users_repository.py deleted file mode 100644 index cc319f5fc52..00000000000 --- a/services/web/server/tests/unit/with_dbs/03/test_users_repository.py +++ /dev/null @@ -1,290 +0,0 @@ -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -# pylint: disable=unused-variable -# pylint: disable=too-many-arguments - -import asyncio -from collections.abc import AsyncIterable, Callable -from typing import Any - -import pytest -import sqlalchemy as sa -from aiohttp import web -from aiohttp.test_utils import TestServer -from faker import Faker -from models_library.api_schemas_webserver.users import UserForAdminGet -from models_library.products import ProductName -from pytest_simcore.helpers.typing_env import EnvVarsDict -from servicelib.aiohttp.application import create_safe_application -from simcore_postgres_database.models.users_details import ( - users_pre_registration_details, -) -from simcore_service_webserver.application_settings import setup_settings -from simcore_service_webserver.db.plugin import get_asyncpg_engine, setup_db -from simcore_service_webserver.users import _users_service -from simcore_service_webserver.users._users_repository import ( - create_user_pre_registration, -) -from sqlalchemy.ext.asyncio import AsyncEngine - - -@pytest.fixture -def web_server( - event_loop: asyncio.AbstractEventLoop, - app_environment: EnvVarsDict, # configs - postgres_db: sa.engine.Engine, # db-ready - webserver_test_server_port: int, - aiohttp_server: Callable, -) -> TestServer: - """Creates an app that setups the database only""" - app = web.Application() - - app = create_safe_application() - setup_settings(app) - setup_db(app) - - return event_loop.run_until_complete( - aiohttp_server(app, port=webserver_test_server_port) - ) - - -@pytest.fixture -def app(web_server: TestServer) -> web.Application: - """Setup and started app with db""" - _app = web_server.app - assert get_asyncpg_engine(_app) - return _app - - -@pytest.fixture -def asyncpg_engine( - app: web.Application, -) -> AsyncEngine: - """Returns the asyncpg engine ready to be used against postgres""" - return get_asyncpg_engine(app) - - -@pytest.fixture -async def product_owner_user( - faker: Faker, - asyncpg_engine: AsyncEngine, -) -> AsyncIterable[dict[str, Any]]: - """A PO user in the database""" - - from pytest_simcore.helpers.faker_factories import ( - random_user, - ) - from pytest_simcore.helpers.postgres_tools import ( - insert_and_get_row_lifespan, - ) - from simcore_postgres_database.models.users import UserRole, users - - async with insert_and_get_row_lifespan( # pylint:disable=contextmanager-generator-missing-cleanup - asyncpg_engine, - table=users, - values=random_user( - faker, - email="po-user@email.com", - name="po-user-fixture", - role=UserRole.PRODUCT_OWNER, - ), - pk_col=users.c.id, - ) as row: - yield row - - -async def test_create_user_pre_registration( - app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] -): - # Arrange - asyncpg_engine = get_asyncpg_engine(app) - - test_email = "test.user@example.com" - created_by_user_id = product_owner_user["id"] - institution = "Test Institution" - other_values: dict[str, Any] = { - "institution": institution, - "pre_first_name": "Test", - "pre_last_name": "User", - } - - # Act - pre_registration_id = await create_user_pre_registration( - asyncpg_engine, - email=test_email, - created_by=created_by_user_id, - product_name=product_name, - **other_values, - ) - - # Assert - async with asyncpg_engine.connect() as conn: - # Query to check if the record was inserted - result = await conn.execute( - sa.select(users_pre_registration_details).where( - (users_pre_registration_details.c.pre_email == test_email) - & (users_pre_registration_details.c.product_name == product_name) - ) - ) - record = result.first() - - # Clean up - delete the test record - await conn.execute( - sa.delete(users_pre_registration_details).where( - users_pre_registration_details.c.id == pre_registration_id - ) - ) - await conn.commit() - - # Verify the record was created with correct values - assert record is not None - assert record.pre_email == test_email - assert record.created_by == created_by_user_id - assert record.product_name == product_name - assert record.institution == institution - - -async def test_search_users_as_admin_real_user( - app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] -): - """Test searching for a real user as admin""" - # Arrange - user_email = product_owner_user["email"] - - # Act - found_users = await _users_service.search_users_as_admin( - app, email_glob=user_email, product_name=product_name, include_products=False - ) - - # Assert - assert len(found_users) == 1 - found_user = found_users[0] - assert found_user.email == user_email - assert found_user.registered is True - assert ( - found_user.products is None - ) # This test user does not have a product associated with it - - # Verify the UserForAdminGet model is populated correctly - assert isinstance(found_user, UserForAdminGet) - # If first_name is populated in the fixture, it should be in the result - if product_owner_user.get("first_name"): - assert found_user.first_name == product_owner_user["first_name"] - - -async def test_search_users_as_admin_pre_registered_user( - app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] -): - """Test searching for a pre-registered user as admin""" - # Arrange - asyncpg_engine = get_asyncpg_engine(app) - pre_registered_email = "pre-registered@example.com" - created_by_user_id = product_owner_user["id"] - - pre_registration_details = { - "pre_first_name": "Pre-Registered", - "pre_last_name": "User", - "institution": "Test University", - "address": "123 Test Street", - "city": "Test City", - "state": "Test State", - "postal_code": "12345", - "country": "US", - } - - # Create a pre-registered user - pre_registration_id = await create_user_pre_registration( - asyncpg_engine, - email=pre_registered_email, - created_by=created_by_user_id, - product_name=product_name, - **pre_registration_details, - ) - - try: - # Act - found_users = await _users_service.search_users_as_admin( - app, email_glob=pre_registered_email, product_name=product_name - ) - - # Assert - assert len(found_users) == 1 - pre_registered_user = found_users[0] - assert pre_registered_user.email == pre_registered_email - assert ( - pre_registered_user.registered is False - ) # Pre-registered users are not yet registered - assert ( - pre_registered_user.institution == pre_registration_details["institution"] - ) - assert ( - pre_registered_user.first_name == pre_registration_details["pre_first_name"] - ) - assert ( - pre_registered_user.last_name == pre_registration_details["pre_last_name"] - ) - assert pre_registered_user.address == pre_registration_details["address"] - - # Verify the invited_by field is populated (should be the name of the product owner) - assert pre_registered_user.invited_by is not None - - finally: - # Clean up - delete the test record - async with asyncpg_engine.connect() as conn: - await conn.execute( - sa.delete(users_pre_registration_details).where( - users_pre_registration_details.c.id == pre_registration_id - ) - ) - await conn.commit() - - -async def test_search_users_as_admin_wildcard( - app: web.Application, product_name: ProductName, product_owner_user: dict[str, Any] -): - """Test searching for users with wildcards""" - # Arrange - asyncpg_engine = get_asyncpg_engine(app) - email_domain = "@example.com" - - # Create multiple pre-registered users with the same domain - emails = [f"user1{email_domain}", f"user2{email_domain}", f"different@other.com"] - created_by_user_id = product_owner_user["id"] - - try: - # Create pre-registered users - for email in emails: - await create_user_pre_registration( - asyncpg_engine, - email=email, - created_by=created_by_user_id, - product_name=product_name, - institution="Test Institution", - ) - - # Act - search with wildcard for the domain - found_users = await _users_service.search_users_as_admin( - app, email_glob=f"*{email_domain}", product_name=product_name - ) - - # Assert - assert len(found_users) == 2 # Should find the two users with @example.com - emails_found = [user.email for user in found_users] - assert f"user1{email_domain}" in emails_found - assert f"user2{email_domain}" in emails_found - assert "different@other.com" not in emails_found - - finally: - # Clean up - delete all test records - async with asyncpg_engine.connect() as conn: - for email in emails: - await conn.execute( - sa.delete(users_pre_registration_details).where( - (users_pre_registration_details.c.pre_email == email) - & ( - users_pre_registration_details.c.product_name - == product_name - ) - ) - ) - await conn.commit() diff --git a/services/web/server/tests/unit/with_dbs/03/users/conftest.py b/services/web/server/tests/unit/with_dbs/03/users/conftest.py new file mode 100644 index 00000000000..7f5545977c5 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/users/conftest.py @@ -0,0 +1,84 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +import asyncio +from collections.abc import AsyncIterable, Callable +from typing import Any + +import pytest +import sqlalchemy as sa +from aiohttp import web +from aiohttp.test_utils import TestServer +from faker import Faker +from pytest_simcore.helpers.typing_env import EnvVarsDict +from servicelib.aiohttp.application import create_safe_application +from simcore_service_webserver.application_settings import setup_settings +from simcore_service_webserver.db.plugin import get_asyncpg_engine, setup_db +from sqlalchemy.ext.asyncio import AsyncEngine + + +@pytest.fixture +def web_server( + event_loop: asyncio.AbstractEventLoop, + app_environment: EnvVarsDict, # configs + postgres_db: sa.engine.Engine, # db-ready + webserver_test_server_port: int, + aiohttp_server: Callable, +) -> TestServer: + """Creates an app that setups the database only""" + app = web.Application() + + app = create_safe_application() + setup_settings(app) + setup_db(app) + + return event_loop.run_until_complete( + aiohttp_server(app, port=webserver_test_server_port) + ) + + +@pytest.fixture +def app(web_server: TestServer) -> web.Application: + """Setup and started app with db""" + _app = web_server.app + assert get_asyncpg_engine(_app) + return _app + + +@pytest.fixture +def asyncpg_engine( + app: web.Application, +) -> AsyncEngine: + """Returns the asyncpg engine ready to be used against postgres""" + return get_asyncpg_engine(app) + + +@pytest.fixture +async def product_owner_user( + faker: Faker, + asyncpg_engine: AsyncEngine, +) -> AsyncIterable[dict[str, Any]]: + """A PO user in the database""" + + from pytest_simcore.helpers.faker_factories import ( + random_user, + ) + from pytest_simcore.helpers.postgres_tools import ( + insert_and_get_row_lifespan, + ) + from simcore_postgres_database.models.users import UserRole, users + + async with insert_and_get_row_lifespan( # pylint:disable=contextmanager-generator-missing-cleanup + asyncpg_engine, + table=users, + values=random_user( + faker, + email="po-user@email.com", + name="po-user-fixture", + role=UserRole.PRODUCT_OWNER, + ), + pk_col=users.c.id, + ) as record: + yield record diff --git a/services/web/server/tests/unit/with_dbs/03/users/test_users_repository.py b/services/web/server/tests/unit/with_dbs/03/users/test_users_repository.py new file mode 100644 index 00000000000..94dfec56a33 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/users/test_users_repository.py @@ -0,0 +1,70 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +from typing import Any + +import sqlalchemy as sa +from aiohttp import web +from models_library.products import ProductName +from simcore_postgres_database.models.users_details import ( + users_pre_registration_details, +) +from simcore_service_webserver.db.plugin import get_asyncpg_engine +from simcore_service_webserver.users._users_repository import ( + create_user_pre_registration, +) + + +async def test_create_user_pre_registration( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], +): + # Arrange + asyncpg_engine = get_asyncpg_engine(app) + + test_email = "test.user@example.com" + created_by_user_id = product_owner_user["id"] + institution = "Test Institution" + pre_registration_details: dict[str, Any] = { + "institution": institution, + "pre_first_name": "Test", + "pre_last_name": "User", + } + + # Act + pre_registration_id = await create_user_pre_registration( + asyncpg_engine, + email=test_email, + created_by=created_by_user_id, + product_name=product_name, + **pre_registration_details, + ) + + # Assert + async with asyncpg_engine.connect() as conn: + # Query to check if the record was inserted + result = await conn.execute( + sa.select(users_pre_registration_details).where( + (users_pre_registration_details.c.pre_email == test_email) + & (users_pre_registration_details.c.product_name == product_name) + ) + ) + record = result.first() + + # Clean up - delete the test record + await conn.execute( + sa.delete(users_pre_registration_details).where( + users_pre_registration_details.c.id == pre_registration_id + ) + ) + await conn.commit() + + # Verify the record was created with correct values + assert record is not None + assert record.pre_email == test_email + assert record.created_by == created_by_user_id + assert record.product_name == product_name + assert record.institution == institution diff --git a/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py b/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py new file mode 100644 index 00000000000..3b6b4b8c203 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py @@ -0,0 +1,189 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +from collections.abc import AsyncGenerator, AsyncIterable +from typing import Any + +import pytest +import sqlalchemy as sa +from aiohttp import web +from models_library.api_schemas_webserver.users import UserForAdminGet +from models_library.products import ProductName +from simcore_postgres_database.models.users_details import ( + users_pre_registration_details, +) +from simcore_service_webserver.db.plugin import get_asyncpg_engine +from simcore_service_webserver.users import _users_service +from simcore_service_webserver.users._users_repository import ( + create_user_pre_registration, +) + + +@pytest.fixture +async def pre_registration_details_cleanup( + app: web.Application, +) -> AsyncGenerator[None, None]: + """Fixture to clean up all pre-registration details after test""" + yield + + # Tear down - clean up the pre-registration details table + async with get_asyncpg_engine(app).connect() as conn: + await conn.execute(sa.delete(users_pre_registration_details)) + await conn.commit() + + +@pytest.fixture +async def pre_registered_user_created( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], +) -> AsyncIterable[dict[str, Any]]: + """Creates a pre-registered user and returns the details. + Automatically cleans up after the test.""" + + asyncpg_engine = get_asyncpg_engine(app) + pre_registered_email = "pre-registered@example.com" + created_by_user_id = product_owner_user["id"] + + pre_registration_details = { + "pre_first_name": "Pre-Registered", + "pre_last_name": "User", + "institution": "Test University", + "address": "123 Test Street", + "city": "Test City", + "state": "Test State", + "postal_code": "12345", + "country": "US", + } + + # Create a pre-registered user + pre_registration_id = await create_user_pre_registration( + asyncpg_engine, + email=pre_registered_email, + created_by=created_by_user_id, + product_name=product_name, + **pre_registration_details, + ) + + # Return all details including the ID for tests to use + yield { + "id": pre_registration_id, + "email": pre_registered_email, + "details": pre_registration_details, + "creator_id": created_by_user_id, + } + + # Clean up after test + async with asyncpg_engine.connect() as conn: + await conn.execute( + sa.delete(users_pre_registration_details).where( + users_pre_registration_details.c.id == pre_registration_id + ) + ) + await conn.commit() + + +async def test_search_users_as_admin_real_user( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], +): + """Test searching for a real user as admin""" + # Arrange + user_email = product_owner_user["email"] + + # Act + found_users = await _users_service.search_users_as_admin( + app, email_glob=user_email, product_name=product_name, include_products=False + ) + + # Assert + assert len(found_users) == 1 + found_user = found_users[0] + assert found_user.email == user_email + assert found_user.registered is True + assert ( + found_user.products is None + ) # This test user does not have a product associated with it + + # Verify the UserForAdminGet model is populated correctly + assert isinstance(found_user, UserForAdminGet) + assert found_user.first_name == product_owner_user["first_name"] + + +async def test_search_users_as_admin_pre_registered_user( + app: web.Application, + product_name: ProductName, + pre_registered_user_created: dict[str, Any], +): + """Test searching for a pre-registered user as admin""" + # Arrange + pre_registered_email = pre_registered_user_created["email"] + pre_registration_details = pre_registered_user_created["details"] + + # Act + found_users = await _users_service.search_users_as_admin( + app, email_glob=pre_registered_email, product_name=product_name + ) + + # Assert + assert len(found_users) == 1 + pre_registered_user_found = found_users[0] + assert pre_registered_user_found.email == pre_registered_email + assert ( + pre_registered_user_found.registered is False + ) # Pre-registered users are not yet registered + assert ( + pre_registered_user_found.institution == pre_registration_details["institution"] + ) + assert ( + pre_registered_user_found.first_name + == pre_registration_details["pre_first_name"] + ) + assert ( + pre_registered_user_found.last_name == pre_registration_details["pre_last_name"] + ) + assert pre_registered_user_found.address == pre_registration_details["address"] + + # Verify the invited_by field is populated (should be the name of the product owner) + assert pre_registered_user_found.invited_by is not None + + +async def test_search_users_as_admin_wildcard( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], + pre_registration_details_cleanup: None, +): + """Test searching for users with wildcards""" + # Arrange + asyncpg_engine = get_asyncpg_engine(app) + email_domain = "@example.com" + + # Create multiple pre-registered users with the same domain + emails = [f"user1{email_domain}", f"user2{email_domain}", f"different@other.com"] + created_by_user_id = product_owner_user["id"] + + # Create pre-registered users + for email in emails: + await create_user_pre_registration( + asyncpg_engine, + email=email, + created_by=created_by_user_id, + product_name=product_name, + institution="Test Institution", + ) + + # Act - search with wildcard for the domain + found_users = await _users_service.search_users_as_admin( + app, email_glob=f"*{email_domain}", product_name=product_name + ) + + # Assert + assert len(found_users) == 2 # Should find the two users with @example.com + emails_found = [user.email for user in found_users] + assert f"user1{email_domain}" in emails_found + assert f"user2{email_domain}" in emails_found + assert "different@other.com" not in emails_found From 2c1b6156e97318771f67151c9c4b85fbca7da0ec Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 21 May 2025 10:18:02 +0200 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=90=9B=20Fix=20return=20value=20in?= =?UTF-8?q?=20create=5Fuser=5Fpre=5Fregistration=20and=20remove=20unused?= =?UTF-8?q?=20import=20in=20=5Fusers=5Frest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/simcore_service_webserver/users/_users_repository.py | 3 ++- .../server/src/simcore_service_webserver/users/_users_rest.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index ae9f9db784f..05e4de80e8f 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -437,7 +437,8 @@ async def create_user_pre_registration( ) .returning(users_pre_registration_details.c.id) ) - return result.scalar_one() + pre_registration_id: int = result.scalar_one() + return pre_registration_id async def get_user_billing_details( diff --git a/services/web/server/src/simcore_service_webserver/users/_users_rest.py b/services/web/server/src/simcore_service_webserver/users/_users_rest.py index c52f1c43e2d..ced4632cec3 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_rest.py @@ -1,6 +1,5 @@ import logging from contextlib import suppress -from itertools import product from aiohttp import web from models_library.api_schemas_webserver.users import ( From 93e7112529cc49cc50fd5a188499f36ba6f2c333 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 21 May 2025 10:36:08 +0200 Subject: [PATCH 09/11] fixing tests --- .../server/src/simcore_service_webserver/login/_auth_service.py | 2 +- .../invitations/test_login_handlers_registration_invitations.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/login/_auth_service.py b/services/web/server/src/simcore_service_webserver/login/_auth_service.py index d439bf94337..31b87305115 100644 --- a/services/web/server/src/simcore_service_webserver/login/_auth_service.py +++ b/services/web/server/src/simcore_service_webserver/login/_auth_service.py @@ -42,7 +42,7 @@ async def create_user( await UsersRepo.link_and_update_user_from_pre_registration( conn, new_user_id=user.id, new_user_email=user.email ) - return dict(user.items()) + return dict(user._mapping) # pylint: disable=protected-access # noqa: SLF001 async def check_authorized_user_credentials_or_raise( diff --git a/services/web/server/tests/unit/with_dbs/03/invitations/test_login_handlers_registration_invitations.py b/services/web/server/tests/unit/with_dbs/03/invitations/test_login_handlers_registration_invitations.py index d0490d01ffd..8bb512d5d4c 100644 --- a/services/web/server/tests/unit/with_dbs/03/invitations/test_login_handlers_registration_invitations.py +++ b/services/web/server/tests/unit/with_dbs/03/invitations/test_login_handlers_registration_invitations.py @@ -110,7 +110,7 @@ def _extract_invitation_code_from_url(invitation_url: HttpUrl) -> str: return query_params["invitation"] -@pytest.mark.acceptance_test() +@pytest.mark.acceptance_test async def test_registration_to_different_product( mocker: MockerFixture, app_products_names: list[ProductName], From b22893652a08e886ea7adbdf1ca41ee126e0b97e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 21 May 2025 10:45:13 +0200 Subject: [PATCH 10/11] fix pylint --- .../server/tests/unit/with_dbs/03/users/test_users_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py b/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py index 3b6b4b8c203..775e9c7a544 100644 --- a/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py +++ b/services/web/server/tests/unit/with_dbs/03/users/test_users_service.py @@ -163,7 +163,7 @@ async def test_search_users_as_admin_wildcard( email_domain = "@example.com" # Create multiple pre-registered users with the same domain - emails = [f"user1{email_domain}", f"user2{email_domain}", f"different@other.com"] + emails = [f"user1{email_domain}", f"user2{email_domain}", "different@other.com"] created_by_user_id = product_owner_user["id"] # Create pre-registered users From 7993efb18b7e91a57f868dc622f42c6315d7ada3 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 21 May 2025 11:17:38 +0200 Subject: [PATCH 11/11] fixes datetimte in db --- .../login/_controller/rest/registration.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/login/_controller/rest/registration.py b/services/web/server/src/simcore_service_webserver/login/_controller/rest/registration.py index f8fbdad2e29..ba13b0dd720 100644 --- a/services/web/server/src/simcore_service_webserver/login/_controller/rest/registration.py +++ b/services/web/server/src/simcore_service_webserver/login/_controller/rest/registration.py @@ -211,7 +211,10 @@ async def register(request: web.Request): app=request.app, ) if invitation.trial_account_days: - expires_at = datetime.now(UTC) + timedelta(invitation.trial_account_days) + # NOTE: expires_at is currently set as offset-naive + expires_at = ( + datetime.now(UTC) + timedelta(invitation.trial_account_days) + ).replace(tzinfo=None) # get authorized user or create new user = await _auth_service.get_user_by_email(request.app, email=registration.email)