Skip to content

Commit

Permalink
Fix property types in PullRequestCommit (#2224)
Browse files Browse the repository at this point in the history
PullRequestCommit Author and Committer fields were mistakenly typed with
Committer when they should be of type User.

Previously the types of properties were:

* `PullRequestCommit.Author/Committer`: `Committer`
* `Commit.Author/Committer`: `Committer`

Correct types should be:

* `PullRequestCommit.Author/Committer`: `User`
* `Commit.Author/Committer`: `Committer`

These fields always fail to deserialize in the API calls but produce no
errors, only objects with default values.
  • Loading branch information
mmv committed Feb 13, 2021
1 parent 46787d2 commit 36829cb
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 3 deletions.
102 changes: 102 additions & 0 deletions Octokit.Tests/Models/PullRequestCommitTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
using System.Collections.Generic;
using System.Linq;
using Octokit;
using Octokit.Internal;
using Xunit;

public class PullRequestCommitTests {
// the following JSON is taken from the documentation available at https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request

const string jsonPayload = @"
[
{
""url"": ""https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""sha"": ""6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""node_id"": ""MDY6Q29tbWl0NmRjYjA5YjViNTc4NzVmMzM0ZjYxYWViZWQ2OTVlMmU0MTkzZGI1ZQ=="",
""html_url"": ""https://github.com/octocat/Hello-World/commit/6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""comments_url"": ""https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/comments"",
""commit"": {
""url"": ""https://api.github.com/repos/octocat/Hello-World/git/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""author"": {
""name"": ""Monalisa Octocat"",
""email"": ""support@github.com"",
""date"": ""2011-04-14T16:00:49Z""
},
""committer"": {
""name"": ""Monalisa Octocat"",
""email"": ""support@github.com"",
""date"": ""2011-04-14T16:00:49Z""
},
""message"": ""Fix all the bugs"",
""tree"": {
""url"": ""https://api.github.com/repos/octocat/Hello-World/tree/6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""sha"": ""6dcb09b5b57875f334f61aebed695e2e4193db5e""
},
""comment_count"": 0,
""verification"": {
""verified"": false,
""reason"": ""unsigned"",
""signature"": null,
""payload"": null
}
},
""author"": {
""login"": ""octocat"",
""id"": 1,
""node_id"": ""MDQ6VXNlcjE="",
""avatar_url"": ""https://github.com/images/error/octocat_happy.gif"",
""gravatar_id"": """",
""url"": ""https://api.github.com/users/octocat"",
""html_url"": ""https://github.com/octocat"",
""followers_url"": ""https://api.github.com/users/octocat/followers"",
""following_url"": ""https://api.github.com/users/octocat/following{/other_user}"",
""gists_url"": ""https://api.github.com/users/octocat/gists{/gist_id}"",
""starred_url"": ""https://api.github.com/users/octocat/starred{/owner}{/repo}"",
""subscriptions_url"": ""https://api.github.com/users/octocat/subscriptions"",
""organizations_url"": ""https://api.github.com/users/octocat/orgs"",
""repos_url"": ""https://api.github.com/users/octocat/repos"",
""events_url"": ""https://api.github.com/users/octocat/events{/privacy}"",
""received_events_url"": ""https://api.github.com/users/octocat/received_events"",
""type"": ""User"",
""site_admin"": false
},
""committer"": {
""login"": ""octocat"",
""id"": 1,
""node_id"": ""MDQ6VXNlcjE="",
""avatar_url"": ""https://github.com/images/error/octocat_happy.gif"",
""gravatar_id"": """",
""url"": ""https://api.github.com/users/octocat"",
""html_url"": ""https://github.com/octocat"",
""followers_url"": ""https://api.github.com/users/octocat/followers"",
""following_url"": ""https://api.github.com/users/octocat/following{/other_user}"",
""gists_url"": ""https://api.github.com/users/octocat/gists{/gist_id}"",
""starred_url"": ""https://api.github.com/users/octocat/starred{/owner}{/repo}"",
""subscriptions_url"": ""https://api.github.com/users/octocat/subscriptions"",
""organizations_url"": ""https://api.github.com/users/octocat/orgs"",
""repos_url"": ""https://api.github.com/users/octocat/repos"",
""events_url"": ""https://api.github.com/users/octocat/events{/privacy}"",
""received_events_url"": ""https://api.github.com/users/octocat/received_events"",
""type"": ""User"",
""site_admin"": false
},
""parents"": [
{
""url"": ""https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e"",
""sha"": ""6dcb09b5b57875f334f61aebed695e2e4193db5e""
}
]
}
]
";

[Fact]
public void AuthorAndCommitterDeserializedCorrectly() {
var serializer = new SimpleJsonSerializer();
var commits = serializer.Deserialize<List<PullRequestCommit>>(jsonPayload);

Assert.Equal("octocat", commits.Single().Author.Login);
Assert.Equal("support@github.com", commits.Single().Commit.Author.Email);
}

}
6 changes: 3 additions & 3 deletions Octokit/Models/Response/PullRequestCommit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class PullRequestCommit
{
public PullRequestCommit() { }

public PullRequestCommit(string nodeId, Committer author, string commentsUrl, Commit commit, Committer committer, string htmlUrl, IEnumerable<GitReference> parents, string sha, string url)
public PullRequestCommit(string nodeId, User author, string commentsUrl, Commit commit, User committer, string htmlUrl, IEnumerable<GitReference> parents, string sha, string url)
{
Ensure.ArgumentNotNull(parents, nameof(parents));

Expand All @@ -31,13 +31,13 @@ public PullRequestCommit(string nodeId, Committer author, string commentsUrl, Co
/// </summary>
public string NodeId { get; protected set; }

public Committer Author { get; protected set; }
public User Author { get; protected set; }

public string CommentsUrl { get; protected set; }

public Commit Commit { get; protected set; }

public Committer Committer { get; protected set; }
public User Committer { get; protected set; }

public string HtmlUrl { get; protected set; }

Expand Down

0 comments on commit 36829cb

Please sign in to comment.