Save and view issue/comment content history (#16909)

* issue content history

* Use timeutil.TimeStampNow() for content history time instead of issue/comment.UpdatedUnix (which are not updated in time)

* i18n for frontend

* refactor

* clean up

* fix refactor

* re-format

* temp refactor

* follow db refactor

* rename IssueContentHistory to ContentHistory, remove empty model tags

* fix html

* use avatar refactor to generate avatar url

* add unit test, keep at most 20 history revisions.

* re-format

* syntax nit

* Add issue content history table

* Update models/migrations/v197.go

Co-authored-by: 6543 <6543@obermui.de>

* fix merge

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Lauris BH <lauris@nix.lv>

models/db/unit_tests.go

@@ -54,7 +54,9 @@ func MainTest(m *testing.M, pathToGiteaRoot string, fixtureFiles ...string) {
 		opts.Dir = fixturesDir
 	} else {
 		for _, f := range fixtureFiles {
-			opts.Files = append(opts.Files, filepath.Join(fixturesDir, f))
+			if len(f) != 0 {
+				opts.Files = append(opts.Files, filepath.Join(fixturesDir, f))
+			}
 		}
 	}
 

models/issue.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/references"
@@ -803,8 +804,13 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
 		return fmt.Errorf("UpdateIssueCols: %v", err)
 	}
 
-	if err = issue.addCrossReferences(db.GetEngine(ctx), doer, true); err != nil {
-		return err
+	if err = issues.SaveIssueContentHistory(db.GetEngine(ctx), issue.PosterID, issue.ID, 0,
+		timeutil.TimeStampNow(), issue.Content, false); err != nil {
+		return fmt.Errorf("SaveIssueContentHistory: %v", err)
+	}
+
+	if err = issue.addCrossReferences(ctx.Engine(), doer, true); err != nil {
+		return fmt.Errorf("addCrossReferences: %v", err)
 	}
 
 	return committer.Commit()
@@ -972,6 +978,12 @@ func newIssue(e db.Engine, doer *User, opts NewIssueOptions) (err error) {
 	if err = opts.Issue.loadAttributes(e); err != nil {
 		return err
 	}
+
+	if err = issues.SaveIssueContentHistory(e, opts.Issue.PosterID, opts.Issue.ID, 0,
+		timeutil.TimeStampNow(), opts.Issue.Content, true); err != nil {
+		return err
+	}
+
 	return opts.Issue.addCrossReferences(e, doer, false)
 }
 
@@ -2132,6 +2144,12 @@ func UpdateReactionsMigrationsByType(gitServiceType structs.GitServiceType, orig
 func deleteIssuesByRepoID(sess db.Engine, repoID int64) (attachmentPaths []string, err error) {
 	deleteCond := builder.Select("id").From("issue").Where(builder.Eq{"issue.repo_id": repoID})
 
+	// Delete content histories
+	if _, err = sess.In("issue_id", deleteCond).
+		Delete(&issues.ContentHistory{}); err != nil {
+		return
+	}
+
 	// Delete comments and attachments
 	if _, err = sess.In("issue_id", deleteCond).
 		Delete(&Comment{}); err != nil {

models/issue_comment.go

@@ -14,6 +14,7 @@ import (
 	"unicode/utf8"
 
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
@@ -1083,6 +1084,12 @@ func deleteComment(e db.Engine, comment *Comment) error {
 		return err
 	}
 
+	if _, err := e.Delete(&issues.ContentHistory{
+		CommentID: comment.ID,
+	}); err != nil {
+		return err
+	}
+
 	if comment.Type == CommentTypeComment {
 		if _, err := e.Exec("UPDATE `issue` SET num_comments = num_comments - 1 WHERE id = ?", comment.IssueID); err != nil {
 			return err

models/issues/content_history.go

@@ -0,0 +1,230 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package issues
+
+import (
+	"context"
+	"fmt"
+
+	"code.gitea.io/gitea/models/avatars"
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/timeutil"
+
+	"xorm.io/builder"
+)
+
+// ContentHistory save issue/comment content history revisions.
+type ContentHistory struct {
+	ID             int64 `xorm:"pk autoincr"`
+	PosterID       int64
+	IssueID        int64              `xorm:"INDEX"`
+	CommentID      int64              `xorm:"INDEX"`
+	EditedUnix     timeutil.TimeStamp `xorm:"INDEX"`
+	ContentText    string             `xorm:"LONGTEXT"`
+	IsFirstCreated bool
+	IsDeleted      bool
+}
+
+// TableName provides the real table name
+func (m *ContentHistory) TableName() string {
+	return "issue_content_history"
+}
+
+func init() {
+	db.RegisterModel(new(ContentHistory))
+}
+
+// SaveIssueContentHistory save history
+func SaveIssueContentHistory(e db.Engine, posterID, issueID, commentID int64, editTime timeutil.TimeStamp, contentText string, isFirstCreated bool) error {
+	ch := &ContentHistory{
+		PosterID:       posterID,
+		IssueID:        issueID,
+		CommentID:      commentID,
+		ContentText:    contentText,
+		EditedUnix:     editTime,
+		IsFirstCreated: isFirstCreated,
+	}
+	_, err := e.Insert(ch)
+	if err != nil {
+		log.Error("can not save issue content history. err=%v", err)
+		return err
+	}
+	// We only keep at most 20 history revisions now. It is enough in most cases.
+	// If there is a special requirement to keep more, we can consider introducing a new setting option then, but not now.
+	keepLimitedContentHistory(e, issueID, commentID, 20)
+	return nil
+}
+
+// keepLimitedContentHistory keeps at most `limit` history revisions, it will hard delete out-dated revisions, sorting by revision interval
+// we can ignore all errors in this function, so we just log them
+func keepLimitedContentHistory(e db.Engine, issueID, commentID int64, limit int) {
+	type IDEditTime struct {
+		ID         int64
+		EditedUnix timeutil.TimeStamp
+	}
+
+	var res []*IDEditTime
+	err := e.Select("id, edited_unix").Table("issue_content_history").
+		Where(builder.Eq{"issue_id": issueID, "comment_id": commentID}).
+		OrderBy("edited_unix ASC").
+		Find(&res)
+	if err != nil {
+		log.Error("can not query content history for deletion, err=%v", err)
+		return
+	}
+	if len(res) <= 1 {
+		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++ {
+			editedInterval := int(res[i].EditedUnix - res[i-1].EditedUnix)
+			if minEditedInterval == -1 || editedInterval < minEditedInterval {
+				minEditedInterval = editedInterval
+				indexToDelete = i
+			}
+		}
+		if indexToDelete == 0 {
+			break
+		}
+
+		// hard delete the found one
+		_, err = e.Delete(&ContentHistory{ID: res[indexToDelete].ID})
+		if err != nil {
+			log.Error("can not delete out-dated content history, err=%v", err)
+			break
+		}
+		res = append(res[:indexToDelete], res[indexToDelete+1:]...)
+		outDatedCount--
+	}
+}
+
+// QueryIssueContentHistoryEditedCountMap query related history count of each comment (comment_id = 0 means the main issue)
+// only return the count map for "edited" (history revision count > 1) issues or comments.
+func QueryIssueContentHistoryEditedCountMap(dbCtx context.Context, issueID int64) (map[int64]int, error) {
+	type HistoryCountRecord struct {
+		CommentID    int64
+		HistoryCount int
+	}
+	records := make([]*HistoryCountRecord, 0)
+
+	err := db.GetEngine(dbCtx).Select("comment_id, COUNT(1) as history_count").
+		Table("issue_content_history").
+		Where(builder.Eq{"issue_id": issueID}).
+		GroupBy("comment_id").
+		Having("history_count > 1").
+		Find(&records)
+	if err != nil {
+		log.Error("can not query issue content history count map. err=%v", err)
+		return nil, err
+	}
+
+	res := map[int64]int{}
+	for _, r := range records {
+		res[r.CommentID] = r.HistoryCount
+	}
+	return res, nil
+}
+
+// IssueContentListItem the list for web ui
+type IssueContentListItem struct {
+	UserID         int64
+	UserName       string
+	UserAvatarLink string
+
+	HistoryID      int64
+	EditedUnix     timeutil.TimeStamp
+	IsFirstCreated bool
+	IsDeleted      bool
+}
+
+// FetchIssueContentHistoryList fetch list
+func FetchIssueContentHistoryList(dbCtx context.Context, issueID int64, commentID int64) ([]*IssueContentListItem, error) {
+	res := make([]*IssueContentListItem, 0)
+	err := db.GetEngine(dbCtx).Select("u.id as user_id, u.name as user_name,"+
+		"h.id as history_id, h.edited_unix, h.is_first_created, h.is_deleted").
+		Table([]string{"issue_content_history", "h"}).
+		Join("LEFT", []string{"user", "u"}, "h.poster_id = u.id").
+		Where(builder.Eq{"issue_id": issueID, "comment_id": commentID}).
+		OrderBy("edited_unix DESC").
+		Find(&res)
+
+	if err != nil {
+		log.Error("can not fetch issue content history list. err=%v", err)
+		return nil, err
+	}
+
+	for _, item := range res {
+		item.UserAvatarLink = avatars.GenerateUserAvatarFastLink(item.UserName, 0)
+	}
+	return res, nil
+}
+
+//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,
+		ContentText: "",
+	}); err != nil {
+		log.Error("failed to soft delete issue content history. err=%v", err)
+		return err
+	}
+	return nil
+}
+
+// ErrIssueContentHistoryNotExist not exist error
+type ErrIssueContentHistoryNotExist struct {
+	ID int64
+}
+
+// Error error string
+func (err ErrIssueContentHistoryNotExist) Error() string {
+	return fmt.Sprintf("issue content history does not exist [id: %d]", err.ID)
+}
+
+// GetIssueContentHistoryByID get issue content history
+func GetIssueContentHistoryByID(dbCtx context.Context, id int64) (*ContentHistory, error) {
+	h := &ContentHistory{}
+	has, err := db.GetEngine(dbCtx).ID(id).Get(h)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrIssueContentHistoryNotExist{id}
+	}
+	return h, nil
+}
+
+// GetIssueContentHistoryAndPrev get a history and the previous non-deleted history (to compare)
+func GetIssueContentHistoryAndPrev(dbCtx context.Context, id int64) (history, prevHistory *ContentHistory, err error) {
+	history = &ContentHistory{}
+	has, err := db.GetEngine(dbCtx).ID(id).Get(history)
+	if err != nil {
+		log.Error("failed to get issue content history %v. err=%v", id, err)
+		return nil, nil, err
+	} else if !has {
+		log.Error("issue content history does not exist. id=%v. err=%v", id, err)
+		return nil, nil, &ErrIssueContentHistoryNotExist{id}
+	}
+
+	prevHistory = &ContentHistory{}
+	has, err = db.GetEngine(dbCtx).Where(builder.Eq{"issue_id": history.IssueID, "comment_id": history.CommentID, "is_deleted": false}).
+		And(builder.Lt{"edited_unix": history.EditedUnix}).
+		OrderBy("edited_unix DESC").Limit(1).
+		Get(prevHistory)
+
+	if err != nil {
+		log.Error("failed to get issue content history %v. err=%v", id, err)
+		return nil, nil, err
+	} else if !has {
+		return history, nil, nil
+	}
+
+	return history, prevHistory, nil
+}

models/issues/content_history_test.go

@@ -0,0 +1,74 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package issues
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/modules/timeutil"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestContentHistory(t *testing.T) {
+	assert.NoError(t, db.PrepareTestDatabase())
+
+	dbCtx := db.DefaultContext
+	dbEngine := db.GetEngine(dbCtx)
+	timeStampNow := timeutil.TimeStampNow()
+
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 0, timeStampNow, "i-a", true)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 0, timeStampNow.Add(2), "i-b", false)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 0, timeStampNow.Add(7), "i-c", false)
+
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 100, timeStampNow, "c-a", true)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 100, timeStampNow.Add(5), "c-b", false)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 100, timeStampNow.Add(20), "c-c", false)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 100, timeStampNow.Add(50), "c-d", false)
+	_ = SaveIssueContentHistory(dbEngine, 1, 10, 100, timeStampNow.Add(51), "c-e", false)
+
+	h1, _ := GetIssueContentHistoryByID(dbCtx, 1)
+	assert.EqualValues(t, 1, h1.ID)
+
+	m, _ := QueryIssueContentHistoryEditedCountMap(dbCtx, 10)
+	assert.Equal(t, 3, m[0])
+	assert.Equal(t, 5, m[100])
+
+	/*
+		we can not have this test with real `User` now, because we can not depend on `User` model (circle-import), so there is no `user` table
+		when the refactor of models are done, this test will be possible to be run then with a real `User` model.
+	*/
+	type User struct {
+		ID   int64
+		Name string
+	}
+	_ = dbEngine.Sync2(&User{})
+
+	list1, _ := FetchIssueContentHistoryList(dbCtx, 10, 0)
+	assert.Len(t, list1, 3)
+	list2, _ := FetchIssueContentHistoryList(dbCtx, 10, 100)
+	assert.Len(t, list2, 5)
+
+	h6, h6Prev, _ := GetIssueContentHistoryAndPrev(dbCtx, 6)
+	assert.EqualValues(t, 6, h6.ID)
+	assert.EqualValues(t, 5, h6Prev.ID)
+
+	// soft-delete
+	_ = SoftDeleteIssueContentHistory(dbCtx, 5)
+	h6, h6Prev, _ = GetIssueContentHistoryAndPrev(dbCtx, 6)
+	assert.EqualValues(t, 6, h6.ID)
+	assert.EqualValues(t, 4, h6Prev.ID)
+
+	// only keep 3 history revisions for comment_id=100
+	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, 4, list2[2].HistoryID)
+}