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:
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 withOR
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:
-
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.
- This function mirrors the existing
-
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
-
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.
- Modified to follow a similar pattern as
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)