[#38672] Project members shown repetitively in member page if a group filter is active

[fraber]

https://community.openproject.org/work_packages/38672

… (and the user is in several groups that have a membership role).

Added a "select distinct * from (...)" around the results of MemberQuery in order to eliminate multiple rows returned by the group filter in MemberQuery if a user belongs to multiple groups. Also added a brand new spec for MemberQuery testing the happy path and the regression of this bug.

[fraber]

Hi @ulferts , Wieland mentioned it would be a good idea if you could have a look at this PR.

[ulferts]

Hi @fraber applying DISTINCT to a query has the disadvantage in PostgreSQL that every field listed in the ORDER BY of the query will have to appear in the list that is checked DISTINCT on (e.g. SELECT DISTINCT id FROM members ORDER BY created_at will break). Applying a distinct will in my experience result in added complexity when one tries to either remove the distinct again to get around the limitation or one tries to manipulate to projection part.

In this case, the fix might be simpler though I don't know if anything will break:
In my very brief test, removing all of https://github.com/opf/openproject/blob/dev/app/models/queries/members/filters/group_filter.rb#L32-L39 fixed the problem. I assume (don't know any more since it has been a while) that the included Queries::Filters::Shared::GroupFilter used to be implemented differently. The uselessly overwritten joins method strongly suggests that.

[jngrb]

@wielinde, @fraber, @ulferts, as creator of the bug report OPENPROJECT 38672, I would like to know if this PR is ready to be merged or if there are still changes to be made.

[wielinde]

@jngrb No, unfortunately it is not ready to merge. The review feedback from @ulferts is valid and needs to be addressed first by @fraber.

[fraber]

@jngrb , sorry, vacation time has started. I'd expect me/us to finish the issue in about 3-4 weeks (mid-August).

[fraber]

@ulferts , I implemented your proposed change for Queries::Members::Filters::GroupFilters above. Your suggestions successfully fixed the reported issue in the /projects/ID/members page.

Then I did quite a bit of manual testing in all places where GroupFilter was used and found that your proposed change broke the API call /api/v3/memberships when using with filters=[{"group":{"operator":"=","values":["17"]}}]:

PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "users"
LINE 1: SELECT COUNT(*) FROM "members" WHERE (users.id IN (SELECT "u...

So I've added the required .joins(:principal) back to Queries::Members::Filters::GroupFilter, together with the other options from the base class to handle from, joins and left_outer_joins (not sure about this...)

I did another round of testing to make sure that both the breaking API and the regression case are covered by specs (spec/models/queries/members/member_query_spec.rb).

After a rebase to current dev I got an issue manually testing in /projects//members with the same error message as in the API call above. The .joins(:principal) fixed this in the same way as it did with the API call. However, this issue disappeared after a while, so I couldn't write a spec for it. I understand the issue would be detected by the spec/feature/members/ suite.