Parametrize test

[iaindillingham]

We parametrize the test to demonstrate that the filter operates on global and local roles. Ideally, the test wouldn't depend on us knowing that OutputPublisher is a global role and ProjectDeveloper is a local role, but making it so would add (arguably unnecessary) complexity. To clarify our intent, we assign descriptive IDs to each parameter set using pytest.param.

@KatieB5 and I parametrized the test when considering #4632. We didn't need to do so to address that issue, but running the test once for a global role and once for a local role has the benefit of documenting how we expect UserList to behave.

[mikerkelly]

We parametrize the test to demonstrate that the filter operates on global and local roles.

It's somewhat surprising that this behavior persists, perhaps enough to warrant a comment in the test. The roles in the view's context only include global roles (as of #4632). Under the current template design, a real user won't normally submit a request with a non-global role. However, the get_queryset() code in the view will still filter by a local role if one is provided in a GET request, and this test demonstrates that.

Ideally, the test wouldn't depend on us knowing that OutputPublisher is a global role and ProjectDeveloper is a local role, but making it so would add (arguably unnecessary) complexity.

This seems fine for this test. I imagine there may be other tests that want a User assigned an arbitrary global or non-global role? I wonder if UserFactory could take an option to generate such a User. This would centralize the dependency somewhat, and it could be tested against GLOBAL_ROLES from #4632.

[mikerkelly]

I can see we use 'local' as an antonym to 'global' for ProjectMembership-based roles vs. User-based roles in a few places. I find this wording a little tough to intuit the meaning of and wonder if "user-based" vs "project-based" might be simpler. Or "global" vs "non-global" or "project" vs "non-project". Hmm.

I'm not suggesting this needs to change here.

[iaindillingham]

Yup, we do. And yup, "global" vs "non-global" makes sense; as does "project" vs "non-project". (We also implemented, but don't actually use, OrgMembership-based roles, just to 🤯.)

[iaindillingham]

I imagine there may be other tests that want a User assigned an arbitrary global or non-global role?

I imagine so, too. Rather than updating UserFactory, we could create a new factory (e.g. UserWithGlobalRoleFactory) and, as you say, centralise the dependency. Thanks for the suggestion.