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

Make comment Author & UpdateAuthor properties pointers #640

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LaurenceTews
Copy link

What type of PR is this?

  • bug

What this PR does / why we need it:

Currently requests built using the AddComment() and UpdateComment() functions will fail (HTTP 400) due to a schema mismatch. Because Author and UpdateAuthor properties are not pointers, they get marshalled into the resulting JSON body, rather than being omitted via omitempty.

This change brings the Comment struct type in line with other structs that using pointers for Author and UpdateAuthor properties.

Which issue(s) this PR fixes:

#604

Fixes #

Special notes for your reviewer:

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

@lgecse
Copy link

lgecse commented Aug 1, 2023

@LaurenceTews thanks for the fix!

@andygrunwald Kind reminder, could you please review this?

@damien-simpson
Copy link

Keen to have this merged, using a workaround for now

@KvistA-ELS
Copy link

I can confirm this works - I've been fighting this problem today and was just about to put in an identical PR. Would be really nice to get merged :)

@justaugustus
Copy link

@andygrunwald – can you take a look at this PR?

We have a confirmations above that this PR fixes the bug, including folks using my downstream project, which is currently importing a fork of this repo which carries the patch:

anwalsh added a commit to anwalsh/go-jira that referenced this pull request Jul 23, 2024
mt40 added a commit to mt40/go-jira that referenced this pull request Aug 15, 2024
mt40 added a commit to mt40/go-jira that referenced this pull request Aug 15, 2024
@KvistA-ELS
Copy link

This would really be nice to have merged - it must be failing for a lot of people...

{"errorMessages":["Invalid request payload. Refer to the REST API documentation and try again."]}

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.

6 participants