Bug introduced when "Removing accept header previews" Merged

[casstait]

Hi there!

I believe a bug was introduced in version 1.0.1 from the PR that removed accept header previews.

What used to occur was:

var data = await ApiConnection.Put<RepositoryTopics>(endpoint, topics, null, AcceptHeaders.RepositoryTopicsPreview).ConfigureAwait(false);

Where the AcceptHeaders was passed as a fourth argument to the PUT method in the APIConnection class.

What now occurs is:

var data = await ApiConnection.Put<RepositoryTopics>(endpoint, topics, null).ConfigureAwait(false);

Where the AcceptHeaders is removed and the twoFactorAuthenticationCode are set to null and the three argument PUT method now uses:

public async Task<T> Put<T>(Uri uri, object data, string twoFactorAuthenticationCode)
{
    Ensure.ArgumentNotNull(uri, nameof(uri));
    Ensure.ArgumentNotNull(data, nameof(data));
    Ensure.ArgumentNotNullOrEmptyString(twoFactorAuthenticationCode, nameof(twoFactorAuthenticationCode));

    var response = await Connection.Put<T>(uri, data, twoFactorAuthenticationCode).ConfigureAwait(false);

    return response.Body;
}

Requiring and ensuring the twoFactorAuthenticationCode to be not null.

This will throw an error now for any use of the Put<T>(Uri uri, object data, string twoFactorAuthenticationCode) method with the twoFactorAuthenticationCode set to null.

We have reverted to version 1.0.0 but would like this to be fixed up as we'd like to use the functionality introduced in #2465.

Stack trace:

System.ArgumentNullException: Value cannot be null. (Parameter 'twoFactorAuthenticationCode')
   at Octokit.Ensure.ArgumentNotNullOrEmptyString(String value, String name) in /home/runner/work/octokit.net/octokit.net/Octokit/Helpers/Ensure.cs:line 32
   at Octokit.ApiConnection.Put[T](Uri uri, Object data, String twoFactorAuthenticationCode) in /home/runner/work/octokit.net/octokit.net/Octokit/Http/ApiConnection.cs:line 372
   at Octokit.RepositoriesClient.ReplaceAllTopics(String owner, String name, RepositoryTopics topics) in /home/runner/work/octokit.net/octokit.net/Octokit/Clients/RepositoriesClient.cs:line 841
   at ReleaseBot.Core.Integration.GitHub.GitHubRepositoryManager.AddNonProductionRepositoryTopic(String repositoryOwner, String repositoryName) in /opt/buildagent/work/5c03f1d3da11ebbd/source/ReleaseBot.Core/Integration/GitHub/GitHubRepositoryManager.cs:line 94

Version Affected:
1.0.1

When was bug introduced:
https://github.com/octokit/octokit.net/pull/2515/files#diff-280ab49c2b24a3635079b0e7005677658e0043dbdfa48639e5fc637c71da2066R841

[JonruAlveus]

Hi! Sorry about that, I'll have a PR to fix it done soon. It looks like the areas affected are:

• RepositoriesClient.ReplaceAllTopics
• RepositoryBranchesClient.UpdateBranchProtection
• RepositoryBranchesClient.UpdateRequiredStatusChecksContexts
• RepositoryBranchesClient.UpdateProtectedBranchTeamRestrictions
• RepositoryBranchesClient.UpdateProtectedBranchUserRestrictions

[nickfloyd]

Thanks @JonruAlveus for getting this knocked out! I'll start working up a release for this now. ❤️

Thank you @casstait for the catch and sorry for the trouble! 🥇

[nickfloyd]

As a side note, this next release has #2521 in it where we made both SDKs .NET Standard 2.0

[nickfloyd]

Released:
https://github.com/octokit/octokit.net/releases/tag/v2.0.0
https://www.nuget.org/packages/Octokit/2.0.0

[matt-richardson]

Thanks for the awesomely fast turnaround time, @JonruAlveus and @nickfloyd!