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.

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

Added enum ItemStateFilter to differentiate between search and list APIs #1140

Merged
merged 17 commits into from
Mar 29, 2016
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
4 changes: 2 additions & 2 deletions Octokit.Tests.Integration/Clients/IssuesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ await _issuesClient.Update(_context.RepositoryOwner, _context.RepositoryName, cl
new IssueUpdate { State = ItemState.Closed });

var issues = await _issuesClient.GetAllForRepository(_context.RepositoryOwner, _context.RepositoryName,
new RepositoryIssueRequest { State = ItemState.Closed });
new RepositoryIssueRequest { State = ItemStateFilter.Closed });

Assert.Equal(1, issues.Count);
Assert.Equal("A closed issue", issues[0].Title);
Expand Down Expand Up @@ -159,7 +159,7 @@ await _issuesClient.Update(_context.RepositoryOwner, _context.RepositoryName, is
new IssueUpdate { State = ItemState.Closed });

var retrieved = await _issuesClient.GetAllForRepository(_context.RepositoryOwner, _context.RepositoryName,
new RepositoryIssueRequest { State = ItemState.All });
new RepositoryIssueRequest { });

Assert.True(retrieved.Count >= 4);
Assert.True(retrieved.Any(i => i.Number == issue1.Number));
Expand Down
4 changes: 2 additions & 2 deletions Octokit.Tests.Integration/Clients/MilestonesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public async Task CanListClosedMilestones()
await _milestonesClient.Create(_context.RepositoryOwner, _context.RepositoryName, milestone3);

var milestones = await _milestonesClient.GetAllForRepository(_context.RepositoryOwner, _context.RepositoryName,
new MilestoneRequest { State = ItemState.Closed });
new MilestoneRequest { State = ItemStateFilter.Closed });

Assert.Equal(1, milestones.Count);
Assert.Equal("milestone 3", milestones[0].Title);
Expand All @@ -103,7 +103,7 @@ await _github.Issue.Update(_context.RepositoryOwner, _context.RepositoryName, is
new IssueUpdate { State = ItemState.Closed });

var retrieved = await _github.Issue.GetAllForRepository(_context.RepositoryOwner, _context.RepositoryName,
new RepositoryIssueRequest { State = ItemState.Closed });
new RepositoryIssueRequest { State = ItemStateFilter.Closed });

Assert.True(retrieved.Count >= 2);
Assert.True(retrieved.Any(i => i.Number == issue1.Number));
Expand Down
6 changes: 3 additions & 3 deletions Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public async Task CanGetOpenPullRequest()
var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
var result = await _fixture.Create(Helper.UserName, _context.RepositoryName, newPullRequest);

var openPullRequests = new PullRequestRequest { State = ItemState.Open };
var openPullRequests = new PullRequestRequest { State = ItemStateFilter.Open };
var pullRequests = await _fixture.GetAllForRepository(Helper.UserName, _context.RepositoryName, openPullRequests);

Assert.Equal(1, pullRequests.Count);
Expand All @@ -75,7 +75,7 @@ public async Task IgnoresOpenPullRequest()
var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
await _fixture.Create(Helper.UserName, _context.RepositoryName, newPullRequest);

var openPullRequests = new PullRequestRequest { State = ItemState.Closed };
var openPullRequests = new PullRequestRequest { State = ItemStateFilter.Closed };
var pullRequests = await _fixture.GetAllForRepository(Helper.UserName, _context.RepositoryName, openPullRequests);

Assert.Empty(pullRequests);
Expand Down Expand Up @@ -123,7 +123,7 @@ public async Task CanFindClosedPullRequest()
var updatePullRequest = new PullRequestUpdate { State = ItemState.Closed };
await _fixture.Update(Helper.UserName, _context.RepositoryName, pullRequest.Number, updatePullRequest);

var closedPullRequests = new PullRequestRequest { State = ItemState.Closed };
var closedPullRequests = new PullRequestRequest { State = ItemStateFilter.Closed };
var pullRequests = await _fixture.GetAllForRepository(Helper.UserName, _context.RepositoryName, closedPullRequests);

Assert.Equal(1, pullRequests.Count);
Expand Down
1 change: 0 additions & 1 deletion Octokit.Tests.Integration/Clients/SearchClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public async Task SearchForAllIssues()
{
var request = new SearchIssuesRequest("phone");
request.Repos.Add("caliburn-micro", "caliburn.micro");
request.State = ItemState.All;

var issues = await _gitHubClient.Search.SearchIssues(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public async Task ReturnsAllMilestones()
var github = Helper.GetAuthenticatedClient();

var client = new ObservableMilestonesClient(github);
var milestones = await client.GetAllForRepository("libgit2", "libgit2sharp", new MilestoneRequest { State = ItemState.Closed }).ToList();
var milestones = await client.GetAllForRepository("libgit2", "libgit2sharp", new MilestoneRequest { State = ItemStateFilter.Closed }).ToList();

Assert.NotEmpty(milestones);
Assert.True(milestones.All(m => m.State == ItemState.Closed));
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests/Models/IssueRequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public void ContainsSetValues()
var request = new IssueRequest
{
Filter = IssueFilter.All,
State = ItemState.Closed,
State = ItemStateFilter.Closed,
SortProperty = IssueSort.Comments,
SortDirection = SortDirection.Ascending,
Since = DateTimeOffset.ParseExact("Wed 23 Jan 2013 8:30 AM -08:00",
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests/Models/MilestoneRequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public void ContainsSetValues()
{
var request = new MilestoneRequest
{
State = ItemState.Closed,
State = ItemStateFilter.Closed,
SortProperty = MilestoneSort.Completeness,
SortDirection = SortDirection.Descending,
};
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests/Models/PullRequestRequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public void ContainsSetValues()
{
var request = new PullRequestRequest
{
State = ItemState.Closed,
State = ItemStateFilter.Closed,
Head = "user:ref-name",
Base = "fake_base_branch"
};
Expand Down
36 changes: 29 additions & 7 deletions Octokit/Models/Request/IssueRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class IssueRequest : RequestParameters
public IssueRequest()
{
Filter = IssueFilter.Assigned;
State = ItemState.Open;
State = ItemStateFilter.Open;
Labels = new Collection<string>();
SortProperty = IssueSort.Created;
SortDirection = SortDirection.Descending;
Expand All @@ -33,12 +33,12 @@ public IssueRequest()
public IssueFilter Filter { get; set; }

/// <summary>
/// Gets or sets the <see cref="ItemState"/> for the issues to return.
/// Gets or sets the <see cref="ItemStateFilter"/> for the issues to return.
/// </summary>
/// <value>
/// The state.
/// </value>
public ItemState State { get; set; }
public ItemStateFilter State { get; set; }

/// <summary>
/// Gets the labels to filter by. Add labels to the collection to only request issues with those labels.
Expand Down Expand Up @@ -121,23 +121,45 @@ public enum IssueFilter
}

/// <summary>
/// The range of states that an issue can be in.
/// Range of states for Issues, Milestones and PullRequest API.
/// </summary>
public enum ItemStateFilter
{
/// <summary>
/// Items that are open.
/// </summary>
Open,

/// <summary>
/// Items that are closed.
/// </summary>
Closed,

/// <summary>
/// All the items.
/// </summary>
All
}

/// <summary>
/// Items that are open OR closed
/// </summary>
public enum ItemState
{
/// <summary>
/// Isuses that are open (default).
/// Issues that are open
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 word this "Items that are open"

/// </summary>
Open,

/// <summary>
/// Isuses that are closed.
/// Issues that are closed
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 word this "Items that are closed"

/// </summary>
Closed,

/// <summary>
/// All the issues.
/// All the issues. The option is Obsolete
/// </summary>
[Obsolete("The value is Obsolete and will be removed in a future release as it is not a valid option for Search, Create and Update operations")]
Copy link
Contributor

Choose a reason for hiding this comment

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

As agreed by @shiftkey we can just DROP the All option here, no need to mark it [Obsolete] anymore

All
}

Expand Down
2 changes: 1 addition & 1 deletion Octokit/Models/Request/IssueUpdate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,4 @@ public void ClearLabels()
}
}
}
}
}
7 changes: 5 additions & 2 deletions Octokit/Models/Request/MilestoneRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ public class MilestoneRequest : RequestParameters
{
public MilestoneRequest()
{
State = ItemState.Open;
State = ItemStateFilter.Open;
SortProperty = MilestoneSort.DueDate;
SortDirection = SortDirection.Ascending;
}

public ItemState State { get; set; }
/// <summary>
/// Which Milestones to get. The default is <see cref="ItemStateFilter.Open"/>.
/// </summary>
public ItemStateFilter State { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where I would mention the default


[Parameter(Key = "sort")]
public MilestoneSort SortProperty { get; set; }
Expand Down
6 changes: 3 additions & 3 deletions Octokit/Models/Request/PullRequestRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ public class PullRequestRequest : RequestParameters
{
public PullRequestRequest()
{
State = ItemState.Open;
State = ItemStateFilter.Open;
SortProperty = PullRequestSort.Created;
SortDirection = SortDirection.Descending;
}

/// <summary>
/// "open" or "closed" to filter by state. Default is "open".
/// "open", "closed" or "all" to filter by state. Default is "open".
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps word this consistently with the other Request objects, eg:

Which PullRequests to get. The default is < see cref="ItemStateFilter.Open"/>.

/// </summary>
public ItemState State { get; set; }
public ItemStateFilter State { get; set; }

/// <summary>
/// Filter pulls by head user and branch name in the format of "user:ref-name".
Expand Down
4 changes: 2 additions & 2 deletions Octokit/Models/Response/Issue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public Issue(Uri url, Uri htmlUrl, Uri commentsUrl, Uri eventsUrl, int number, I
public int Number { get; protected set; }

/// <summary>
/// Whether the issue is open or closed.
/// Whether the issue is open, closed or all.
Copy link
Contributor

Choose a reason for hiding this comment

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

all is not a valid option here so this comment can be reverted back to what it was

/// </summary>
public ItemState State { get; protected set; }

Expand Down Expand Up @@ -155,4 +155,4 @@ public IssueUpdate ToUpdate()
return issueUpdate;
}
}
}
}