Skip to content

Commit

Permalink
Fix factories.Group creator membership
Browse files Browse the repository at this point in the history
When creating a group, the user `my_group.creator` should also be added
as the first user in `my_group.members`. With `factories.Group` this
currently works correctly as long as no `members` argument is passed to
the factory:

    >>> group = factories.Group()
    >>> group.creator
    <User: cookekathleen__0>
    >>> group.members
    [<User: cookekathleen__0>]
    >>> user = factories.User()
    >>> group = factories.Group(creator=user)
    >>> group.creator
    <User: pmitchell__1>
    >>> group.members
    [<User: pmitchell__1>]

But when a `members` argument is passed to the factory a group can be
created with `group.creator` missing from `group.members`, which is
abnormal:

    >>> group = factories.Group(members=[user])
    >>> group.creator
    <User: arthurturner__2>
    >>> group.members
    [<User: pmitchell__1>]

Fix `factories.Group` to ensure that `group.creator` is always added as
the first user in `group.members` even if a `members` list that does not
include the creator is passed to the factory.

Tests of the new version:

    >>> group = factories.Group()
    >>> group.creator
    <User: abeasley__0>
    >>> group.members
    [<User: abeasley__0>]
    >>> user = factories.User()
    >>> group = factories.Group(members=[user])
    >>> group.creator
    <User: kristinegutierrez__2>
    >>> group.members
    [<User: kristinegutierrez__2>, <User: kristina79__1>]
    >>> group = factories.Group(creator=user, members=[user])
    >>> group.creator
    <User: kristina79__1>
    >>> group.members
    [<User: kristina79__1>]
  • Loading branch information
seanh committed May 16, 2024
1 parent 00fcdb8 commit 088925d
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
21 changes: 20 additions & 1 deletion tests/common/factories/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Meta:
joinable_by = JoinableBy.authority
readable_by = ReadableBy.members
writeable_by = WriteableBy.members
members = factory.LazyAttribute(lambda obj: [obj.creator])
members = factory.LazyFunction(list)
authority_provided_id = Faker("hexify", text="^" * 30)
enforce_scope = True

Expand All @@ -33,6 +33,17 @@ def scopes( # pylint: disable=method-hidden,unused-argument

self.scopes = scopes or []

@factory.post_generation
def add_creator_as_member( # pylint:disable=no-self-argument
obj, _create, _extracted, **_kwargs
):
if (
obj.creator
and obj.creator
not in obj.members # pylint:disable=unsupported-membership-test
):
obj.members.insert(0, obj.creator) # pylint:disable=no-member


class OpenGroup(Group):
name = factory.Sequence(lambda n: f"Open Group {n}")
Expand All @@ -42,6 +53,14 @@ class OpenGroup(Group):
writeable_by = WriteableBy.authority
members = []

@factory.post_generation
def add_creator_as_member( # pylint:disable=no-self-argument
_obj, _create, _extracted, **_kwargs
):
# Open groups don't have members, so don't add group.creator to
# group.members for open groups.
pass


class RestrictedGroup(Group):
name = factory.Sequence(lambda n: f"Restricted Group {n}")
Expand Down
9 changes: 6 additions & 3 deletions tests/unit/h/services/group_list_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ def test_it_does_not_return_private_groups_if_user_is_not_member(
# the creator of the group. That means that this private group is still associated
# with this user in some form—but we want to make sure it does not appear
# in these results.
private_group = factories.Group(
creator=user, authority=user.authority, members=[]
)
private_group = factories.Group(creator=user, authority=user.authority)
# Remove `private_group.creator` from `private_group.members`.
# The creator is still attached to `private_group` as `private_group.creator`.
private_group.members = [
user for user in private_group.members if user is not private_group.creator
]

groups = svc.associated_groups(user)

Expand Down
6 changes: 2 additions & 4 deletions tests/unit/h/views/activity_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1208,8 +1208,7 @@ def group(factories):

@pytest.fixture
def no_creator_group(factories):
group = factories.Group()
group.creator = None
group = factories.Group(creator=None)
group.members.extend([factories.User(), factories.User()])
return group

Expand All @@ -1236,8 +1235,7 @@ def restricted_group(factories):

@pytest.fixture
def no_creator_open_group(factories):
open_group = factories.OpenGroup()
open_group.creator = None
open_group = factories.OpenGroup(creator=None)
return open_group


Expand Down

0 comments on commit 088925d

Please sign in to comment.