Skip to content

Commit

Permalink
Remove unused group upsert API
Browse files Browse the repository at this point in the history
This API was added for our own LMS app:

#5428

But the LMS app doesn't use this API anymore (and actually I'm not sure
it ever did) and it doesn't look like anything else ever used it.

In upcoming PRs we need to make changes to group-related APIs (for
example instead of only a group's creator being able to edit/update a
group, all of a group's owners and admins need to be able to). Let's
first remove this unused API so we don't have to deal with updating it,
especially since this API's code is entangled with other APIs that we do
need to update.

Slack thread: https://hypothes-is.slack.com/archives/C4K6M7P5E/p1724926065440579
  • Loading branch information
seanh committed Oct 3, 2024
1 parent 7ce59ed commit 48fb691
Show file tree
Hide file tree
Showing 13 changed files with 2 additions and 601 deletions.
28 changes: 0 additions & 28 deletions docs/_extra/api-reference/hypothesis-v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -846,34 +846,6 @@ paths:
schema:
$ref: '#/components/schemas/Group'

# ---------------------------------------------------------------------
# PUT groups/{id} - Replace (Create or Update) a Group, a.k.a. "upsert"
# ---------------------------------------------------------------------
put:
tags:
- groups
summary: Create or Update a Group
description: |
Update the group with the indicated `id` or create one if it does
not exist.
security:
- ApiKey: []
- AuthClientForwardedUser: []
requestBody:
description: Full representation of Group resource
required: true
content:
application/*json:
schema:
$ref: '#/components/schemas/GroupCreate'
responses:
'200':
description: Success
content:
application/json:
schema:
$ref: '#/components/schemas/Group'

# ---------------------------------------------------------------------------
# Operations on Group Membership
# ---------------------------------------------------------------------------
Expand Down
28 changes: 0 additions & 28 deletions docs/_extra/api-reference/hypothesis-v2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -844,34 +844,6 @@ paths:
schema:
$ref: '#/components/schemas/Group'

# ---------------------------------------------------------------------
# PUT groups/{id} - Replace (Create or Update) a Group, a.k.a. "upsert"
# ---------------------------------------------------------------------
put:
tags:
- groups
summary: Create or Update a Group
description: |
Update the group with the indicated `id` or create one if it does
not exist.
security:
- ApiKey: []
- AuthClientForwardedUser: []
requestBody:
description: Full representation of Group resource
required: true
content:
application/*json:
schema:
$ref: '#/components/schemas/GroupCreate'
responses:
'200':
description: Success
content:
application/json:
schema:
$ref: '#/components/schemas/Group'

# ---------------------------------------------------------------------------
# Operations on Group Membership
# ---------------------------------------------------------------------------
Expand Down
7 changes: 0 additions & 7 deletions h/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ def includeme(config): # pylint: disable=too-many-statements
)

config.add_route("api.groups", "/api/groups", factory="h.traversal.GroupRoot")
config.add_route(
"api.group_upsert",
"/api/groups/{id}",
request_method="PUT",
factory="h.traversal.GroupRoot",
traverse="/{id}",
)
config.add_route(
"api.group",
"/api/groups/{id}",
Expand Down
4 changes: 0 additions & 4 deletions h/security/permission_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@
],
Permission.Group.MEMBER_ADD: [[p.group_matches_authenticated_client_authority]],
Permission.Group.MODERATE: [[p.group_created_by_user]],
Permission.Group.UPSERT: [
[p.group_created_by_user],
[p.group_not_found, p.authenticated_user],
],
# --------------------------------------------------------------------- #
# Annotations
Permission.Annotation.CREATE: [[p.authenticated]],
Expand Down
3 changes: 0 additions & 3 deletions h/security/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ class Group(Enum):
EDIT = "group:edit"
"""Update the details of a group."""

UPSERT = "group:upsert"
"""Update the details of a group or create a new one."""

FLAG = "group:flag"
"""Mark annotations in this group as inappropriate for moderators."""

Expand Down
1 change: 0 additions & 1 deletion h/security/policy/_auth_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class AuthClientPolicy:
("api.group", "GET"),
("api.group_member", "POST"),
("api.group_members", "GET"),
("api.group_upsert", "PUT"),
("api.users", "POST"),
("api.user_read", "GET"),
("api.user", "PATCH"),
Expand Down
4 changes: 0 additions & 4 deletions h/security/predicates.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ def group_found(_identity, context):
return hasattr(context, "group") and context.group


def group_not_found(_identity, context):
return not hasattr(context, "group") or not context.group


@requires(group_found)
def group_writable_by_members(_identity, context):
return context.group.writeable_by == WriteableBy.members
Expand Down
58 changes: 0 additions & 58 deletions h/views/api/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,64 +140,6 @@ def update(context: GroupContext, request):
return GroupJSONPresenter(group, request).asdict(expand=["organization", "scopes"])


@api_config(
versions=["v1", "v2"],
route_name="api.group_upsert",
request_method="PUT",
permission=Permission.Group.UPSERT,
link_name="group.create_or_update",
description="Create or update a group",
)
def upsert(context: GroupContext, request):
"""
Create or update a group from a PUT payload.
If no group model is present in the passed ``context`` (on ``context.group``),
treat this as a create action and delegate to ``create``.
Otherwise, replace the existing group's resource properties entirely and update
the object.
"""
if context.group is None:
return create(request)

group = context.group

# Because this is a PUT endpoint and not a PATCH, a full replacement of the
# entire resource is expected. Thus, we're validating against the Create schema
# here as we want to make sure properties required for a fresh object are present
appstruct = CreateGroupAPISchema(
default_authority=request.default_authority,
group_authority=request.effective_authority,
).validate(_json_payload(request))

group_update_service = request.find_service(name="group_update")
group_service = request.find_service(name="group")

# Check for duplicate group
groupid = appstruct.get("groupid", None)
if groupid is not None:
duplicate_group = group_service.fetch(pubid_or_groupid=groupid)
if duplicate_group and (duplicate_group != group):
raise HTTPConflict(
_("group with groupid '{}' already exists").format(groupid)
)

# Need to make sure every resource-defined property is present, as this
# is meant as a full-resource-replace operation.
# TODO: This may be better handled in the schema at some point
update_properties = {
"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)

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


@api_config(
versions=["v1", "v2"],
route_name="api.group_members",
Expand Down
Loading

0 comments on commit 48fb691

Please sign in to comment.