Skip to content

Commit

Permalink
@odeimaiz review: adds username
Browse files Browse the repository at this point in the history
  • Loading branch information
pcrespov committed Dec 10, 2024
1 parent 740c711 commit 3e9b78b
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 18 deletions.
2 changes: 1 addition & 1 deletion api/specs/web-server/_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ async def add_group_user(
_body: GroupUserAdd,
):
"""
Adds a user to an organization group
Adds a user to an organization group using their username, user ID, or email (subject to privacy settings
"""


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ class GroupUserAdd(InputSchema):
"""

uid: UserID | None = None
user_name: Annotated[IDStr | None, Field(alias="userName")] = None
email: Annotated[
LowerCaseEmailStr | None,
Field(
Expand All @@ -296,7 +297,7 @@ class GroupUserAdd(InputSchema):
] = None

_check_uid_or_email = model_validator(mode="after")(
create__check_only_one_is_set__root_validator(["uid", "email"])
create__check_only_one_is_set__root_validator(["uid", "email", "user_name"])
)

model_config = ConfigDict(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ def null_or_none_str_to_none_validator(value: Any):
return value


def create__check_only_one_is_set__root_validator(alternative_field_names: list[str]):
def create__check_only_one_is_set__root_validator(
mutually_exclusive_field_names: list[str],
):
"""Ensure exactly one and only one of the alternatives is set
NOTE: a field is considered here `unset` when it is `not None`. When None
Expand All @@ -104,17 +106,16 @@ def create__check_only_one_is_set__root_validator(alternative_field_names: list[
"""

def _validator(cls: type[BaseModel], values):
assert set(alternative_field_names).issubset(cls.model_fields) # nosec

assert set(mutually_exclusive_field_names).issubset( # nosec
cls.model_fields
), f"Invalid {mutually_exclusive_field_names=} passed in the factory arguments"
got = {
field_name: getattr(values, field_name)
for field_name in alternative_field_names
for field_name in mutually_exclusive_field_names
}

if not functools.reduce(operator.xor, (v is not None for v in got.values())):
msg = (
f"Either { 'or'.join(got.keys()) } must be set, but not both. Got {got}"
)
msg = f"Either { ' or '.join(got.keys()) } must be set, but not both. Got {got}"
raise ValueError(msg)
return values

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,8 @@ paths:
tags:
- groups
summary: Add Group User
description: Adds a user to an organization group
description: Adds a user to an organization group using their username, user
ID, or email (subject to privacy settings
operationId: add_group_user
parameters:
- name: gid
Expand Down Expand Up @@ -9911,6 +9912,13 @@ components:
minimum: 0
- type: 'null'
title: Uid
userName:
anyOf:
- type: string
maxLength: 100
minLength: 1
- type: 'null'
title: Username
email:
anyOf:
- type: string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from aiohttp import web
from models_library.basic_types import IDStr
from models_library.emails import LowerCaseEmailStr
from models_library.groups import (
AccessRightsDict,
Expand Down Expand Up @@ -236,12 +237,17 @@ async def auto_add_user_to_product_group(
)


def _only_one_true(*args):
return sum(bool(arg) for arg in args) == 1


async def add_user_in_group(
app: web.Application,
user_id: UserID,
group_id: GroupID,
*,
new_user_id: UserID | None = None,
new_user_name: IDStr | None = None,
new_user_email: EmailStr | None = None,
access_rights: AccessRightsDict | None = None,
) -> None:
Expand All @@ -251,9 +257,8 @@ async def add_user_in_group(
UserInGroupNotFoundError
GroupsException
"""

if not new_user_id and not new_user_email:
msg = "Invalid method call, missing user id or user email"
if not _only_one_true(new_user_id, new_user_name, new_user_email):
msg = "Invalid method call, required one of these: user id, username or user email, none provided"
raise GroupsError(msg=msg)

if new_user_email:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import sqlalchemy as sa
from aiohttp import web
from models_library.basic_types import IDStr
from models_library.groups import (
AccessRightsDict,
Group,
Expand Down Expand Up @@ -589,7 +590,9 @@ async def add_new_user_in_group(
*,
user_id: UserID,
group_id: GroupID,
new_user_id: UserID,
# either user_id or user_name
new_user_id: UserID | None = None,
new_user_name: IDStr | None = None,
access_rights: AccessRightsDict | None = None,
) -> None:
"""
Expand All @@ -602,10 +605,17 @@ async def add_new_user_in_group(
)
_check_group_permissions(group, user_id, group_id, "write")

query = sa.select(sa.func.count())
if new_user_id:
query = query.where(users.c.id == new_user_id)
elif new_user_name:
query = query.where(users.c.name == new_user_name)
else:
msg = "Either user name or id but none provided"
raise ValueError(msg)

# now check the new user exists
users_count = await conn.scalar(
sa.select(sa.func.count()).where(users.c.id == new_user_id)
)
users_count = await conn.scalar(query)
if not users_count:
assert new_user_id is not None # nosec
raise UserInGroupNotFoundError(uid=new_user_id, gid=group_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ async def delete_group(request: web.Request):
@login_required
@permission_required("groups.*")
@handle_plugin_requests_exceptions
async def get_group_users(request: web.Request):
async def get_all_group_users(request: web.Request):
"""Gets users in organization groups"""
req_ctx = GroupsRequestContext.model_validate(request)
path_params = parse_request_path_parameters_as(GroupsPathParams, request)
Expand Down Expand Up @@ -194,6 +194,7 @@ async def add_group_user(request: web.Request):
req_ctx.user_id,
path_params.gid,
new_user_id=added.uid,
new_user_name=added.user_name,
new_user_email=added.email,
)

Expand Down
22 changes: 22 additions & 0 deletions services/web/server/tests/unit/isolated/test_groups_models.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import models_library.groups
import pytest
import simcore_postgres_database.models.groups
from faker import Faker
from models_library.api_schemas_webserver._base import OutputSchema
from models_library.api_schemas_webserver.groups import (
GroupCreate,
GroupGet,
GroupUpdate,
GroupUserAdd,
GroupUserGet,
)
from models_library.groups import (
Expand All @@ -17,6 +19,7 @@
OrganizationUpdate,
)
from models_library.utils.enums import enum_to_dict
from pydantic import ValidationError


def test_models_library_and_postgress_database_enums_are_equivalent():
Expand Down Expand Up @@ -100,3 +103,22 @@ def test_input_schemas_to_models(faker: Faker):
domain_model = input_schema.to_model()
assert isinstance(domain_model, OrganizationUpdate)
assert domain_model.name == input_schema.label


def test_group_user_add_options(faker: Faker):
def _only_one_true(*args):
return sum(bool(arg) for arg in args) == 1

input_schema = GroupUserAdd(uid=faker.pyint())
assert input_schema.uid
assert _only_one_true(input_schema.uid, input_schema.user_name, input_schema.email)

input_schema = GroupUserAdd(userName=faker.user_name())
assert input_schema.user_name
assert _only_one_true(input_schema.uid, input_schema.user_name, input_schema.email)

input_schema = GroupUserAdd(email=faker.email())
assert _only_one_true(input_schema.uid, input_schema.user_name, input_schema.email)

with pytest.raises(ValidationError):
GroupUserAdd(userName=faker.user_name(), email=faker.email())

0 comments on commit 3e9b78b

Please sign in to comment.