Skip to content

Commit

Permalink
Remove legacy unpaginated list-group-members API
Browse files Browse the repository at this point in the history
  • Loading branch information
@seanh
seanh committed Dec 4, 2024
1 parent 45f15ae commit ac6c469
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 184 deletions.
44 changes: 8 additions & 36 deletions h/views/api/group_members.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import logging

from pyramid.config import not_
from pyramid.httpexceptions import HTTPNoContent, HTTPNotFound

from h.presenters import GroupMembershipJSONPresenter
Expand All @@ -15,41 +14,14 @@
log = logging.getLogger(__name__)


LIST_MEMBERS_API_CONFIG = {
"versions": ["v1", "v2"],
"route_name": "api.group_members",
"request_method": "GET",
"link_name": "group.members.read",
"description": "Fetch a list of all members of a group",
"permission": Permission.Group.READ,
}


@api_config(request_param=not_("page[offset]"), **LIST_MEMBERS_API_CONFIG)
def list_members_legacy(context: GroupContext, request):
"""Legacy version of the list-members API, maintained for backwards-compatibility."""

log.info(
"list_members_legacy() was called. User-Agent: %s, Referer: %s, pubid: %s",
request.headers.get("User-Agent"),
request.headers.get("Referer"),
context.group.pubid,
)

# Get the list of memberships from GroupMembersService instead of just
# accessing `context.memberships` because GroupMembersService returns the
# memberships explictly sorted by username whereas `context.memberships` is
# unsorted.
group_members_service = request.find_service(name="group_members")
memberships = group_members_service.get_memberships(context.group)

return [
GroupMembershipJSONPresenter(request, membership).asdict()
for membership in memberships
]


@api_config(request_param="page[offset]", **LIST_MEMBERS_API_CONFIG)
@api_config(
versions=["v1", "v2"],
route_name="api.group_members",
request_method="GET",
link_name="group.members.read",
description="Fetch a list of all members of a group",
permission=Permission.Group.READ,
)
def list_members(context: GroupContext, request):
group = context.group
group_members_service = request.find_service(name="group_members")
Expand Down
130 changes: 31 additions & 99 deletions tests/functional/api/groups/members_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,100 +8,6 @@
from h.models.auth_client import AuthClient, GrantType


class TestListMembersLegacy:
def test_it_returns_list_of_members_for_restricted_group_without_authn(
self, app, factories, db_session, caplog
):
group = factories.RestrictedGroup(
memberships=[
GroupMembership(user=user)
for user in factories.User.create_batch(size=3)
]
)
db_session.commit()

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
headers={"User-Agent": "test_user_agent", "Referer": "test_referer"},
)

assert caplog.messages == [
f"list_members_legacy() was called. User-Agent: test_user_agent, Referer: test_referer, pubid: {group.pubid}"
]
assert res.status_code == 200
assert res.json == [
{
"authority": membership.group.authority,
"userid": membership.user.userid,
"username": membership.user.username,
"display_name": membership.user.display_name,
"roles": membership.roles,
"actions": [],
}
for membership in sorted(
group.memberships, key=lambda membership: membership.user.username
)
]

def test_it_returns_list_of_members_if_user_has_access_to_private_group(
self, app, factories, db_session
):
group = factories.Group()
user, other_user = factories.User.create_batch(size=2)
token = factories.DeveloperToken(user=user)
group.memberships.extend(
[GroupMembership(user=user), GroupMembership(user=other_user)]
)
db_session.commit()

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
headers=token_authorization_header(token),
)

assert res.status_code == 200
assert res.json == sorted(
[
{
"authority": group.authority,
"userid": user.userid,
"username": user.username,
"display_name": user.display_name,
"roles": [GroupMembershipRoles.MEMBER],
"actions": ["delete"],
},
{
"authority": group.authority,
"userid": other_user.userid,
"username": other_user.username,
"display_name": other_user.display_name,
"roles": [GroupMembershipRoles.MEMBER],
"actions": [],
},
],
key=lambda membership: membership["username"],
)

def test_it_returns_404_if_user_does_not_have_read_access_to_group(
self, app, db_session, factories
):
group = factories.Group()
db_session.commit()

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
headers=token_authorization_header(factories.DeveloperToken()),
expect_errors=True,
)

assert res.status_code == 404

def test_it_returns_empty_list_if_no_members_in_group(self, app):
res = app.get("/api/groups/__world__/members")

assert res.json == []


class TestListMembers:
def test_it_returns_list_of_members_for_restricted_group_without_auth(
self, app, factories, db_session
Expand All @@ -116,7 +22,6 @@ def test_it_returns_list_of_members_for_restricted_group_without_auth(

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
params={"page[offset]": 1, "page[limit]": 2},
headers={"User-Agent": "test_user_agent", "Referer": "test_referer"},
)

Expand All @@ -134,7 +39,7 @@ def test_it_returns_list_of_members_for_restricted_group_without_auth(
}
for membership in sorted(
group.memberships, key=lambda membership: membership.user.username
)[1:3]
)
],
}

Expand All @@ -151,7 +56,6 @@ def test_it_returns_list_of_members_if_user_has_access_to_private_group(

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
params={"page[offset]": 0},
headers=token_authorization_header(token),
)

Expand Down Expand Up @@ -189,7 +93,6 @@ def test_it_returns_404_if_user_does_not_have_read_access_to_group(

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
params={"page[offset]": 0},
headers=token_authorization_header(factories.DeveloperToken()),
expect_errors=True,
)
Expand All @@ -199,11 +102,40 @@ def test_it_returns_404_if_user_does_not_have_read_access_to_group(
def test_it_returns_empty_list_if_no_members_in_group(self, app):
res = app.get(
"/api/groups/__world__/members",
params={"page[offset]": 0},
)

assert res.json == {"meta": {"page": {"total": 0}}, "data": []}

def test_pagination(self, app, factories, db_session):
group = factories.RestrictedGroup(
memberships=[
GroupMembership(user=user)
for user in factories.User.create_batch(size=4)
]
)
db_session.commit()

res = app.get(
"/api/groups/{pubid}/members".format(pubid=group.pubid),
params={"page[offset]": 1, "page[limit]": 2},
headers={"User-Agent": "test_user_agent", "Referer": "test_referer"},
)

assert res.json["meta"]["page"]["total"] == 4
assert res.json["data"] == [
{
"authority": membership.group.authority,
"userid": membership.user.userid,
"username": membership.user.username,
"display_name": membership.user.display_name,
"roles": membership.roles,
"actions": [],
}
for membership in sorted(
group.memberships, key=lambda membership: membership.user.username
)[1:3]
]

def test_it_returns_an_error_if_offset_and_limit_are_invalid(
self, app, db_session, factories
):
Expand Down
49 changes: 0 additions & 49 deletions tests/unit/h/views/api/group_members_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,6 @@
from h.views.api.exceptions import PayloadError


class TestListMembersLegacyLegacy:
def test_it(
self,
context,
pyramid_request,
GroupMembershipJSONPresenter,
group_members_service,
caplog,
):
pyramid_request.headers["User-Agent"] = sentinel.user_agent
pyramid_request.headers["Referer"] = sentinel.referer
group_members_service.get_memberships.return_value = [
sentinel.membership_1,
sentinel.membership_2,
]

presenter_instances = GroupMembershipJSONPresenter.side_effect = [
create_autospec(
presenters.GroupMembershipJSONPresenter, instance=True, spec_set=True
),
create_autospec(
presenters.GroupMembershipJSONPresenter, instance=True, spec_set=True
),
]

response = views.list_members_legacy(context, pyramid_request)

assert caplog.messages == [
f"list_members_legacy() was called. User-Agent: {sentinel.user_agent}, Referer: {sentinel.referer}, pubid: {context.group.pubid}"
]
group_members_service.get_memberships.assert_called_once_with(context.group)
assert GroupMembershipJSONPresenter.call_args_list == [
call(pyramid_request, sentinel.membership_1),
call(pyramid_request, sentinel.membership_2),
]
presenter_instances[0].asdict.assert_called_once_with()
presenter_instances[1].asdict.assert_called_once_with()
assert response == [
presenter_instances[0].asdict.return_value,
presenter_instances[1].asdict.return_value,
]

@pytest.fixture
def context(self, factories):
return create_autospec(
GroupContext, instance=True, spec_set=True, group=factories.Group()
)


class TestListMembers:
def test_it(
self,
Expand Down

0 comments on commit ac6c469

Please sign in to comment.