Skip to content

Do not track edits and deletions of pending review comments #18846

@delvh

Description

@delvh
Member

Gitea Version

1.16.1-dev

Git Version

NA

Operating System

Linux

How are you running Gitea?

NA

Database

No response

Can you reproduce the bug on the Gitea demo site?

Yes, unfortunately

Log Gist

No response

Description

Currently, any edit to a comment will be logged in its change history.
For PR Reviews, however, edits should only be logged after the review was submitted.
Edits prior to that should simply be discarded.

Scope

There are two aspects that makes this issue way worse:

  1. The webhooks are also fired, meaning that even though your review is not yet present, others will be informed about the content of what you edited
  2. This does not only apply to editing pending comments, but also to deleting pending comments.

Keep in mind

The creation change should be adapted to not show the original text,
but the text of the last edit before submitting the review.
However, this appears to already be done within models/issues/content_history.go#keepLimitedContentHistory

see https://try.gitea.io/delvh/kanban-test/pulls/16#issuecomment-113571 for example.

Screenshots

No response

Activity

changed the title [-]Do not track edits to messages made during reviews[/-] [+]Do not track edits to comments made during reviews[/+] on Feb 21, 2022
added
type/proposalThe new feature has not been accepted yet but needs to be discussed first.
on Feb 21, 2022
added this to the 1.17.0 milestone on Feb 21, 2022
modified the milestones: 1.17.0, 1.18.0 on Jun 12, 2022
changed the title [-]Do not track edits to comments made during reviews[/-] [+]Do not track edits and deletions of pending review comments[/+] on Jun 14, 2022
wxiaoguang

wxiaoguang commented on Jul 5, 2022

@wxiaoguang
Contributor

There is an old issue, it seems related.

lunny

lunny commented on Jul 5, 2022

@lunny
Member

duplicated

removed this from the 1.18.0 milestone on Jul 5, 2022
wxiaoguang

wxiaoguang commented on Jul 5, 2022

@wxiaoguang
Contributor

@delvh are you working on this? Should we keep this issue open, or use the old one?

delvh

delvh commented on Jul 5, 2022

@delvh
MemberAuthor

We can use the other one, and so far I'm not working on it because
a) I have close to no time right now (see my sparse interactions in the last few days)
b) I haven't found the root location of this "bug" yet, whenever I attempt to follow the trail, it leads me throughout the whole application...

wxiaoguang

wxiaoguang commented on Jul 5, 2022

@wxiaoguang
Contributor

Hmm ... these 2 issues are related, but not exactly the same (#4388 is about sending notification before summit, this is about saving content history before submit). I think we can keep both open, just link them for further investigation.

13 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    issue/criticalThis issue should be fixed ASAP. If it is a PR, the PR should be merged ASAPtype/bugtype/proposalThe new feature has not been accepted yet but needs to be discussed first.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Participants

      @lunny@xgdgsc@wxiaoguang@madxmike@delvh

      Issue actions

        Do not track edits and deletions of pending review comments · Issue #18846 · go-gitea/gitea