fix(api): enable group MR listing with reviewer and assignee filters

PR Summary: Fix Group MR Listing with Reviewer and Assignee Filters

Problem

When listing MRs for a group with reviewer and/or assignee filters, the group flag is ignored.

i.e.

glab mr list --group=gitlab-community --reviewer=Taucher2003

ignores --group and just fetches through the default project mr endpoint with a reviewer filter.

See issue #7597 (closed)

Technical Notes

Click to expand technical notes

The cause of this issue can be traced to a conditional in:

https://gitlab.com/gitlab-community/cli/-/blob/main/commands/mr/list/mr_list.go?ref_type=heads#L252-262

	if len(assigneeIds) > 0 || len(reviewerIds) > 0 {
		mergeRequests, err = api.ListMRsWithAssigneesOrReviewers(apiClient, repo.FullName(), l, assigneeIds, reviewerIds)
	} else if opts.Group != "" {
		mergeRequests, err = api.ListGroupMRs(apiClient, opts.Group, api.ProjectListMROptionsToGroup(l))
		title.RepoName = opts.Group
	} else {
		mergeRequests, err = api.ListMRs(apiClient, repo.FullName(), l)
	}
	if err != nil {
		return err
	}

Before we check for the presence of a Group option, we fork in the conditional if there are assigneeIds or reviewerIds and call the repo based ListMRsWithAssigneesOrReviewers. We should not be assuming that the presence of assigneeIds or reviewerIds precludes the possibility of an option to filter by group.

In addition, we also need to the ListGroupMRs function to receive and act on assigneeIds and reviewerIds filter options. We should look to the existing ListMRsWithAssigneesOrReviewers function to understand how this should happen:

https://gitlab.com/gitlab-community/cli/-/blob/main/api/merge_request.go?ref_type=heads#L109-136

var ListMRsWithAssigneesOrReviewers = func(client *gitlab.Client, projectID interface{}, opts *gitlab.ListProjectMergeRequestsOptions, assigneeIds []int, reviewerIds []int) ([]*gitlab.MergeRequest, error) {
	if client == nil {
		client = apiClient.Lab()
	}
	if opts.PerPage == 0 {
		opts.PerPage = DefaultListLimit
	}

	mrs := make([]*gitlab.MergeRequest, 0)
	for _, id := range assigneeIds {
		opts.AssigneeID = gitlab.AssigneeID(id)
		assingeMrs, err := ListMRs(client, projectID, opts)
		if err != nil {
			return nil, err
		}
		mrs = append(mrs, assingeMrs...)
	}
	opts.AssigneeID = nil // reset because it's Assignee OR Reviewer
	for _, id := range reviewerIds {
		opts.ReviewerID = gitlab.ReviewerID(id)
		reviewerMrs, err := ListMRs(client, projectID, opts)
		if err != nil {
			return nil, err
		}
		mrs = append(mrs, reviewerMrs...)
	}
	return mrs, nil
}

Fetching a project's MRs with an assignee or reviewer filter is not just a matter of adding query options here. This is complicated by two factors:

• The existing API endpoint only accepts a single reviewerId and a single assigneeId
• The existing API endpoint combines filters with AND logic, where the cli intended usage seems to be work by combining filter flags with OR logic.

Because of this, the above function iterates through all the provided reviewerIds and assigneeIds, making a request for each one.

We can mirror this logic and implement a ListGroupMRsWithAssigneesOrReviewers function that hits /groups/:group_id/merge_requests instead of /projects/:project_id/merge_requests.

With the addition of this function, the conditionals in mr_list.go may become more complicated than we want. A potential refactor is to delegate part of this conditional to ListMRs and ListGroupMRs by passing optional reviewerIds and assigneeIds arguments. If these optional arguments are received, then these parent functions will fork to ListMRsWithAssigneesOrReviewers or ListGroupMRsWithAssigneesOrReviewers. Otherwise, these functions will delegate to another function with the default fetch logic. This will allow consumers of the api package to call these functions without the additional conditionals checking reviewer and assignee options.

Solution

To address this issue, we implemented the following changes:

1. New Function: ListGroupMRsWithAssigneesOrReviewers

   • This function mirrors the existing ListMRsWithAssigneesOrReviewers but is specifically for group MRs.
   • It allows filtering of group MRs by both assignee and reviewer IDs.

2. Refactored ListGroupMRs:

   • Updated to check for assignee or reviewer IDs.
   • If assignee or reviewer is present, it calls the new ListGroupMRsWithAssigneesOrReviewers function.
   • If not, it falls back to the existing list group MRs function, which has been renamed to ListGroupMRsBase function

3. Updated ListMRs:

   • Modified to follow a similar pattern as ListGroupMRs for consistency.
   • This ensures that both project and group MR listing functions handle filters similarly.

Manual Testing

I used the gitlab-community group for testing: https://gitlab.com/groups/gitlab-community/-/merge_requests

I have an alias of glab-dev-cli that executes my local dev cli.

main	this feature branch
main-glab-cli	feature-branch-glab-cli

commands:

glab-dev-cli mr list --group gitlab-community --reviewer=Taucher2003
glab-dev-cli mr list --group gitlab-community --assignee=leetickett-gitlab
glab-dev-cli mr list --group gitlab-community --reviewer=Taucher2003 --assignee=leetickett-gitlab

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation
• Chore (Related to CI or Packaging to platforms)
• Test gap

Closes #7597 (closed)