Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pullrequestmerged #936

Closed
wants to merge 6 commits into from
Closed

Pullrequestmerged #936

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
55632ee
added "Merged" in searchissues which allows search repos by merged da…
poisonous-milk Oct 9, 2015
fc1b448
added tests for the merged qualifier
poisonous-milk Oct 9, 2015
e315a96
comment out new tests.
poisonous-milk Oct 9, 2015
7e6fbd4
recovered tests
poisonous-milk Oct 9, 2015
bc8c502
Merged property will be set according to MergedAt now. It is not alwa…
poisonous-milk Oct 10, 2015
7d4b525
removed merged property for PR. also removed related tests for that p…
poisonous-milk Oct 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions Octokit.Tests/Clients/SearchClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,78 @@ public void TestingTheCreatedQualifier_Between()
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+created:2014-01-01..2014-02-02"));
}
[Fact]
public void TestingTheMergedQualifier_GreaterThan()
{
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Merged = DateRange.GreaterThan(new DateTime(2014, 1, 1));

client.SearchIssues(request);

connection.Received().Get<SearchIssuesResult>(
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+merged:>2014-01-01"));
}

[Fact]
public void TestingTheMergedQualifier_GreaterThanOrEquals()
{
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Merged = DateRange.GreaterThanOrEquals(new DateTime(2014, 1, 1));

client.SearchIssues(request);

connection.Received().Get<SearchIssuesResult>(
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+merged:>=2014-01-01"));
}

[Fact]
public void TestingTheMergedQualifier_LessThan()
{
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Merged = DateRange.LessThan(new DateTime(2014, 1, 1));

client.SearchIssues(request);

connection.Received().Get<SearchIssuesResult>(
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+merged:<2014-01-01"));
}
[Fact]
public void TestingTheMergedQualifier_LessThanOrEquals()
{
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Merged = DateRange.LessThanOrEquals(new DateTime(2014, 1, 1));

client.SearchIssues(request);

connection.Received().Get<SearchIssuesResult>(
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+merged:<=2014-01-01"));
}
[Fact]
public void TestingTheMergedQualifier_Between()
{
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Merged = DateRange.Between(new DateTime(2014, 1, 1), new DateTime(2014, 2, 2));

client.SearchIssues(request);

connection.Received().Get<SearchIssuesResult>(
Arg.Is<Uri>(u => u.ToString() == "search/issues"),
Arg.Is<Dictionary<string, string>>(d => d["q"] == "something+merged:2014-01-01..2014-02-02"));
}
[Fact]
public void TestingTheUpdatedQualifier_GreaterThan()
{
Expand Down
22 changes: 19 additions & 3 deletions Octokit/Models/Request/SearchIssuesRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public SearchIssuesRequest(string term, string owner, string name)
}

/// <summary>
/// Optional Sort field. One of comments, created, or updated.
/// Optional Sort field. One of comments, created, updated,or merged
/// If not provided, results are sorted by best match.
/// </summary>
/// <remarks>
Expand Down Expand Up @@ -177,6 +177,13 @@ public IEnumerable<string> Labels
/// </remarks>
public DateRange Updated { get; set; }

/// <summary>
/// Filters issues based on times when they were last merged
/// </summary>
/// <remarks>
/// https://help.github.com/articles/searching-issues/#search-based-on-when-a-pull-request-was-merged
/// </remarks>
public DateRange Merged { get; set; }
/// <summary>
/// Filters issues based on the quantity of comments.
/// </summary>
Expand All @@ -196,6 +203,7 @@ public IEnumerable<string> Labels
[SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")]
public RepositoryCollection Repos { get; set; }

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity")]
public override IReadOnlyList<string> MergedQualifiers()
{
var parameters = new List<string>();
Expand Down Expand Up @@ -261,7 +269,10 @@ public override IReadOnlyList<string> MergedQualifiers()
{
parameters.Add(String.Format(CultureInfo.InvariantCulture, "updated:{0}", Updated));
}

if (Merged != null)
{
parameters.Add(String.Format(CultureInfo.InvariantCulture, "merged:{0}", Merged));
}
if (Comments != null)
{
parameters.Add(String.Format(CultureInfo.InvariantCulture, "comments:{0}", Comments));
Expand Down Expand Up @@ -312,7 +323,12 @@ public enum IssueSearchSort
/// search by last updated
/// </summary>
[Parameter(Value = "updated")]
Updated
Updated,
/// <summary>
/// search by last merged
/// </summary>
[Parameter(Value = "merged")]
Merged
}

public enum IssueInQualifier
Expand Down
2 changes: 1 addition & 1 deletion Octokit/Models/Response/PullRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public PullRequest(Uri url, Uri htmlUrl, Uri diffUrl, Uri patchUrl, Uri issueUrl
Base = @base;
User = user;
MergeCommitSha = mergeCommitSha;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets remove this property and constructor parameter since it's deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well. Another quuestion is merge-commit-sha. It is suggested that we use Mergeable instead of merge-commit-sha. Shall we also remove that?
https://developer.github.com/v3/pulls/#get-a-single-pull-request

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I'm referring to. We should remove MergeCommitSha

Merged = merged;
Merged = merged || MergedAt.HasValue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's completely remove the merged constructor parameter. Merged will be a calculated property. So this should be Merged = MergedAt.HasValue.

Mergeable = mergeable;
MergedBy = mergedBy;
Comments = comments;
Expand Down