Prevent simultaneous editing of comments and issues

models/issues/issue.go

@@ -94,6 +94,8 @@ func (err ErrIssueWasClosed) Error() string {
 	return fmt.Sprintf("Issue [%d] %d was already closed", err.ID, err.Index)
 }
 
+var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already changed")
+
 // Issue represents an issue or pull request of repository.
 type Issue struct {
 	ID               int64                  `xorm:"pk autoincr"`
@@ -121,6 +123,7 @@ type Issue struct {
 	NumComments      int
 	Ref              string
 	PinOrder         int `xorm:"DEFAULT 0"`
+	Version          int `xorm:"version"`
 
 	DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
 

models/issues/issue_update.go

@@ -235,7 +235,7 @@ func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string)
 }
 
 // ChangeIssueContent changes issue content, as the given user.
-func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string) (err error) {
+func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string, version int) (err error) {
 	ctx, committer, err := db.TxContext(ctx)
 	if err != nil {
 		return err
@@ -254,9 +254,14 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User
 	}
 
 	issue.Content = content
+	issue.Version = version
 
-	if err = UpdateIssueCols(ctx, issue, "content"); err != nil {
-		return fmt.Errorf("UpdateIssueCols: %w", err)
+	affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content").Update(issue)
+	if err != nil {
+		return err
+	}
+	if affected == 0 {
+		return ErrIssueAlreadyChanged
 	}
 
 	if err = SaveIssueContentHistory(ctx, doer.ID, issue.ID, 0,

models/migrations/migrations.go

@@ -21,6 +21,7 @@ import (
 	"code.gitea.io/gitea/models/migrations/v1_20"
 	"code.gitea.io/gitea/models/migrations/v1_21"
 	"code.gitea.io/gitea/models/migrations/v1_22"
+	"code.gitea.io/gitea/models/migrations/v1_23"
 	"code.gitea.io/gitea/models/migrations/v1_6"
 	"code.gitea.io/gitea/models/migrations/v1_7"
 	"code.gitea.io/gitea/models/migrations/v1_8"
@@ -587,6 +588,9 @@ var migrations = []Migration{
 	NewMigration("Drop wrongly created table o_auth2_application", v1_22.DropWronglyCreatedTable),
 
 	// Gitea 1.22.0-rc1 ends at 299
+
+	// v299 -> v300
+	NewMigration("Add version to issue table", v1_23.AddVersionToIssue),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v1_23/v299.go

@@ -0,0 +1,25 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_23 //nolint
+
+import "xorm.io/xorm"
+
+func AddVersionToIssue(x *xorm.Engine) error {
+	type Issue struct {
+		Version int `xorm:"version"`
+	}
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	if err := sess.Sync(new(Issue)); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}

options/locale/locale_en-US.ini

@@ -1443,6 +1443,7 @@ issues.new.clear_assignees = Clear assignees
 issues.new.no_assignees = No Assignees
 issues.new.no_reviewers = No reviewers
 issues.new.blocked_user = Cannot create issue because you are blocked by the repository owner.
+issues.edit.already_changed = Unable to change issue content because issue is already changed
 issues.edit.blocked_user = Cannot edit content because you are blocked by the poster or repository owner.
 issues.choose.get_started = Get Started
 issues.choose.open_external_link = Open

routers/api/v1/repo/issue.go

@@ -810,8 +810,13 @@ func EditIssue(ctx *context.APIContext) {
 		}
 	}
 	if form.Body != nil {
-		err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body)
+		err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.Version)
 		if err != nil {
+			if errors.Is(err, issues_model.ErrIssueAlreadyChanged) {
+				ctx.Error(http.StatusBadRequest, "ChangeContent", err)
+				return
+			}
+
 			ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
 			return
 		}

routers/api/v1/repo/issue_attachment.go

@@ -198,7 +198,7 @@ func CreateIssueAttachment(ctx *context.APIContext) {
 
 	issue.Attachments = append(issue.Attachments, attachment)
 
-	if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, issue.Content); err != nil {
+	if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, issue.Content, issue.Version); err != nil {
 		ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
 		return
 	}

routers/api/v1/repo/pull.go

@@ -610,8 +610,13 @@ func EditPullRequest(ctx *context.APIContext) {
 		}
 	}
 	if form.Body != nil {
-		err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body)
+		err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.Version)
 		if err != nil {
+			if errors.Is(err, issues_model.ErrIssueAlreadyChanged) {
+				ctx.Error(http.StatusBadRequest, "ChangeContent", err)
+				return
+			}
+
 			ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
 			return
 		}

routers/web/repo/issue.go

@@ -2247,9 +2247,11 @@ func UpdateIssueContent(ctx *context.Context) {
 		return
 	}
 
-	if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, ctx.Req.FormValue("content")); err != nil {
+	if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, ctx.Req.FormValue("content"), ctx.FormInt("version")); err != nil {
 		if errors.Is(err, user_model.ErrBlockedUser) {
 			ctx.JSONError(ctx.Tr("repo.issues.edit.blocked_user"))
+		} else if errors.Is(err, issues_model.ErrIssueAlreadyChanged) {
+			ctx.JSONError(ctx.Tr("repo.issues.edit.already_changed"))
 		} else {
 			ctx.ServerError("ChangeContent", err)
 		}

services/issue/content.go

@@ -13,7 +13,7 @@ import (
 )
 
 // ChangeContent changes issue content, as the given user.
-func ChangeContent(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, content string) error {
+func ChangeContent(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, content string, version int) error {
 	if err := issue.LoadRepo(ctx); err != nil {
 		return err
 	}
@@ -26,7 +26,7 @@ func ChangeContent(ctx context.Context, issue *issues_model.Issue, doer *user_mo
 
 	oldContent := issue.Content
 
-	if err := issues_model.ChangeIssueContent(ctx, issue, doer, content); err != nil {
+	if err := issues_model.ChangeIssueContent(ctx, issue, doer, content, version); err != nil {
 		return err
 	}
 

templates/repo/issue/view_content.tmpl

@@ -60,7 +60,7 @@
 							{{end}}
 						</div>
 						<div id="issue-{{.Issue.ID}}-raw" class="raw-content tw-hidden">{{.Issue.Content}}</div>
-						<div class="edit-content-zone tw-hidden" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/content" data-context="{{.RepoLink}}" data-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/attachments" data-view-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/view-attachments"></div>
+						<div class="edit-content-zone tw-hidden" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/content" data-version="{{.Issue.Version}}" data-context="{{.RepoLink}}" data-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/attachments" data-view-attachment-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/view-attachments"></div>
 						{{if .Issue.Attachments}}
 							{{template "repo/issue/view_content/attachments" dict "Attachments" .Issue.Attachments "RenderedContent" .Issue.RenderedContent}}
 						{{end}}

web_src/js/features/repo-issue-edit.js

@@ -3,6 +3,7 @@ import {handleReply} from './repo-issue.js';
 import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js';
 import {createDropzone} from './dropzone.js';
 import {GET, POST} from '../modules/fetch.js';
+import {showErrorToast} from '../modules/toast.js';
 import {hideElem, showElem} from '../utils/dom.js';
 import {attachRefIssueContextPopup} from './contextpopup.js';
 import {initCommentContent, initMarkupContent} from '../markup/content.js';
@@ -121,14 +122,22 @@ async function onEditContent(event) {
     hideElem(editContentZone);
     const dropzoneInst = comboMarkdownEditor.attachedDropzoneInst;
     try {
+      const version = editContentZone.getAttribute('data-version');
+
       const params = new URLSearchParams({
         content: comboMarkdownEditor.value(),
         context: editContentZone.getAttribute('data-context'),
+        version,
       });
       for (const fileInput of dropzoneInst?.element.querySelectorAll('.files [name=files]')) params.append('files[]', fileInput.value);
 
       const response = await POST(editContentZone.getAttribute('data-update-url'), {data: params});
       const data = await response.json();
+      if (response.status === 400) {
+        showErrorToast(data.errorMessage);
+        return;
+      }
+      editContentZone.setAttribute('data-version', parseInt(version) + 1);
       if (!data.content) {
         renderContent.innerHTML = document.getElementById('no-content').innerHTML;
         rawContent.textContent = '';

web_src/js/markup/tasklist.js

@@ -1,4 +1,5 @@
 import {POST} from '../modules/fetch.js';
+import {showErrorToast} from '../modules/toast.js';
 
 const preventListener = (e) => e.preventDefault();
 
@@ -54,13 +55,21 @@ export function initMarkupTasklist() {
           const editContentZone = container.querySelector('.edit-content-zone');
           const updateUrl = editContentZone.getAttribute('data-update-url');
           const context = editContentZone.getAttribute('data-context');
+          const version = editContentZone.getAttribute('data-version');
 
           const requestBody = new FormData();
           requestBody.append('ignore_attachments', 'true');
           requestBody.append('content', newContent);
           requestBody.append('context', context);
-          await POST(updateUrl, {data: requestBody});
-
+          requestBody.append('version', version);
+          const response = await POST(updateUrl, {data: requestBody});
+          const data = await response.json();
+          if (response.status === 400) {
+            showErrorToast(data.errorMessage);
+            rawContent.textContent = oldContent;
+            return;
+          }
+          editContentZone.setAttribute('data-version', parseInt(version) + 1);
           rawContent.textContent = newContent;
         } catch (err) {
           checkbox.checked = !checkbox.checked;