Skip to content

Commit

Permalink
Change Payload to a Dictionary<string, string> (#2303)
Browse files Browse the repository at this point in the history
When serializing the `NewDeployment` type, the `Payload` is serialized as an escaped string because JSON.NET doesn't know it's meant to be JSON.

This causes a problem when you call the API because the Payload is supposed to be a JSON dictionary that's just part of the overall payload. It's not supposed to be an escaped string.

That's why the JSON deserializer fails on it. Not only that, any deployments created using the current Octokit.net will create an invalid payload.

This PR fixes it by changing the type of `Payload` to a dictionary. THIS IS A BREAKING CHANGE, but the old behavior was broken so it forces a new correct behavior.

Fixes #2250
  • Loading branch information
haacked committed Feb 21, 2021
1 parent 8b0fb13 commit e40c792
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
12 changes: 12 additions & 0 deletions Octokit.Tests/Models/DeploymentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ namespace Octokit.Tests.Models
{
public class DeploymentTests
{
[Fact]
public void CanSerialize()
{
var deployment = new NewDeployment("ref")
{
Payload = new Dictionary<string, string> {{"environment", "production"}}
};
var deserialized = new SimpleJsonSerializer().Serialize(deployment);

Assert.Equal(@"{""ref"":""ref"",""task"":""deploy"",""payload"":{""environment"":""production""}}", deserialized);
}

[Fact]
public void CanDeserialize()
{
Expand Down
5 changes: 3 additions & 2 deletions Octokit/Models/Request/NewDeployment.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.ObjectModel;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
Expand Down Expand Up @@ -59,7 +60,7 @@ public NewDeployment(string @ref)
/// <summary>
/// JSON payload with extra information about the deployment.
/// </summary>
public string Payload { get; set; }
public Dictionary<string, string> Payload { get; set; }

/// <summary>
/// Optional name for the target deployment environment (e.g., production, staging, qa). Default: "production"
Expand Down

0 comments on commit e40c792

Please sign in to comment.