Skip to content

Commit 9e1c2ec

Browse files
committed
✨ users: Update user schema and repository queries to use 'created_by' instead of 'invited_by' and adjust related tests
1 parent cc88388 commit 9e1c2ec

File tree

5 files changed

+34
-32
lines changed

5 files changed

+34
-32
lines changed

packages/models-library/src/models_library/api_schemas_webserver/users.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ class UserForAdminGet(OutputSchema):
291291
] = DEFAULT_FACTORY
292292

293293
# authorization
294-
invited_by: str | None = None
294+
invited_by: Annotated[str | None, Field(alias="created_by")] = None
295295

296296
# user status
297297
registered: bool

services/web/server/src/simcore_service_webserver/users/_users_repository.py

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -807,21 +807,6 @@ async def list_merged_pre_and_registered_users(
807807
Returns:
808808
Tuple of (list of merged user data, total count)
809809
"""
810-
# Create alias for users table for creator and reviewer lookups
811-
creators_alias = sa.alias(users, name="creators")
812-
reviewers_alias = sa.alias(users, name="reviewers")
813-
814-
# Query for pre-registered users
815-
invited_by = (
816-
sa.select(
817-
creators_alias.c.name,
818-
)
819-
.where(users_pre_registration_details.c.created_by == creators_alias.c.id)
820-
.correlate(None)
821-
.scalar_subquery()
822-
.label("invited_by")
823-
)
824-
825810
# Base where conditions for both queries
826811
pre_reg_where = [users_pre_registration_details.c.product_name == product_name]
827812
users_where = []
@@ -851,15 +836,16 @@ async def list_merged_pre_and_registered_users(
851836
users_pre_registration_details.c.state,
852837
users_pre_registration_details.c.postal_code,
853838
users_pre_registration_details.c.country,
854-
users_pre_registration_details.c.user_id,
839+
users_pre_registration_details.c.user_id.label("pre_reg_user_id"),
855840
users_pre_registration_details.c.extras,
856841
users_pre_registration_details.c.created,
857842
users_pre_registration_details.c.account_request_status,
858843
users.c.id.label("user_id"),
859844
users.c.name.label("user_name"),
860845
users.c.status,
861846
users.c.created_at,
862-
invited_by,
847+
# Use created_by directly instead of a subquery
848+
users_pre_registration_details.c.created_by.label("created_by"),
863849
sa.literal(True).label("is_pre_registered"),
864850
)
865851
.select_from(
@@ -883,15 +869,16 @@ async def list_merged_pre_and_registered_users(
883869
sa.literal(None).label("state"),
884870
sa.literal(None).label("postal_code"),
885871
sa.literal(None).label("country"),
886-
users.c.id.label("user_id"),
872+
sa.literal(None).label("pre_reg_user_id"),
887873
sa.literal(None).label("extras"),
888874
users.c.created_at.label("created"),
889875
sa.literal(None).label("account_request_status"),
890876
users.c.id.label("user_id"),
891877
users.c.name.label("user_name"),
892878
users.c.status,
893879
users.c.created_at,
894-
sa.literal(None).label("invited_by"),
880+
# Match the created_by field from the pre_reg query
881+
sa.literal(None).label("created_by"),
895882
sa.literal(False).label("is_pre_registered"),
896883
)
897884
.select_from(
@@ -905,24 +892,28 @@ async def list_merged_pre_and_registered_users(
905892
# Combine with a UNION ALL query
906893
merged_query = pre_reg_query.union_all(users_query)
907894

908-
# Add distinct on user_id to eliminate duplicates
895+
# Add distinct on email to eliminate duplicates
896+
merged_query_subq = merged_query.subquery()
909897
distinct_query = (
910-
sa.select(merged_query.c)
911-
.select_from(merged_query)
912-
.distinct(merged_query.c.email)
898+
sa.select(merged_query_subq)
899+
.select_from(merged_query_subq)
900+
.distinct(merged_query_subq.c.email)
913901
.order_by(
914-
merged_query.c.email,
902+
merged_query_subq.c.email,
915903
# Prioritize pre-registration records if duplicate emails exist
916-
merged_query.c.is_pre_registered.desc(),
917-
merged_query.c.created.desc(),
904+
merged_query_subq.c.is_pre_registered.desc(),
905+
merged_query_subq.c.created.desc(),
918906
)
919907
.limit(pagination_limit)
920908
.offset(pagination_offset)
921909
)
922910

923911
# Count query (for pagination)
924912
count_query = sa.select(sa.func.count().label("total")).select_from(
925-
sa.select(merged_query.c.email).select_from(merged_query).distinct().subquery()
913+
sa.select(merged_query_subq.c.email)
914+
.select_from(merged_query_subq)
915+
.distinct()
916+
.subquery()
926917
)
927918

928919
_logger.debug(

services/web/server/src/simcore_service_webserver/users/_users_rest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ async def list_users_for_admin(request: web.Request) -> web.Response:
177177
UsersForAdminListQueryParams, request
178178
)
179179

180-
users, total_count = await _users_service.list_users_as_admin(
180+
users, total_count = await _users_service.list_all_users_as_admin(
181181
request.app,
182182
product_name=req_ctx.product_name,
183183
filter_account_request_status=(

services/web/server/src/simcore_service_webserver/users/_users_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ async def is_user_in_product(
209209
)
210210

211211

212-
async def list_users_as_admin(
212+
async def list_all_users_as_admin(
213213
app: web.Application,
214214
*,
215215
product_name: ProductName,

services/web/server/tests/unit/with_dbs/03/users/test_users_repository.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,15 +419,19 @@ async def test_list_merged_pre_and_registered_users(
419419
)
420420
assert pre_reg_only_user is not None, "Pre-registered user should be in results"
421421
assert pre_reg_only_user["is_pre_registered"] is True
422+
# Check the pre_registration user_id is None but using the new column name
423+
assert pre_reg_only_user["pre_reg_user_id"] is None
424+
# For non-linked users, user_id is still None
422425
assert (
423426
pre_reg_only_user["user_id"] is None
424427
), "Pre-registered only user shouldn't have a user_id"
425428
assert pre_reg_only_user["institution"] == "Pre-Reg Institution"
426429
assert pre_reg_only_user["first_name"] == "Pre-Registered"
427430
assert pre_reg_only_user["last_name"] == "Only"
431+
# Check created_by field instead of invited_by
428432
assert (
429-
pre_reg_only_user["invited_by"] is not None
430-
), "Should have invited_by field"
433+
pre_reg_only_user["created_by"] == created_by_user_id
434+
), "Should have created_by field with the creator's ID"
431435

432436
# 3. Check the product owner (both registered and pre-registered)
433437
product_owner = next(
@@ -442,6 +446,10 @@ async def test_list_merged_pre_and_registered_users(
442446
assert (
443447
product_owner["is_pre_registered"] is True
444448
), "Should prefer pre-registration record"
449+
# Check both the pre_reg_user_id (from pre-registration) and user_id (from users table)
450+
assert (
451+
product_owner["pre_reg_user_id"] == product_owner_user["id"]
452+
), "pre_reg_user_id should match the product owner id"
445453
assert (
446454
product_owner["user_id"] == product_owner_user["id"]
447455
), "Should be linked to existing user"
@@ -455,6 +463,9 @@ async def test_list_merged_pre_and_registered_users(
455463
assert (
456464
product_owner["status"] is not None
457465
), "Should include status from users table"
466+
assert (
467+
product_owner["created_by"] == created_by_user_id
468+
), "Should have created_by field with the creator's ID"
458469

459470
# 4. Test filtering by account request status
460471
pending_users, pending_count = (

0 commit comments

Comments
 (0)