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

feat: add UpdateIssueWithOptions #9

Closed

Conversation

RichardoC
Copy link

and add testing for addOptions to make clear what it does

What type of PR is this?

  • feature

What this PR does / why we need it:

  • Added an equivalent to func (*IssueService) UpdateWithOptions for just issue patches
  • added some testing coverages to addOptions to help show how it works and
  • Fixes a bug where "NotifyUsers: false" would not get sent to the server due to ommit empty, and empty for booleans are false. This may be present in other fields but just focussed on fixing my own issue

Which issue(s) this PR fixes:

Special notes for your reviewer:

Additional documentation e.g., usage docs, etc.:

… make clear what it does

Signed-off-by: rtweed <RichardoC@users.noreply.github.com>
@@ -73,6 +73,19 @@ func testRequestParams(t *testing.T, r *http.Request, want map[string]string) {

}

func Test_addOptions(t *testing.T) {
v, err := addOptions("rest/api/2/issue/123", &UpdateQueryOptions{NotifyUsers: false})
Copy link
Author

Choose a reason for hiding this comment

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

This test fails without the omitEmpty changes

@RichardoC
Copy link
Author

There could be other "default true" fields that have this same issue with omitempty

chrisnovakovic added a commit to chrisnovakovic/go-jira that referenced this pull request Feb 22, 2024
On Jira API endpoints that accept a `notifyUsers` parameter, the value
of `notifyUsers` defaults to true if it isn't supplied in the request.
When the `UpdateQueryOptions` and `AddWorklogQueryOptions` structs are
serialised for inclusion in a request, the `omitempty` tag on the
`NotifyUsers` field causes it to be omitted from the serialised string
even when it is given an explicit false value, thereby preventing the
`notifyUsers` parameter from ever being included in the request.

Don't omit `NotifyUsers` from the serialised string when it has a false
value, allowing it to be sent in the request.

Spotted and fixed by @RichardoC in thought-machine#9 - splitting it out into a separate
PR for visibility.
@chrisnovakovic
Copy link
Collaborator

This is based on go-jira v2, which differs significantly to v1 - I've implemented these features in a v1-appropriate way in #12 and #13. Hopefully this will be merged upstream though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants