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 #658

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RichardoC
Copy link

@RichardoC RichardoC commented Feb 8, 2024

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 omit empty, and empty for booleans are false. This may be present in other fields but just focussed on fixing my own issue
  • Fixes all boolean options to avoid repeats of this bug

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>
@RichardoC RichardoC changed the title feature: add UpdateIssueWithOptions … feature: add UpdateIssueWithOptions Feb 8, 2024
@RichardoC RichardoC changed the title feature: add UpdateIssueWithOptions feat: add UpdateIssueWithOptions Feb 8, 2024
@RichardoC
Copy link
Author

I'd love a patch release to be made including this, as it means I can send thousands fewer emails a day from automation that noone wants to read

testMux.HandleFunc("/rest/api/2/issue/PROJ-9001", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPut)
testRequestURL(t, r, "/rest/api/2/issue/PROJ-9001")
testRequestParams(t, r, map[string]string{"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

Signed-off-by: rtweed <RichardoC@users.noreply.github.com>
@RichardoC
Copy link
Author

Credit to @chrisnovakovic for finding this fix in thought-machine@3870a80

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.

None yet

1 participant