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

MergeRequestDiscussions.create does not create note at line #3494

Closed
2 tasks done
nopol10 opened this issue Dec 21, 2023 · 3 comments · Fixed by #3497
Closed
2 tasks done

MergeRequestDiscussions.create does not create note at line #3494

nopol10 opened this issue Dec 21, 2023 · 3 comments · Fixed by #3497
Labels
released This issue/pull request has been released. type:bug Changes fix a minor bug

Comments

@nopol10
Copy link

nopol10 commented Dec 21, 2023

Description

  • Node.js version: 18.15.0
  • Gitbeaker version: 39.26.1
  • Gitbeaker release (cli, node, browser, core, requester-utils): node
  • OS & version: Windows 10

When I pass in a position object into MergeRequestDiscussions.create, I expected my MR comment to appear on a given line.
However it appears directly in the Overview tab.

Steps to reproduce

Invoke the api with appropriate arguments.

Expected behaviour

Comment created on a diff line.

Actual behaviour

Comment created as a standalone note.

Possible fixes

Looking at the request and response that was sent, it seems that the type of the created note is "DiscussionNote" when it should be "DiffNote".
I went and tried the same request and body in postman and was able to get it to work (it returned "DiffNote" and the comment appeared on the line). This was when the data was sent as form-data.
However when I changed the body to JSON (with Content-Type = application/json), it generated a "DiscussionNote" instead.

If I forcefully add "isForm" to the options passed to MergeRequestDiscussions.create, it works as expected.
Should isForm be defaulted to true for this? Otherwise position options don't work as expected.

Checklist

  • I have checked that this is not a duplicate issue.
  • I have read the documentation.
@jdalrymple
Copy link
Owner

100p on me. I switched this up in #3478. This latest PR should address this one piece though #3497

@jdalrymple jdalrymple added the type:bug Changes fix a minor bug label Dec 25, 2023
@jdalrymple
Copy link
Owner

Give the https://www.npmjs.com/package/@gitbeaker/rest/v/39.28.0--canary.3497.1119579763.0 version a look. It should solve the issue, but best to double check first!

@jdalrymple
Copy link
Owner

🚀 Issue was released in 39.28.0 🚀

@jdalrymple jdalrymple added the released This issue/pull request has been released. label Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released. type:bug Changes fix a minor bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants