Merge branch 'main' into readeable-error

Author: wxiaoguang

docs/content/doc/developers/guidelines-frontend.md

@@ -40,6 +40,19 @@ We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/h
 7. Simple pages and SEO-related pages use Go HTML Template render to generate static Fomantic-UI HTML output. Complex pages can use Vue2 (or Vue3 in future).
 
 
+### Framework Usage
+
+Mixing different frameworks together is highly discouraged. A JavaScript module should follow one major framework and follow the framework's best practice.
+
+Recommended implementations:
+* Vue + Native
+* Fomantic-UI (jQuery)
+* Native only
+
+Discouraged implementations:
+* Vue + jQuery
+* jQuery + Native
+
 ### `async` Functions
 
 Only mark a function as `async` if and only if there are `await` calls 
@@ -98,6 +111,19 @@ $('#el').on('click', async (e) => {  // not recommended but acceptable
 });
 ```
 
+### HTML Attributes and `dataset`
+
+We forbid `dataset` usage, its camel-casing behaviour makes it hard to grep for attributes. However there are still some special cases, so the current guideline is:
+
+* For legacy code:
+  * `$.data()` should be refactored to `$.attr()`.
+  * `$.data()` can be used to bind some non-string data to elements in rare cases, but it is highly discouraged.
+
+* For new code:
+  * `node.dataset` should not be used, use `node.getAttribute` instead. 
+  * never bind any user data to a DOM node, use a suitable design pattern to describe the relation between node and data.
+
+
 ### Vue2/Vue3 and JSX
 
 Gitea is using Vue2 now, we plan to upgrade to Vue3. We decided not to introduce JSX to keep the HTML and the JavaScript code separated.

models/issue.go

@@ -824,14 +824,25 @@ func (issue *Issue) UpdateAttachments(uuids []string) (err error) {
 
 // ChangeContent changes issue content, as the given user.
 func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
-	issue.Content = content
-
 	ctx, committer, err := db.TxContext()
 	if err != nil {
 		return err
 	}
 	defer committer.Close()
 
+	hasContentHistory, err := issues.HasIssueContentHistory(ctx, issue.ID, 0)
+	if err != nil {
+		return fmt.Errorf("HasIssueContentHistory: %v", err)
+	}
+	if !hasContentHistory {
+		if err = issues.SaveIssueContentHistory(db.GetEngine(ctx), issue.PosterID, issue.ID, 0,
+			issue.CreatedUnix, issue.Content, true); err != nil {
+			return fmt.Errorf("SaveIssueContentHistory: %v", err)
+		}
+	}
+
+	issue.Content = content
+
 	if err = updateIssueCols(db.GetEngine(ctx), issue, "content"); err != nil {
 		return fmt.Errorf("UpdateIssueCols: %v", err)
 	}
@@ -1012,11 +1023,6 @@ func newIssue(ctx context.Context, doer *User, opts NewIssueOptions) (err error)
 		return err
 	}
 
-	if err = issues.SaveIssueContentHistory(e, doer.ID, opts.Issue.ID, 0,
-		timeutil.TimeStampNow(), opts.Issue.Content, true); err != nil {
-		return err
-	}
-
 	return opts.Issue.addCrossReferences(ctx, doer, false)
 }
 

models/issues/content_history.go

@@ -75,16 +75,16 @@ func keepLimitedContentHistory(e db.Engine, issueID, commentID int64, limit int)
 		log.Error("can not query content history for deletion, err=%v", err)
 		return
 	}
-	if len(res) <= 1 {
+	if len(res) <= 2 {
 		return
 	}
 
 	outDatedCount := len(res) - limit
 	for outDatedCount > 0 {
 		var indexToDelete int
 		minEditedInterval := -1
-		// find a history revision with minimal edited interval to delete
-		for i := 1; i < len(res); i++ {
+		// find a history revision with minimal edited interval to delete, the first and the last should never be deleted
+		for i := 1; i < len(res)-1; i++ {
 			editedInterval := int(res[i].EditedUnix - res[i-1].EditedUnix)
 			if minEditedInterval == -1 || editedInterval < minEditedInterval {
 				minEditedInterval = editedInterval
@@ -167,7 +167,20 @@ func FetchIssueContentHistoryList(dbCtx context.Context, issueID int64, commentI
 	return res, nil
 }
 
-//SoftDeleteIssueContentHistory soft delete
+// HasIssueContentHistory check if a ContentHistory entry exists
+func HasIssueContentHistory(dbCtx context.Context, issueID int64, commentID int64) (bool, error) {
+	exists, err := db.GetEngine(dbCtx).Cols("id").Exist(&ContentHistory{
+		IssueID:   issueID,
+		CommentID: commentID,
+	})
+	if err != nil {
+		log.Error("can not fetch issue content history. err=%v", err)
+		return false, err
+	}
+	return exists, err
+}
+
+// SoftDeleteIssueContentHistory soft delete
 func SoftDeleteIssueContentHistory(dbCtx context.Context, historyID int64) error {
 	if _, err := db.GetEngine(dbCtx).ID(historyID).Cols("is_deleted", "content_text").Update(&ContentHistory{
 		IsDeleted:   true,

models/issues/content_history_test.go

@@ -53,6 +53,11 @@ func TestContentHistory(t *testing.T) {
 	list2, _ := FetchIssueContentHistoryList(dbCtx, 10, 100)
 	assert.Len(t, list2, 5)
 
+	hasHistory1, _ := HasIssueContentHistory(dbCtx, 10, 0)
+	assert.True(t, hasHistory1)
+	hasHistory2, _ := HasIssueContentHistory(dbCtx, 10, 1)
+	assert.False(t, hasHistory2)
+
 	h6, h6Prev, _ := GetIssueContentHistoryAndPrev(dbCtx, 6)
 	assert.EqualValues(t, 6, h6.ID)
 	assert.EqualValues(t, 5, h6Prev.ID)
@@ -63,13 +68,13 @@ func TestContentHistory(t *testing.T) {
 	assert.EqualValues(t, 6, h6.ID)
 	assert.EqualValues(t, 4, h6Prev.ID)
 
-	// only keep 3 history revisions for comment_id=100
+	// only keep 3 history revisions for comment_id=100, the first and the last should never be deleted
 	keepLimitedContentHistory(dbEngine, 10, 100, 3)
 	list1, _ = FetchIssueContentHistoryList(dbCtx, 10, 0)
 	assert.Len(t, list1, 3)
 	list2, _ = FetchIssueContentHistoryList(dbCtx, 10, 100)
 	assert.Len(t, list2, 3)
-	assert.EqualValues(t, 7, list2[0].HistoryID)
-	assert.EqualValues(t, 6, list2[1].HistoryID)
+	assert.EqualValues(t, 8, list2[0].HistoryID)
+	assert.EqualValues(t, 7, list2[1].HistoryID)
 	assert.EqualValues(t, 4, list2[2].HistoryID)
 }

services/comments/comments.go

@@ -25,10 +25,6 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model
 	if err != nil {
 		return nil, err
 	}
-	err = issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), doer.ID, issue.ID, comment.ID, timeutil.TimeStampNow(), comment.Content, true)
-	if err != nil {
-		return nil, err
-	}
 
 	mentions, err := issue.FindAndUpdateIssueMentions(db.DefaultContext, doer, comment.Content)
 	if err != nil {
@@ -42,11 +38,26 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model
 
 // UpdateComment updates information of comment.
 func UpdateComment(c *models.Comment, doer *models.User, oldContent string) error {
+	var needsContentHistory = c.Content != oldContent &&
+		(c.Type == models.CommentTypeComment || c.Type == models.CommentTypeReview || c.Type == models.CommentTypeCode)
+	if needsContentHistory {
+		hasContentHistory, err := issues.HasIssueContentHistory(db.DefaultContext, c.IssueID, c.ID)
+		if err != nil {
+			return err
+		}
+		if !hasContentHistory {
+			if err = issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), c.PosterID, c.IssueID, c.ID,
+				c.CreatedUnix, oldContent, true); err != nil {
+				return err
+			}
+		}
+	}
+
 	if err := models.UpdateComment(c, doer); err != nil {
 		return err
 	}
 
-	if c.Type == models.CommentTypeComment && c.Content != oldContent {
+	if needsContentHistory {
 		err := issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), doer.ID, c.IssueID, c.ID, timeutil.TimeStampNow(), c.Content, false)
 		if err != nil {
 			return err

templates/repo/pulls/files.tmpl

@@ -1,4 +1,8 @@
 {{template "base/head" .}}
+
+<input type="hidden" id="repolink" value="{{$.RepoRelPath}}">
+<input type="hidden" id="issueIndex" value="{{.Issue.Index}}"/>
+
 <div class="page-content repository view issue pull files diff">
 	{{template "repo/header" .}}
 	<div class="ui container {{if .IsSplitStyle}}fluid padded{{end}}">

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

@@ -106,8 +106,11 @@ function showContentHistoryMenu(issueBaseUrl, $item, commentId) {
 
 export function initRepoIssueContentHistory() {
   const issueIndex = $('#issueIndex').val();
-  const $itemIssue = $('.timeline-item.comment.first');
-  if (!issueIndex || !$itemIssue.length) return;
+  if (!issueIndex) return;
+
+  const $itemIssue = $('.repository.issue .timeline-item.comment.first'); // issue(PR) main content
+  const $comments = $('.repository.issue .comment-list .comment'); // includes: issue(PR) comments, code rerview comments
+  if (!$itemIssue.length && !$comments.length) return;
 
   const repoLink = $('#repolink').val();
   const issueBaseUrl = `${appSubUrl}/${repoLink}/issues/${issueIndex}`;
@@ -123,7 +126,7 @@ export function initRepoIssueContentHistory() {
     i18nTextDeleteFromHistoryConfirm = resp.i18n.textDeleteFromHistoryConfirm;
     i18nTextOptions = resp.i18n.textOptions;
 
-    if (resp.editedHistoryCountMap[0]) {
+    if (resp.editedHistoryCountMap[0] && $itemIssue.length) {
       showContentHistoryMenu(issueBaseUrl, $itemIssue, '0');
     }
     for (const [commentId, _editedCount] of Object.entries(resp.editedHistoryCountMap)) {

web_src/js/features/repo-projects.js

@@ -1,22 +1,24 @@
 const {csrfToken} = window.config;
 
 async function initRepoProjectSortable() {
-  const els = document.getElementsByClassName('board');
+  const els = document.querySelectorAll('#project-board > .board');
   if (!els.length) return;
 
   const {Sortable} = await import(/* webpackChunkName: "sortable" */'sortablejs');
-  const boardColumns = document.getElementsByClassName('board-column');
 
-  new Sortable(els[0], {
+  // the HTML layout is: #project-board > .board > .board-column .board.cards > .board-card.card .content
+  const mainBoard = els[0];
+  let boardColumns = mainBoard.getElementsByClassName('board-column');
+  new Sortable(mainBoard, {
     group: 'board-column',
     draggable: '.board-column',
+    filter: '[data-id="0"]',
     animation: 150,
     ghostClass: 'card-ghost',
     onSort: () => {
-      const board = document.getElementsByClassName('board')[0];
-      const boardColumns = board.getElementsByClassName('board-column');
-
-      for (const [i, column] of boardColumns.entries()) {
+      boardColumns = mainBoard.getElementsByClassName('board-column');
+      for (let i = 0; i < boardColumns.length; i++) {
+        const column = boardColumns[i];
         if (parseInt($(column).data('sorting')) !== i) {
           $.ajax({
             url: $(column).data('url'),
@@ -33,8 +35,9 @@ async function initRepoProjectSortable() {
     },
   });
 
-  for (const column of boardColumns) {
-    new Sortable(column.getElementsByClassName('board')[0], {
+  for (const boardColumn of boardColumns) {
+    const boardCardList = boardColumn.getElementsByClassName('board')[0];
+    new Sortable(boardCardList, {
       group: 'shared',
       animation: 150,
       ghostClass: 'card-ghost',