octokit.net icon indicating copy to clipboard operation
octokit.net copied to clipboard

Issue.Update remove milestone from issue no matter what

Open Eilon opened this issue 6 years ago • 5 comments
trafficstars

I have code like this:

            var issueUpdate = new IssueUpdate();
            issueUpdate.AddLabel("some label");
            await gitHub.Issue.Update("owner", "repo", issueNumber, issueUpdate);

But over the wire, this request body is sent:

{"milestone":null,"labels":["some label"]}

As you can see, the milestone is being forced to null, even though I didn't explicitly do that.

This makes the Update operation cause data loss.

To work around this I have to get the issue's current milestone and set it in the Update to avoid it being cleared out.

Eilon avatar Jan 10 '19 18:01 Eilon

OK looks like I can call:

Issue.Labels.AddToIssue(......);

To more easily do what I want. But the issue above still seems valid.

Eilon avatar Jan 10 '19 18:01 Eilon

Note: Looks like Issue.Labels.AddToIssue() doesn't work due to an Octokit bug (https://github.com/octokit/octokit.net/issues/1928), and also due to a GitHub issue anyway.

Eilon avatar Jan 10 '19 19:01 Eilon

Here's the code I have to do to work around this issue:

            var issue = await gitHub.Issue.Get("owner", "repo", issueNumber);

            var issueUpdate = new IssueUpdate
            {
                Milestone = issue.Milestone?.Number // Have to re-set milestone because otherwise it gets cleared out. See https://github.com/octokit/octokit.net/issues/1927
            };
            issueUpdate.AddLabel("some label");
            // Add all existing labels to the update so that they don't get removed
            foreach (var label in issue.Labels)
            {
                issueUpdate.AddLabel(label.Name);
            }

            await gitHub.Issue.Update("owner", "repo", issueNumber, issueUpdate);

Eilon avatar Jan 10 '19 21:01 Eilon

@Eilon thanks for the report and the snippet - I've been able to get this into a integration test and see the behaviour for myself. This is down to the serializing of IssueUpdate sending a null Milestone rather than omitting the field from the JSON payload if it's not cleared by the user.

A cleaner workaround if you have the issue from the API would be to use issue.ToUpdate() which gives you the pre-populated milestone and issues ready for you to then augment, so you don't need to manually enumerate things yourself (Assignees are also caught up in this null behaviour, in case you're using that in these issues).

var issue = await gitHub.Issue.Get("owner", "repo", issueNumber);

var issueUpdate = issue.ToUpdate();
issueUpdate.AddLabel("some label");

await gitHub.Issue.Update("owner", "repo", issueNumber, issueUpdate);

shiftkey avatar Jan 11 '19 12:01 shiftkey

BTW I don't see the problem with Assignee: I tested it out by only setting milestone and labels, and the existing assignee(s) on the issue remained.

Eilon avatar Jan 17 '19 19:01 Eilon

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

github-actions[bot] avatar Jul 27 '23 01:07 github-actions[bot]

I just hit this too. I was updating issue descriptions and ended mass-removing issues from milestones. Are there plans for this to be fixed?

I see the workaround above, and yes it's cleaner. But for users (like me) who didn't know about the cleaner option before trying the new IssueUpdate approach, this is a bit of a footgun.

rabuckley avatar May 04 '24 21:05 rabuckley