Fix review conversation reply #23846
Conversation
|
1.19 unaffected, right? |
GiteaBot
added
the
lgtm/need 2
GiteaBot
added
lgtm/need 1
Update: Yes, unaffected, no backport. |
| @@ -27,6 +27,7 @@ | |||
| {{if $.reply}} | |||
| <button class="ui submit green tiny button btn-reply" type="submit">{{$.root.locale.Tr "repo.diff.comment.reply"}}</button> | |||
| <input type="hidden" name="reply" value="{{$.reply}}"> | |||
| <input type="hidden" name="single_review" value="true"> | |||
There was a problem hiding this comment.
I have a slight feeling that the current mechanism will always be somewhat erroneous:
- With your proposed approach, it won't be possible to use replies as part of a pending review.
The question is whether we want to allow that or not… - With the current approach, replies are listed as a separate review.
There was a problem hiding this comment.
That's the old behavior before the regression. At that time, a default form submission means comment but not pending.
Or maybe I made something wrong, but I haven't figured out. I felt that this fix works like before
Update: I think this fix is not wrong, see comment below.
There was a problem hiding this comment.
-
"With your proposed approach"
That's not my proposal, that's how it worked before. Actually I think after my refactoring, the logic is slightly clearer than before, it's easier to add a correct pending behavior in the future. -
"With the current approach, replies are listed as a separate review."
And confirmed with Gitea 1.16, it was already like that before
So this PR only fixes the "pending" problem, I think we should leave other improvements to the future.
Screenshot of 1.16
Screenshot of 1.20
GiteaBot
added
lgtm/done
Codecov Report
@@ Coverage Diff @@
## main #23846 +/- ##
==========================================
- Coverage 47.14% 47.03% -0.12%
==========================================
Files 1149 1158 +9
Lines 151446 153179 +1733
==========================================
+ Hits 71397 72045 +648
- Misses 71611 72631 +1020
- Partials 8438 8503 +65
... and 72 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
* upstream/main: [skip ci] Updated translations via Crowdin Update JS deps (go-gitea#23853) Added close/open button to details page of milestone (go-gitea#23877) Check `IsActionsToken` for LFS authentication (go-gitea#23841) Prefill input values in oauth settings as intended (go-gitea#23829) Display image size for multiarch container images (go-gitea#23821) Use clippie module to copy to clipboard (go-gitea#23801) Remove assertion debug code for show/hide refactoring (go-gitea#23576) [skip ci] Updated translations via Crowdin Remove jQuery ready usage (go-gitea#23858) Fix JS error when changing PR's target branch (go-gitea#23862) Improve action log display with control chars (go-gitea#23820) Fix review conversation reply (go-gitea#23846) Improve home page template, fix Sort dropdown menu flash (go-gitea#23856) Make first section on home page full width (go-gitea#23854) [skip ci] Updated translations via Crowdin Fix incorrect CORS failure detection logic (go-gitea#23844)
Regression of #23245
Close #23843