From c7433b3dae8409789a91c465a8ee595ff0ed5af4 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Wed, 22 May 2024 23:31:15 +0000 Subject: [PATCH 01/11] add issue version to prevent change issue simultaneously --- models/issues/issue.go | 3 +++ models/issues/issue_update.go | 12 +++++++++--- models/migrations/migrations.go | 4 ++++ models/migrations/v1_23/v299.go | 25 +++++++++++++++++++++++++ modules/structs/issue.go | 7 ++++--- modules/structs/pull.go | 1 + options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/issue.go | 11 ++++++++--- routers/api/v1/repo/issue_attachment.go | 2 +- routers/api/v1/repo/pull.go | 11 ++++++++--- routers/web/repo/issue.go | 4 +++- services/issue/content.go | 4 ++-- templates/repo/issue/view_content.tmpl | 2 +- templates/swagger/v1_json.tmpl | 10 ++++++++++ web_src/js/features/repo-issue-edit.js | 9 +++++++++ 15 files changed, 89 insertions(+), 17 deletions(-) create mode 100644 models/migrations/v1_23/v299.go diff --git a/models/issues/issue.go b/models/issues/issue.go index 87c1c86eb15be..41d56def39f7f 100644 --- a/models/issues/issue.go +++ b/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:"NOT NULL DEFAULT 0"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 147b7eb3b91b0..fc070dc9bdb10 100644 --- a/models/issues/issue_update.go +++ b/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,15 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content + issue.Version++ - if err = UpdateIssueCols(ctx, issue, "content"); err != nil { - return fmt.Errorf("UpdateIssueCols: %w", err) + affected, err := db.GetEngine(ctx).ID(issue.ID).Where("Version = ?", version).Cols("Content", "Version").Update(issue) + if err != nil { + return err + } + + if affected == 0 { + return ErrIssueAlreadyChanged } if err = SaveIssueContentHistory(ctx, doer.ID, issue.ID, 0, diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4501585250597..35189a01b6f86 100644 --- a/models/migrations/migrations.go +++ b/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 diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go new file mode 100644 index 0000000000000..e88ad029bfe46 --- /dev/null +++ b/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:"NOT NULL DEFAULT 0"` + } + + 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() +} diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 16242d18ada58..526c882bafc4e 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -101,9 +101,10 @@ type CreateIssueOption struct { // EditIssueOption options for editing an issue type EditIssueOption struct { - Title string `json:"title"` - Body *string `json:"body"` - Ref *string `json:"ref"` + Title string `json:"title"` + Version int `json:"version"` + Body *string `json:"body"` + Ref *string `json:"ref"` // deprecated Assignee *string `json:"assignee"` Assignees []string `json:"assignees"` diff --git a/modules/structs/pull.go b/modules/structs/pull.go index b04def52b8661..0107261303993 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -85,6 +85,7 @@ type CreatePullRequestOption struct { // EditPullRequestOption options when modify pull request type EditPullRequestOption struct { Title string `json:"title"` + Version int `json:"version"` Body *string `json:"body"` Base string `json:"base"` Assignee string `json:"assignee"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index db4e3ec56bf84..151a041352999 100644 --- a/options/locale/locale_en-US.ini +++ b/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 diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index b91fbc33bfc5f..efe7cae05ebe4 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -810,10 +810,15 @@ 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, form.Version) if err != nil { - ctx.Error(http.StatusInternalServerError, "ChangeContent", err) - return + if errors.Is(err, issues_model.ErrIssueAlreadyChanged) { + ctx.Error(http.StatusBadRequest, "ChangeContent", err) + return + } else { + ctx.Error(http.StatusInternalServerError, "ChangeContent", err) + return + } } } if form.Ref != nil { diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index f5a28e6fa6bb1..9404b96fa4349 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/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 } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 38a32a73c78e7..1ab4e45843ea2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -610,10 +610,15 @@ 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, form.Version) if err != nil { - ctx.Error(http.StatusInternalServerError, "ChangeContent", err) - return + if errors.Is(err, issues_model.ErrIssueAlreadyChanged) { + ctx.Error(http.StatusBadRequest, "ChangeContent", err) + return + } else { + ctx.Error(http.StatusInternalServerError, "ChangeContent", err) + return + } } } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 0c8363a1681e5..5e7fbf713c097 100644 --- a/routers/web/repo/issue.go +++ b/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) } diff --git a/services/issue/content.go b/services/issue/content.go index 2f9bee806a9b1..0b8693a41898e 100644 --- a/services/issue/content.go +++ b/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 } diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index d40134ed08598..dc57793dc77b5 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -60,7 +60,7 @@ {{end}}
{{.Issue.Content}}
-
+
{{if .Issue.Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Issue.Attachments "RenderedContent" .Issue.RenderedContent}} {{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0b3f5cdcadfaa..3a770841b3191 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -20477,6 +20477,11 @@ "unset_due_date": { "type": "boolean", "x-go-name": "RemoveDeadline" + }, + "version": { + "type": "integer", + "format": "int64", + "x-go-name": "Version" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" @@ -20632,6 +20637,11 @@ "unset_due_date": { "type": "boolean", "x-go-name": "RemoveDeadline" + }, + "version": { + "type": "integer", + "format": "int64", + "x-go-name": "Version" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" diff --git a/web_src/js/features/repo-issue-edit.js b/web_src/js/features/repo-issue-edit.js index abf2d31221024..b1798ee45d862 100644 --- a/web_src/js/features/repo-issue-edit.js +++ b/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 = ''; From b4d8ad7d7ffdbacef6d8404ca9fda603fb161f9f Mon Sep 17 00:00:00 2001 From: metiftikci Date: Thu, 23 May 2024 12:57:36 +0000 Subject: [PATCH 02/11] revert api breaking change. use xorm lock. fix tasklist edit --- models/issues/issue.go | 2 +- models/issues/issue_update.go | 5 ++--- models/migrations/v1_23/v299.go | 2 +- modules/structs/issue.go | 7 +++---- modules/structs/pull.go | 1 - routers/api/v1/repo/issue.go | 8 ++++---- routers/api/v1/repo/pull.go | 8 ++++---- templates/swagger/v1_json.tmpl | 10 ---------- web_src/js/markup/tasklist.js | 13 +++++++++++-- 9 files changed, 26 insertions(+), 30 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 41d56def39f7f..05aed05239be3 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -123,7 +123,7 @@ type Issue struct { NumComments int Ref string PinOrder int `xorm:"DEFAULT 0"` - Version int `xorm:"NOT NULL DEFAULT 0"` + Version int `xorm:"version"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index fc070dc9bdb10..bb60c956fc47e 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -254,13 +254,12 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content - issue.Version++ + issue.Version = version - affected, err := db.GetEngine(ctx).ID(issue.ID).Where("Version = ?", version).Cols("Content", "Version").Update(issue) + affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content").Update(issue) if err != nil { return err } - if affected == 0 { return ErrIssueAlreadyChanged } diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index e88ad029bfe46..d7617eb892d5d 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -7,7 +7,7 @@ import "xorm.io/xorm" func AddVersionToIssue(x *xorm.Engine) error { type Issue struct { - Version int `xorm:"NOT NULL DEFAULT 0"` + Version int `xorm:"version"` } sess := x.NewSession() diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 526c882bafc4e..16242d18ada58 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -101,10 +101,9 @@ type CreateIssueOption struct { // EditIssueOption options for editing an issue type EditIssueOption struct { - Title string `json:"title"` - Version int `json:"version"` - Body *string `json:"body"` - Ref *string `json:"ref"` + Title string `json:"title"` + Body *string `json:"body"` + Ref *string `json:"ref"` // deprecated Assignee *string `json:"assignee"` Assignees []string `json:"assignees"` diff --git a/modules/structs/pull.go b/modules/structs/pull.go index 0107261303993..b04def52b8661 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -85,7 +85,6 @@ type CreatePullRequestOption struct { // EditPullRequestOption options when modify pull request type EditPullRequestOption struct { Title string `json:"title"` - Version int `json:"version"` Body *string `json:"body"` Base string `json:"base"` Assignee string `json:"assignee"` diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index efe7cae05ebe4..7c2e353274ab7 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -810,15 +810,15 @@ func EditIssue(ctx *context.APIContext) { } } if form.Body != nil { - err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, form.Version) + 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 - } else { - ctx.Error(http.StatusInternalServerError, "ChangeContent", err) - return } + + ctx.Error(http.StatusInternalServerError, "ChangeContent", err) + return } } if form.Ref != nil { diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 1ab4e45843ea2..fe2bf0bf4cdd7 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -610,15 +610,15 @@ func EditPullRequest(ctx *context.APIContext) { } } if form.Body != nil { - err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, form.Version) + 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 - } else { - ctx.Error(http.StatusInternalServerError, "ChangeContent", err) - return } + + ctx.Error(http.StatusInternalServerError, "ChangeContent", err) + return } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3a770841b3191..0b3f5cdcadfaa 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -20477,11 +20477,6 @@ "unset_due_date": { "type": "boolean", "x-go-name": "RemoveDeadline" - }, - "version": { - "type": "integer", - "format": "int64", - "x-go-name": "Version" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" @@ -20637,11 +20632,6 @@ "unset_due_date": { "type": "boolean", "x-go-name": "RemoveDeadline" - }, - "version": { - "type": "integer", - "format": "int64", - "x-go-name": "Version" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" diff --git a/web_src/js/markup/tasklist.js b/web_src/js/markup/tasklist.js index 00076bce58915..7f70ff5b7e581 100644 --- a/web_src/js/markup/tasklist.js +++ b/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; From 0277bc6513ca0a5f581de30d31c2c6391dbd568f Mon Sep 17 00:00:00 2001 From: metiftikci Date: Thu, 23 May 2024 16:31:47 +0000 Subject: [PATCH 03/11] revet xorm lock. fix pr message. get new version from backend --- models/issues/issue.go | 2 +- models/issues/issue_update.go | 4 ++-- options/locale/locale_en-US.ini | 3 ++- routers/web/repo/issue.go | 7 ++++++- web_src/js/features/repo-issue-edit.js | 6 ++---- web_src/js/markup/tasklist.js | 3 +-- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 05aed05239be3..41d56def39f7f 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -123,7 +123,7 @@ type Issue struct { NumComments int Ref string PinOrder int `xorm:"DEFAULT 0"` - Version int `xorm:"version"` + Version int `xorm:"NOT NULL DEFAULT 0"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index bb60c956fc47e..090cba8da874e 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -254,9 +254,9 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content - issue.Version = version + issue.Version = version + 1 - affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content").Update(issue) + affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "version").Where("version = ?", version).Update(issue) if err != nil { return err } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 151a041352999..54bafc8f45504 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1443,7 +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.already_changed = Unable to save changes to the issue. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes 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 @@ -1759,6 +1759,7 @@ compare.compare_head = compare pulls.desc = Enable pull requests and code reviews. pulls.new = New Pull Request pulls.new.blocked_user = Cannot create pull request because you are blocked by the repository owner. +pulls.edit.already_changed = Unable to save changes to the pull request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes pulls.view = View Pull Request pulls.compare_changes = New Pull Request pulls.allow_edits_from_maintainers = Allow edits from maintainers diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 5e7fbf713c097..b263c208b8ec1 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2251,7 +2251,11 @@ func UpdateIssueContent(ctx *context.Context) { 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")) + if issue.IsPull { + ctx.JSONError(ctx.Tr("repo.pulls.edit.already_changed")) + } else { + ctx.JSONError(ctx.Tr("repo.issues.edit.already_changed")) + } } else { ctx.ServerError("ChangeContent", err) } @@ -2281,6 +2285,7 @@ func UpdateIssueContent(ctx *context.Context) { ctx.JSON(http.StatusOK, map[string]any{ "content": content, + "version": issue.Version, "attachments": attachmentsHTML(ctx, issue.Attachments, issue.Content), }) } diff --git a/web_src/js/features/repo-issue-edit.js b/web_src/js/features/repo-issue-edit.js index b1798ee45d862..3b7a357a275bb 100644 --- a/web_src/js/features/repo-issue-edit.js +++ b/web_src/js/features/repo-issue-edit.js @@ -122,12 +122,10 @@ 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, + version: editContentZone.getAttribute('data-version'), }); for (const fileInput of dropzoneInst?.element.querySelectorAll('.files [name=files]')) params.append('files[]', fileInput.value); @@ -137,7 +135,7 @@ async function onEditContent(event) { showErrorToast(data.errorMessage); return; } - editContentZone.setAttribute('data-version', parseInt(version) + 1); + editContentZone.setAttribute('data-version', data.version); if (!data.content) { renderContent.innerHTML = document.getElementById('no-content').innerHTML; rawContent.textContent = ''; diff --git a/web_src/js/markup/tasklist.js b/web_src/js/markup/tasklist.js index 7f70ff5b7e581..4b9df4c402e90 100644 --- a/web_src/js/markup/tasklist.js +++ b/web_src/js/markup/tasklist.js @@ -66,10 +66,9 @@ export function initMarkupTasklist() { const data = await response.json(); if (response.status === 400) { showErrorToast(data.errorMessage); - rawContent.textContent = oldContent; return; } - editContentZone.setAttribute('data-version', parseInt(version) + 1); + editContentZone.setAttribute('data-version', data.version); rawContent.textContent = newContent; } catch (err) { checkbox.checked = !checkbox.checked; From aadeb2e225a12666e7da92ec11f231731f06af59 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Thu, 23 May 2024 16:33:51 +0000 Subject: [PATCH 04/11] fix migration --- models/migrations/v1_23/v299.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index d7617eb892d5d..e88ad029bfe46 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -7,7 +7,7 @@ import "xorm.io/xorm" func AddVersionToIssue(x *xorm.Engine) error { type Issue struct { - Version int `xorm:"version"` + Version int `xorm:"NOT NULL DEFAULT 0"` } sess := x.NewSession() From c3e57a2483ea28a7cbb17b05f0e4e0a730289f00 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Fri, 24 May 2024 15:55:52 +0000 Subject: [PATCH 05/11] rename Version to ContentVersion --- models/issues/issue.go | 2 +- models/issues/issue_update.go | 4 ++-- models/migrations/migrations.go | 2 +- models/migrations/v1_23/v299.go | 4 ++-- routers/api/v1/repo/issue.go | 2 +- routers/api/v1/repo/issue_attachment.go | 2 +- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/issue.go | 8 ++++---- templates/repo/issue/view_content.tmpl | 2 +- web_src/js/features/repo-issue-edit.js | 4 ++-- web_src/js/markup/tasklist.js | 6 +++--- 11 files changed, 19 insertions(+), 19 deletions(-) diff --git a/models/issues/issue.go b/models/issues/issue.go index 41d56def39f7f..aad855522d164 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -109,6 +109,7 @@ type Issue struct { Title string `xorm:"name"` Content string `xorm:"LONGTEXT"` RenderedContent template.HTML `xorm:"-"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` Labels []*Label `xorm:"-"` MilestoneID int64 `xorm:"INDEX"` Milestone *Milestone `xorm:"-"` @@ -123,7 +124,6 @@ type Issue struct { NumComments int Ref string PinOrder int `xorm:"DEFAULT 0"` - Version int `xorm:"NOT NULL DEFAULT 0"` DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"` diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 090cba8da874e..581f7db0a0c23 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -254,9 +254,9 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content - issue.Version = version + 1 + issue.ContentVersion = version + 1 - affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "version").Where("version = ?", version).Update(issue) + affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", version).Update(issue) if err != nil { return err } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 35189a01b6f86..506c77667edd4 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -590,7 +590,7 @@ var migrations = []Migration{ // Gitea 1.22.0-rc1 ends at 299 // v299 -> v300 - NewMigration("Add version to issue table", v1_23.AddVersionToIssue), + NewMigration("Add content version to issue table", v1_23.AddContentVersionToIssue), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index e88ad029bfe46..1c598baf37987 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -5,9 +5,9 @@ package v1_23 //nolint import "xorm.io/xorm" -func AddVersionToIssue(x *xorm.Engine) error { +func AddContentVersionToIssue(x *xorm.Engine) error { type Issue struct { - Version int `xorm:"NOT NULL DEFAULT 0"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` } sess := x.NewSession() diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 7c2e353274ab7..ddfc36f17dffb 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -810,7 +810,7 @@ func EditIssue(ctx *context.APIContext) { } } if form.Body != nil { - err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.Version) + err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.ContentVersion) if err != nil { if errors.Is(err, issues_model.ErrIssueAlreadyChanged) { ctx.Error(http.StatusBadRequest, "ChangeContent", err) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 9404b96fa4349..ef846a43a346e 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/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, issue.Version); err != nil { + if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, issue.Content, issue.ContentVersion); err != nil { ctx.Error(http.StatusInternalServerError, "ChangeContent", err) return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index fe2bf0bf4cdd7..a9aa5c4d8efde 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -610,7 +610,7 @@ func EditPullRequest(ctx *context.APIContext) { } } if form.Body != nil { - err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.Version) + err = issue_service.ChangeContent(ctx, issue, ctx.Doer, *form.Body, issue.ContentVersion) if err != nil { if errors.Is(err, issues_model.ErrIssueAlreadyChanged) { ctx.Error(http.StatusBadRequest, "ChangeContent", err) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index b263c208b8ec1..2072e3a0103b3 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2247,7 +2247,7 @@ func UpdateIssueContent(ctx *context.Context) { return } - if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, ctx.Req.FormValue("content"), ctx.FormInt("version")); err != nil { + if err := issue_service.ChangeContent(ctx, issue, ctx.Doer, ctx.Req.FormValue("content"), ctx.FormInt("content_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) { @@ -2284,9 +2284,9 @@ func UpdateIssueContent(ctx *context.Context) { } ctx.JSON(http.StatusOK, map[string]any{ - "content": content, - "version": issue.Version, - "attachments": attachmentsHTML(ctx, issue.Attachments, issue.Content), + "content": content, + "contentVersion": issue.ContentVersion, + "attachments": attachmentsHTML(ctx, issue.Attachments, issue.Content), }) } diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index dc57793dc77b5..3088c605104f1 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -60,7 +60,7 @@ {{end}}
{{.Issue.Content}}
-
+
{{if .Issue.Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Issue.Attachments "RenderedContent" .Issue.RenderedContent}} {{end}} diff --git a/web_src/js/features/repo-issue-edit.js b/web_src/js/features/repo-issue-edit.js index 3b7a357a275bb..9a8d737e0158c 100644 --- a/web_src/js/features/repo-issue-edit.js +++ b/web_src/js/features/repo-issue-edit.js @@ -125,7 +125,7 @@ async function onEditContent(event) { const params = new URLSearchParams({ content: comboMarkdownEditor.value(), context: editContentZone.getAttribute('data-context'), - version: editContentZone.getAttribute('data-version'), + content_version: editContentZone.getAttribute('data-content-version'), }); for (const fileInput of dropzoneInst?.element.querySelectorAll('.files [name=files]')) params.append('files[]', fileInput.value); @@ -135,7 +135,7 @@ async function onEditContent(event) { showErrorToast(data.errorMessage); return; } - editContentZone.setAttribute('data-version', data.version); + editContentZone.setAttribute('data-content-version', data.contentVersion); if (!data.content) { renderContent.innerHTML = document.getElementById('no-content').innerHTML; rawContent.textContent = ''; diff --git a/web_src/js/markup/tasklist.js b/web_src/js/markup/tasklist.js index 4b9df4c402e90..a40b5e4abd37c 100644 --- a/web_src/js/markup/tasklist.js +++ b/web_src/js/markup/tasklist.js @@ -55,20 +55,20 @@ 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 contentVersion = editContentZone.getAttribute('data-content-version'); const requestBody = new FormData(); requestBody.append('ignore_attachments', 'true'); requestBody.append('content', newContent); requestBody.append('context', context); - requestBody.append('version', version); + requestBody.append('content_version', contentVersion); const response = await POST(updateUrl, {data: requestBody}); const data = await response.json(); if (response.status === 400) { showErrorToast(data.errorMessage); return; } - editContentZone.setAttribute('data-version', data.version); + editContentZone.setAttribute('data-content-version', data.contentVersion); rawContent.textContent = newContent; } catch (err) { checkbox.checked = !checkbox.checked; From 36f80595d7e7aee9a340a6dd681c06725a47ee00 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Fri, 24 May 2024 16:03:45 +0000 Subject: [PATCH 06/11] change parameter name --- services/issue/content.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/issue/content.go b/services/issue/content.go index 0b8693a41898e..6894182909275 100644 --- a/services/issue/content.go +++ b/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, version int) error { +func ChangeContent(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, content string, contentVersion 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, version); err != nil { + if err := issues_model.ChangeIssueContent(ctx, issue, doer, content, contentVersion); err != nil { return err } From c42ad73433a0efe8e747766e3e2b49ab62797b98 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Fri, 24 May 2024 16:07:45 +0000 Subject: [PATCH 07/11] one more parameter rename --- models/issues/issue_update.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 581f7db0a0c23..31d76be5e0aea 100644 --- a/models/issues/issue_update.go +++ b/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, version int) (err error) { +func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string, contentVersion int) (err error) { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -254,9 +254,9 @@ func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User } issue.Content = content - issue.ContentVersion = version + 1 + issue.ContentVersion = contentVersion + 1 - affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", version).Update(issue) + affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", contentVersion).Update(issue) if err != nil { return err } From be256750678b65593a12c02e370b10d72897d48f Mon Sep 17 00:00:00 2001 From: metiftikci Date: Sat, 25 May 2024 22:20:51 +0000 Subject: [PATCH 08/11] add content version to comment table --- models/issues/comment.go | 13 +++++++++++-- models/migrations/migrations.go | 2 +- models/migrations/v1_23/v299.go | 10 +++++++++- options/locale/locale_en-US.ini | 2 ++ routers/api/v1/repo/issue_comment.go | 2 +- routers/api/v1/repo/issue_comment_attachment.go | 2 +- routers/web/repo/issue.go | 10 +++++++--- services/issue/comments.go | 4 ++-- templates/repo/diff/comments.tmpl | 2 +- templates/repo/issue/view_content/comments.tmpl | 4 ++-- templates/repo/issue/view_content/conversation.tmpl | 2 +- 11 files changed, 38 insertions(+), 15 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 353163ebd6f99..584142d98c7f0 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -52,6 +52,8 @@ func (err ErrCommentNotExist) Unwrap() error { return util.ErrNotExist } +var ErrCommentAlreadyChanged = util.NewInvalidArgumentErrorf("the comment is already changed") + // CommentType defines whether a comment is just a simple comment, an action (like close) or a reference. type CommentType int @@ -262,6 +264,7 @@ type Comment struct { Line int64 // - previous line / + proposed line TreePath string Content string `xorm:"LONGTEXT"` + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` RenderedContent template.HTML `xorm:"-"` // Path represents the 4 lines of code cemented by this comment @@ -1111,7 +1114,7 @@ func UpdateCommentInvalidate(ctx context.Context, c *Comment) error { } // UpdateComment updates information of comment. -func UpdateComment(ctx context.Context, c *Comment, doer *user_model.User) error { +func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *user_model.User) error { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -1119,9 +1122,15 @@ func UpdateComment(ctx context.Context, c *Comment, doer *user_model.User) error defer committer.Close() sess := db.GetEngine(ctx) - if _, err := sess.ID(c.ID).AllCols().Update(c); err != nil { + c.ContentVersion = contentVersion + 1 + + affected, err := sess.ID(c.ID).AllCols().Where("content_version = ?", contentVersion).Update(c) + if err != nil { return err } + if affected == 0 { + return ErrCommentAlreadyChanged + } if err := c.LoadIssue(ctx); err != nil { return err } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 506c77667edd4..08882fb1195cc 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -590,7 +590,7 @@ var migrations = []Migration{ // Gitea 1.22.0-rc1 ends at 299 // v299 -> v300 - NewMigration("Add content version to issue table", v1_23.AddContentVersionToIssue), + NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index 1c598baf37987..9f0d373a7bc14 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -5,11 +5,15 @@ package v1_23 //nolint import "xorm.io/xorm" -func AddContentVersionToIssue(x *xorm.Engine) error { +func AddContentVersionToIssueAndComment(x *xorm.Engine) error { type Issue struct { ContentVersion int `xorm:"NOT NULL DEFAULT 0"` } + type Comment struct { + ContentVersion int `xorm:"NOT NULL DEFAULT 0"` + } + sess := x.NewSession() defer sess.Close() @@ -21,5 +25,9 @@ func AddContentVersionToIssue(x *xorm.Engine) error { return err } + if err := sess.Sync(new(Comment)); err != nil { + return err + } + return sess.Commit() } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 54bafc8f45504..e294c6372cab9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1905,6 +1905,8 @@ pulls.recently_pushed_new_branches = You pushed on branch %[1]s pull.deleted_branch = (deleted):%s +comments.edit.already_changed = Unable to save changes to the comment. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes + milestones.new = New Milestone milestones.closed = Closed %s milestones.update_ago = Updated %s diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 070571ba62e72..910cc1ce7456b 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -611,7 +611,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) oldContent := comment.Content comment.Content = form.Body - if err := issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent); err != nil { + if err := issue_service.UpdateComment(ctx, comment, comment.ContentVersion, ctx.Doer, oldContent); err != nil { if errors.Is(err, user_model.ErrBlockedUser) { ctx.Error(http.StatusForbidden, "UpdateComment", err) } else { diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 77aa7f0400499..1ec758ec2ce8f 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -210,7 +210,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { return } - if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, comment.Content); err != nil { + if err = issue_service.UpdateComment(ctx, comment, comment.ContentVersion, ctx.Doer, comment.Content); err != nil { if errors.Is(err, user_model.ErrBlockedUser) { ctx.Error(http.StatusForbidden, "UpdateComment", err) } else { diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 2072e3a0103b3..734cb1461dcc3 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -3160,12 +3160,15 @@ func UpdateCommentContent(ctx *context.Context) { oldContent := comment.Content newContent := ctx.FormString("content") + contentVersion := ctx.FormInt("content_version") // allow to save empty content comment.Content = newContent - if err = issue_service.UpdateComment(ctx, comment, ctx.Doer, oldContent); err != nil { + if err = issue_service.UpdateComment(ctx, comment, contentVersion, ctx.Doer, oldContent); err != nil { if errors.Is(err, user_model.ErrBlockedUser) { ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user")) + } else if errors.Is(err, issues_model.ErrCommentAlreadyChanged) { + ctx.JSONError(ctx.Tr("repo.comments.edit.already_changed")) } else { ctx.ServerError("UpdateComment", err) } @@ -3205,8 +3208,9 @@ func UpdateCommentContent(ctx *context.Context) { } ctx.JSON(http.StatusOK, map[string]any{ - "content": renderedContent, - "attachments": attachmentsHTML(ctx, comment.Attachments, comment.Content), + "content": renderedContent, + "contentVersion": comment.ContentVersion, + "attachments": attachmentsHTML(ctx, comment.Attachments, comment.Content), }) } diff --git a/services/issue/comments.go b/services/issue/comments.go index d68623aff6833..33b5702a00c4d 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -82,7 +82,7 @@ func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_m } // UpdateComment updates information of comment. -func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_model.User, oldContent string) error { +func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion int, doer *user_model.User, oldContent string) error { if err := c.LoadIssue(ctx); err != nil { return err } @@ -110,7 +110,7 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, doer *user_mode } } - if err := issues_model.UpdateComment(ctx, c, doer); err != nil { + if err := issues_model.UpdateComment(ctx, c, contentVersion, doer); err != nil { return err } diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index c7f4337182415..90d6a511bfab6 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -61,7 +61,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index acc04e4c61514..3da2f3815ee3f 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -67,7 +67,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} @@ -441,7 +441,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index ac32a2db5d2a2..43ec9d75c4176 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -96,7 +96,7 @@ {{end}}
{{.Content}}
-
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" dict "Attachments" .Attachments "RenderedContent" .RenderedContent}} {{end}} From c0ee1945d49062e79fddbae10d5c3d2aa2019c34 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Sat, 25 May 2024 22:23:36 +0000 Subject: [PATCH 09/11] try to add test --- tests/integration/issue_test.go | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index d74516d110a61..de83f32166ce0 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -257,6 +257,44 @@ func TestIssueCommentUpdate(t *testing.T) { assert.Equal(t, modifiedContent, comment.Content) } +func TestIssueCommentUpdateSimultaneously(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description") + comment1 := "Test comment 1" + commentID := testIssueAddComment(t, session, issueURL, comment1, "") + + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID}) + assert.Equal(t, comment1, comment.Content) + + modifiedContent := comment.Content + "MODIFIED" + + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + }) + session.MakeRequest(t, req, http.StatusOK) + + modifiedContent = comment.Content + "2" + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + }) + session.MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": modifiedContent, + "content_version": "1", + }) + session.MakeRequest(t, req, http.StatusOK) + + comment = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID}) + assert.Equal(t, modifiedContent, comment.Content) + assert.Equal(t, 2, comment.ContentVersion) +} + func TestIssueReaction(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") From 1eb57b04c94c3e7c8a1e618ecbf8cdabea55c285 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Sat, 25 May 2024 23:03:55 +0000 Subject: [PATCH 10/11] add test --- tests/integration/issue_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index de83f32166ce0..308b82d4b950b 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -191,6 +191,34 @@ func TestNewIssue(t *testing.T) { testNewIssue(t, session, "user2", "repo1", "Title", "Description") } +func TestEditIssue(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description") + + req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusOK) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusBadRequest) + + req = NewRequestWithValues(t, "POST", fmt.Sprintf("%s/content", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "content": "modified content", + "content_version": "1", + "context": fmt.Sprintf("/%s/%s", "user2", "repo1"), + }) + session.MakeRequest(t, req, http.StatusOK) +} + func TestIssueCommentClose(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") From bc47d9f11c9e997096840bfde9d98d8e63005d38 Mon Sep 17 00:00:00 2001 From: metiftikci Date: Mon, 27 May 2024 13:43:01 +0000 Subject: [PATCH 11/11] fix migration --- models/migrations/v1_23/v299.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/models/migrations/v1_23/v299.go b/models/migrations/v1_23/v299.go index 9f0d373a7bc14..f6db960c3b41b 100644 --- a/models/migrations/v1_23/v299.go +++ b/models/migrations/v1_23/v299.go @@ -14,20 +14,5 @@ func AddContentVersionToIssueAndComment(x *xorm.Engine) error { ContentVersion int `xorm:"NOT NULL DEFAULT 0"` } - sess := x.NewSession() - defer sess.Close() - - if err := sess.Begin(); err != nil { - return err - } - - if err := sess.Sync(new(Issue)); err != nil { - return err - } - - if err := sess.Sync(new(Comment)); err != nil { - return err - } - - return sess.Commit() + return x.Sync(new(Comment), new(Issue)) }