Skip to content

Commit

Permalink
fix(gitlab): Fix linking of closed Merge Requests and Milestones
Browse files Browse the repository at this point in the history
Insertion of links for GitLab Merge Request and Milestone references did not work when the Merge Request or Milestone was closed, because query by default only return open items.

This fix adjusts the queries sent to GitLab to include all Merge Requests and Milestones with a given id regardless of the item's state.
  • Loading branch information
ap0llo committed Apr 15, 2020
1 parent fbceab4 commit 238c5f6
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 7 deletions.
71 changes: 67 additions & 4 deletions src/ChangeLog.Test/Integrations/GitLab/GitLabLinkTaskTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Reflection;
using System.Threading.Tasks;
using GitLabApiClient;
using GitLabApiClient.Internal.Paths;
Expand Down Expand Up @@ -64,6 +65,43 @@ private ProjectId MatchProjectId(string expected)
return It.Is<ProjectId>((ProjectId actual) => actual.ToString() == ((ProjectId)expected).ToString());
}

/// <summary>
/// Helper method to test if an Action performs the expected changes to an object
/// </summary>
/// <typeparam name="T">The type of object the changes are applied to.</typeparam>
/// <param name="actionToVerify">The action to apply to the object.</param>
/// <param name="assertions">The assertions to apply to the object after applying <paramref name="actionToVerify"/></param>
private bool AssertAction<T>(Action<T> actionToVerify, params Action<T>[] assertions)
{
// I'm not sure this is a good idea
// but the only way we can make any assertions about an action
// is calling it an afterwards checking if it made the expected
// changes to the instance.
// The instance in turn needs to be created through reflection because
// GitLabApiClient's query types are sealed with an internal constructor

var type = typeof(T);
var instance = (T)type.Assembly.CreateInstance(
type.FullName!, false,
BindingFlags.Instance | BindingFlags.NonPublic,
null, null, null, null)!;

if (instance == null)
throw new InvalidOperationException($"Failed to create instance of '{type.FullName}'");

// Apply the action we want to verify
actionToVerify.Invoke(instance);

// Apply the assertions to the instance
foreach (var assertion in assertions)
{
assertion.Invoke(instance);
}

return true;
}


[Fact]
public async Task Run_does_nothing_if_repository_does_not_have_remotes()
{
Expand Down Expand Up @@ -355,8 +393,21 @@ public async Task Run_adds_merge_request_links_to_footers(string footerText, int

});

m_MergeRequestsClientMock.Verify(x => x.GetAsync(It.IsAny<ProjectId>(), It.IsAny<Action<ProjectMergeRequestsQueryOptions>>()), Times.Once);
m_MergeRequestsClientMock.Verify(x => x.GetAsync(MatchProjectId(projectPath), It.IsAny<Action<ProjectMergeRequestsQueryOptions>>()), Times.Once);
m_MergeRequestsClientMock.Verify(
x => x.GetAsync(It.IsAny<ProjectId>(), It.IsAny<Action<ProjectMergeRequestsQueryOptions>>()),
Times.Once);

m_MergeRequestsClientMock.Verify(
x => x.GetAsync(
MatchProjectId(projectPath),
It.Is<Action<ProjectMergeRequestsQueryOptions>>(
action =>
AssertAction(
action,
opts => Assert.Equal(id, Assert.Single(opts.MergeRequestsIds)),
opts => Assert.Equal(QueryMergeRequestState.All, opts.State)
))),
Times.Once);
}

[Theory]
Expand Down Expand Up @@ -473,8 +524,20 @@ public async Task Run_adds_milestone_links_to_footers(string footerText, int id,

});

m_ProjectsClientMock.Verify(x => x.GetMilestonesAsync(It.IsAny<ProjectId>(), It.IsAny<Action<MilestonesQueryOptions>>()), Times.Once);
m_ProjectsClientMock.Verify(x => x.GetMilestonesAsync(MatchProjectId(projectPath), It.IsAny<Action<MilestonesQueryOptions>>()), Times.Once);
m_ProjectsClientMock.Verify(
x => x.GetMilestonesAsync(It.IsAny<ProjectId>(), It.IsAny<Action<MilestonesQueryOptions>>()),
Times.Once);

m_ProjectsClientMock.Verify(
x => x.GetMilestonesAsync(
MatchProjectId(projectPath),
It.Is<Action<MilestonesQueryOptions>>(action =>
AssertAction(
action,
opts => Assert.Equal(id, Assert.Single(opts.MilestoneIds)),
opts => Assert.Equal(MilestoneState.All, opts.State)
))),
Times.Once);
}

[Theory]
Expand Down
14 changes: 11 additions & 3 deletions src/ChangeLog/Integrations/GitLab/GitLabLinkTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using GitLabApiClient;
using GitLabApiClient.Models.MergeRequests.Requests;
using GitLabApiClient.Models.Milestones.Responses;
using Grynwald.ChangeLog.Git;
using Grynwald.ChangeLog.Model;
using Grynwald.ChangeLog.Tasks;
Expand Down Expand Up @@ -227,7 +229,12 @@ private bool TryParseReference(string input, [NotNullWhen(true)] out GitLabRefer

try
{
var mergeRequests = await gitlabClient.MergeRequests.GetAsync(projectPath, query => { query.MergeRequestsIds.Add(id); });
var mergeRequests = await gitlabClient.MergeRequests.GetAsync(projectPath, queryOptions =>
{
queryOptions.MergeRequestsIds = new[] { id };
queryOptions.State = QueryMergeRequestState.All;
});

if (mergeRequests.Count == 1)
{
return new Uri(mergeRequests.Single().WebUrl); ;
Expand Down Expand Up @@ -255,9 +262,10 @@ private bool TryParseReference(string input, [NotNullWhen(true)] out GitLabRefer
// within the project, but some other id.
// Instead, use GetMilestonesAsync() and query for a single milestone id
// for which we can use the id in the reference.
var milestones = await gitlabClient.Projects.GetMilestonesAsync(projectPath, options =>
var milestones = await gitlabClient.Projects.GetMilestonesAsync(projectPath, queryOptions =>
{
options.MilestoneIds = new[] { id };
queryOptions.MilestoneIds = new[] { id };
queryOptions.State = MilestoneState.All;
});

if (milestones.Count == 1)
Expand Down

0 comments on commit 238c5f6

Please sign in to comment.