Skip to content

Commit

Permalink
Add type param to group create, update and upsert APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
@seanh
seanh committed Sep 5, 2024
1 parent 480717d commit 69858e1
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 19 deletions.
4 changes: 4 additions & 0 deletions docs/_extra/api-reference/schemas/group-new.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ Group:
type: string
description: group description
maxLength: 250
type:
type: string
enum: [private, restricted, open]
default: private
groupid:
type: string
pattern: "group:[a-zA-Z0-9._\\-+!~*()']{1,1024}@.*$"
Expand Down
4 changes: 4 additions & 0 deletions docs/_extra/api-reference/schemas/group-update.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ Group:
type: string
description: group description
maxLength: 250
type:
type: string
enum: [private, restricted, open]
default: private
groupid:
type: string
pattern: "group:[a-zA-Z0-9._\\-+!~*()']{1,1024}@.*$"
Expand Down
1 change: 1 addition & 0 deletions h/schemas/api/group.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
},
"description": {"type": "string", "maxLength": GROUP_DESCRIPTION_MAX_LENGTH},
"groupid": {"type": "string", "pattern": GROUPID_PATTERN},
"type": {"enum": ["private", "restricted", "open"]},
}


Expand Down
32 changes: 26 additions & 6 deletions h/views/api/groups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from h.views.api.config import api_config
from h.views.api.exceptions import PayloadError

DEFAULT_GROUP_TYPE = "private"


@api_config(
versions=["v1", "v2"],
Expand Down Expand Up @@ -64,12 +66,29 @@ def create(request):
_("group with groupid '{}' already exists").format(groupid)
)

group = group_create_service.create_private_group(
name=appstruct["name"],
userid=request.user.userid,
description=appstruct.get("description", None),
groupid=groupid,
)
group_type = appstruct.get("type", DEFAULT_GROUP_TYPE)

kwargs = {
"name": appstruct["name"],
"userid": request.user.userid,
"description": appstruct.get("description", None),
"groupid": groupid,
}

if group_type == "private":
method = group_create_service.create_private_group
else:
assert group_type in ("restricted", "open")
kwargs["scopes"] = []

if group_type == "restricted":
method = group_create_service.create_restricted_group
else:
assert group_type == "open"
method = group_create_service.create_open_group

group = method(**kwargs)

return GroupJSONPresenter(group, request).asdict(expand=["organization", "scopes"])


Expand Down Expand Up @@ -171,6 +190,7 @@ def upsert(context: GroupContext, request):
"name": appstruct["name"],
"description": appstruct.get("description", ""),
"groupid": appstruct.get("groupid", None),
"type": appstruct.get("type", DEFAULT_GROUP_TYPE),
}

group = group_update_service.update(group, **update_properties)
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/h/schemas/api/group_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ class TestGroupAPISchema:
{"groupid": "group:1234abcd!~*()@thirdparty.com"},
{"groupid": "group:1234abcd!~*()@thirdparty.com"},
),
( # Valid type.
DEFAULT_AUTHORITY,
{"type": "private"},
{"type": "private"},
),
( # Valid type.
DEFAULT_AUTHORITY,
{"type": "restricted"},
{"type": "restricted"},
),
( # Valid type.
DEFAULT_AUTHORITY,
{"type": "open"},
{"type": "open"},
),
( # All valid fields at once.
"thirdparty.com",
{
Expand Down Expand Up @@ -147,6 +162,18 @@ def test_valid(self, group_authority, data, expected_appstruct):
{"groupid": 42},
"groupid: 42 is not of type 'string'",
),
(
# type not a string.
DEFAULT_AUTHORITY,
{"type": 42},
r"type: 42 is not one of \['private', 'restricted', 'open'\]",
),
(
# Invalid type.
DEFAULT_AUTHORITY,
{"type": "invalid"},
r"type: 'invalid' is not one of \['private', 'restricted', 'open'\]",
),
],
)
def test_invalid(self, group_authority, data, error_message):
Expand Down
148 changes: 135 additions & 13 deletions tests/unit/h/views/api/groups_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ def test_it_when_request_json_body_is_invalid(
"appstruct,group_create_service_method_call",
[
(
{"name": sentinel.name, "description": sentinel.description},
{
"name": sentinel.name,
"description": sentinel.description,
"type": "private",
},
call.create_private_group(
name=sentinel.name,
userid=sentinel.userid,
Expand Down Expand Up @@ -130,6 +134,100 @@ def test_create_private_group(
group=group_create_service.create_private_group.return_value,
)

@pytest.mark.parametrize(
"appstruct,group_create_service_method_call",
[
(
{
"name": sentinel.name,
"description": sentinel.description,
"type": "restricted",
},
call.create_restricted_group(
name=sentinel.name,
userid=sentinel.userid,
scopes=[],
description=sentinel.description,
groupid=None,
),
),
(
{"name": sentinel.name, "type": "restricted"},
call.create_restricted_group(
name=sentinel.name,
userid=sentinel.userid,
scopes=[],
description=None,
groupid=None,
),
),
],
)
def test_create_restricted_group(
self,
pyramid_request,
CreateGroupAPISchema,
group_create_service,
assert_it_returns_group_as_json,
appstruct,
group_create_service_method_call,
):
CreateGroupAPISchema.return_value.validate.return_value = appstruct

response = views.create(pyramid_request)

assert group_create_service.method_calls == [group_create_service_method_call]
assert_it_returns_group_as_json(
response, group=group_create_service.create_restricted_group.return_value
)

@pytest.mark.parametrize(
"appstruct,group_create_service_method_call",
[
(
{
"name": sentinel.name,
"description": sentinel.description,
"type": "open",
},
call.create_open_group(
name=sentinel.name,
userid=sentinel.userid,
scopes=[],
description=sentinel.description,
groupid=None,
),
),
(
{"name": sentinel.name, "type": "open"},
call.create_open_group(
name=sentinel.name,
userid=sentinel.userid,
scopes=[],
description=None,
groupid=None,
),
),
],
)
def test_create_open_group(
self,
pyramid_request,
CreateGroupAPISchema,
group_create_service,
assert_it_returns_group_as_json,
appstruct,
group_create_service_method_call,
):
CreateGroupAPISchema.return_value.validate.return_value = appstruct

response = views.create(pyramid_request)

assert group_create_service.method_calls == [group_create_service_method_call]
assert_it_returns_group_as_json(
response, group=group_create_service.create_open_group.return_value
)

def test_it_with_groupid_request_param(
self, pyramid_request, CreateGroupAPISchema, group_service, group_create_service
):
Expand Down Expand Up @@ -231,15 +329,23 @@ def test_it_when_request_json_body_is_invalid(
{"description": sentinel.description},
call.update(sentinel.group, description=sentinel.description),
),
({"type": "private"}, call.update(sentinel.group, type="private")),
(
{"name": sentinel.name, "description": sentinel.description},
{
"name": sentinel.name,
"description": sentinel.description,
"type": "private",
},
call.update(
sentinel.group, name=sentinel.name, description=sentinel.description
sentinel.group,
name=sentinel.name,
description=sentinel.description,
type="private",
),
),
],
)
def test_update_private_group(
def test_update(
self,
context,
pyramid_request,
Expand Down Expand Up @@ -361,6 +467,7 @@ def test_if_the_group_doesnt_exist_yet_it_creates_it(
name=sentinel.name,
description="",
groupid=None,
type="private",
),
),
(
Expand All @@ -370,11 +477,32 @@ def test_if_the_group_doesnt_exist_yet_it_creates_it(
name=sentinel.name,
description=sentinel.description,
groupid=None,
type="private",
),
),
(
{"name": sentinel.name, "type": "restricted"},
call.update(
sentinel.group,
name=sentinel.name,
description="",
groupid=None,
type="restricted",
),
),
(
{"name": sentinel.name, "type": "open"},
call.update(
sentinel.group,
name=sentinel.name,
description="",
groupid=None,
type="open",
),
),
],
)
def test_update_private_group(
def test_it_updates_an_existing_group(
self,
context,
pyramid_request,
Expand Down Expand Up @@ -587,10 +715,7 @@ def CreateGroupAPISchema(mocker):
CreateGroupAPISchema = mocker.patch(
"h.views.api.groups.CreateGroupAPISchema", autospec=True, spec_set=True
)
CreateGroupAPISchema.return_value.validate.return_value = {
"name": sentinel.name,
"description": sentinel.description,
}
CreateGroupAPISchema.return_value.validate.return_value = {"name": sentinel.name}
return CreateGroupAPISchema


Expand All @@ -599,10 +724,7 @@ def UpdateGroupAPISchema(mocker):
UpdateGroupAPISchema = mocker.patch(
"h.views.api.groups.UpdateGroupAPISchema", autospec=True, spec_set=True
)
UpdateGroupAPISchema.return_value.validate.return_value = {
"name": sentinel.name,
"description": sentinel.description,
}
UpdateGroupAPISchema.return_value.validate.return_value = {}
return UpdateGroupAPISchema


Expand Down

0 comments on commit 69858e1

Please sign in to comment.