Skip to content

Conversation

@sotho
Copy link
Contributor

@sotho sotho commented Mar 24, 2021

The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.

close #15127

The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 24, 2021
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 24, 2021
@6543
Copy link
Member

6543 commented Mar 24, 2021

thanks for the pull :) @sotho - finished it for you

@zeripath zeripath changed the title Fix wrong user returned in API (#15127) Fix wrong user returned in API Mar 24, 2021
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 24, 2021
@6543
Copy link
Member

6543 commented Mar 24, 2021

🚀

@6543 6543 merged commit f2844b7 into go-gitea:master Mar 24, 2021
@6543
Copy link
Member

6543 commented Mar 24, 2021

@sotho pleace backport ... if you dont know just ask :)

sotho added a commit to sotho/gitea that referenced this pull request Mar 25, 2021
The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
sotho added a commit to sotho/gitea that referenced this pull request Mar 25, 2021
The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
@sotho
Copy link
Contributor Author

sotho commented Mar 25, 2021

Thanks. I cherry-picked the commit and opened two pull requests: for 1.13 and 1.14.

@6543 6543 added the backport/done All backports for this PR have been created label Mar 25, 2021
@sotho
Copy link
Contributor Author

sotho commented Mar 25, 2021

Hmm. I tried it out, but I think the change you added is not correct.
The ReviewID should stay as review.ID and not become comment.Poster.ID.

The struct is misleading: the "Reviewer" should be "Commenter" or "Poster", something like:

Commenter:  ToUser(comment.Poster, doer != nil, auth),
ReviewID:   review.ID,

@zeripath
Copy link
Contributor

Damn I think @sotho is right here. it should be reviewID

@6543 6543 mentioned this pull request Mar 26, 2021
@6543
Copy link
Member

6543 commented Mar 26, 2021

-> #15164

and after that adjust backports + merge

6543 added a commit that referenced this pull request Mar 26, 2021
The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
lafriks pushed a commit that referenced this pull request Mar 26, 2021
* Fix wrong user returned in API (#15139)

The API call: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
returns always the reviewer, but should return the poster.

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>

* rm regression

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong user returned in API: GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments

4 participants