From 2b99a58f540a15a04b48cba507ace8abf3c52014 Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Tue, 15 Apr 2025 22:35:22 +0800 Subject: [PATCH] Mark parent directory as viewed when all files are viewed (#33958) Fix #25644 --------- Co-authored-by: wxiaoguang --- routers/web/repo/commit.go | 2 +- routers/web/repo/compare.go | 2 +- routers/web/repo/pull.go | 24 ++---- routers/web/repo/treelist.go | 94 +++++++++++++++++----- routers/web/repo/treelist_test.go | 60 ++++++++++++++ services/gitdiff/gitdiff.go | 13 +-- web_src/js/components/DiffFileTree.vue | 15 +--- web_src/js/components/DiffFileTreeItem.vue | 60 +++++++------- web_src/js/features/file-fold.ts | 2 +- web_src/js/features/pull-view-file.ts | 9 +-- web_src/js/modules/diff-file.test.ts | 47 +++++++++++ web_src/js/modules/diff-file.ts | 78 ++++++++++++++++++ web_src/js/modules/stores.ts | 16 ---- web_src/js/utils/filetree.test.ts | 86 -------------------- web_src/js/utils/filetree.ts | 85 ------------------- 15 files changed, 313 insertions(+), 280 deletions(-) create mode 100644 routers/web/repo/treelist_test.go create mode 100644 web_src/js/modules/diff-file.test.ts create mode 100644 web_src/js/modules/diff-file.ts delete mode 100644 web_src/js/modules/stores.ts delete mode 100644 web_src/js/utils/filetree.test.ts delete mode 100644 web_src/js/utils/filetree.ts diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 3569a356d1..973d68d45c 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -369,7 +369,7 @@ func Diff(ctx *context.Context) { return } - ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil) + ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, nil) } statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 2c36477e6a..13fbac981c 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -639,7 +639,7 @@ func PrepareCompareDiff( return false } - ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil) + ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, nil) } headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c72664f8e9..0124c163f3 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -759,12 +759,9 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi // have to load only the diff and not get the viewed information // as the viewed information is designed to be loaded only on latest PR // diff and if you're signed in. - shouldGetUserSpecificDiff := false - if !ctx.IsSigned || willShowSpecifiedCommit || willShowSpecifiedCommitRange { - // do nothing - } else { - shouldGetUserSpecificDiff = true - err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions, files...) + var reviewState *pull_model.ReviewState + if ctx.IsSigned && !willShowSpecifiedCommit && !willShowSpecifiedCommitRange { + reviewState, err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions) if err != nil { ctx.ServerError("SyncUserSpecificDiff", err) return @@ -823,18 +820,11 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.ServerError("GetDiffTree", err) return } - - filesViewedState := make(map[string]pull_model.ViewedState) - if shouldGetUserSpecificDiff { - // This sort of sucks because we already fetch this when getting the diff - review, err := pull_model.GetNewestReviewState(ctx, ctx.Doer.ID, issue.ID) - if err == nil && review != nil && review.UpdatedFiles != nil { - // If there wasn't an error and we have a review with updated files, use that - filesViewedState = review.UpdatedFiles - } + var filesViewedState map[string]pull_model.ViewedState + if reviewState != nil { + filesViewedState = reviewState.UpdatedFiles } - - ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, filesViewedState) + ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, filesViewedState) } ctx.Data["Diff"] = diff diff --git a/routers/web/repo/treelist.go b/routers/web/repo/treelist.go index 9c5ec8f206..994b2d0c0a 100644 --- a/routers/web/repo/treelist.go +++ b/routers/web/repo/treelist.go @@ -5,6 +5,7 @@ package repo import ( "net/http" + "strings" pull_model "code.gitea.io/gitea/models/pull" "code.gitea.io/gitea/modules/base" @@ -57,34 +58,85 @@ func isExcludedEntry(entry *git.TreeEntry) bool { return false } -type FileDiffFile struct { - Name string +// WebDiffFileItem is used by frontend, check the field names in frontend before changing +type WebDiffFileItem struct { + FullName string + DisplayName string NameHash string - IsSubmodule bool + DiffStatus string + EntryMode string IsViewed bool - Status string + Children []*WebDiffFileItem + // TODO: add icon support in the future } -// transformDiffTreeForUI transforms a DiffTree into a slice of FileDiffFile for UI rendering +// WebDiffFileTree is used by frontend, check the field names in frontend before changing +type WebDiffFileTree struct { + TreeRoot WebDiffFileItem +} + +// transformDiffTreeForWeb transforms a gitdiff.DiffTree into a WebDiffFileTree for Web UI rendering // it also takes a map of file names to their viewed state, which is used to mark files as viewed -func transformDiffTreeForUI(diffTree *gitdiff.DiffTree, filesViewedState map[string]pull_model.ViewedState) []FileDiffFile { - files := make([]FileDiffFile, 0, len(diffTree.Files)) - - for _, file := range diffTree.Files { - nameHash := git.HashFilePathForWebUI(file.HeadPath) - isSubmodule := file.HeadMode == git.EntryModeCommit - isViewed := filesViewedState[file.HeadPath] == pull_model.Viewed - - files = append(files, FileDiffFile{ - Name: file.HeadPath, - NameHash: nameHash, - IsSubmodule: isSubmodule, - IsViewed: isViewed, - Status: file.Status, - }) +func transformDiffTreeForWeb(diffTree *gitdiff.DiffTree, filesViewedState map[string]pull_model.ViewedState) (dft WebDiffFileTree) { + dirNodes := map[string]*WebDiffFileItem{"": &dft.TreeRoot} + addItem := func(item *WebDiffFileItem) { + var parentPath string + pos := strings.LastIndexByte(item.FullName, '/') + if pos == -1 { + item.DisplayName = item.FullName + } else { + parentPath = item.FullName[:pos] + item.DisplayName = item.FullName[pos+1:] + } + parentNode, parentExists := dirNodes[parentPath] + if !parentExists { + parentNode = &dft.TreeRoot + fields := strings.Split(parentPath, "/") + for idx, field := range fields { + nodePath := strings.Join(fields[:idx+1], "/") + node, ok := dirNodes[nodePath] + if !ok { + node = &WebDiffFileItem{EntryMode: "tree", DisplayName: field, FullName: nodePath} + dirNodes[nodePath] = node + parentNode.Children = append(parentNode.Children, node) + } + parentNode = node + } + } + parentNode.Children = append(parentNode.Children, item) } - return files + for _, file := range diffTree.Files { + item := &WebDiffFileItem{FullName: file.HeadPath, DiffStatus: file.Status} + item.IsViewed = filesViewedState[item.FullName] == pull_model.Viewed + item.NameHash = git.HashFilePathForWebUI(item.FullName) + + switch file.HeadMode { + case git.EntryModeTree: + item.EntryMode = "tree" + case git.EntryModeCommit: + item.EntryMode = "commit" // submodule + default: + // default to empty, and will be treated as "blob" file because there is no "symlink" support yet + } + addItem(item) + } + + var mergeSingleDir func(node *WebDiffFileItem) + mergeSingleDir = func(node *WebDiffFileItem) { + if len(node.Children) == 1 { + if child := node.Children[0]; child.EntryMode == "tree" { + node.FullName = child.FullName + node.DisplayName = node.DisplayName + "/" + child.DisplayName + node.Children = child.Children + mergeSingleDir(node) + } + } + } + for _, node := range dft.TreeRoot.Children { + mergeSingleDir(node) + } + return dft } func TreeViewNodes(ctx *context.Context) { diff --git a/routers/web/repo/treelist_test.go b/routers/web/repo/treelist_test.go new file mode 100644 index 0000000000..2dff64a028 --- /dev/null +++ b/routers/web/repo/treelist_test.go @@ -0,0 +1,60 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "testing" + + pull_model "code.gitea.io/gitea/models/pull" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/services/gitdiff" + + "github.com/stretchr/testify/assert" +) + +func TestTransformDiffTreeForWeb(t *testing.T) { + ret := transformDiffTreeForWeb(&gitdiff.DiffTree{Files: []*gitdiff.DiffTreeRecord{ + { + Status: "changed", + HeadPath: "dir-a/dir-a-x/file-deep", + HeadMode: git.EntryModeBlob, + }, + { + Status: "added", + HeadPath: "file1", + HeadMode: git.EntryModeBlob, + }, + }}, map[string]pull_model.ViewedState{ + "dir-a/dir-a-x/file-deep": pull_model.Viewed, + }) + + assert.Equal(t, WebDiffFileTree{ + TreeRoot: WebDiffFileItem{ + Children: []*WebDiffFileItem{ + { + EntryMode: "tree", + DisplayName: "dir-a/dir-a-x", + FullName: "dir-a/dir-a-x", + Children: []*WebDiffFileItem{ + { + EntryMode: "", + DisplayName: "file-deep", + FullName: "dir-a/dir-a-x/file-deep", + NameHash: "4acf7eef1c943a09e9f754e93ff190db8583236b", + DiffStatus: "changed", + IsViewed: true, + }, + }, + }, + { + EntryMode: "", + DisplayName: "file1", + FullName: "file1", + NameHash: "60b27f004e454aca81b0480209cce5081ec52390", + DiffStatus: "added", + }, + }, + }, + }, ret) +} diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 9ee86d9dfc..a859945378 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1337,10 +1337,13 @@ func GetDiffShortStat(gitRepo *git.Repository, beforeCommitID, afterCommitID str // SyncUserSpecificDiff inserts user-specific data such as which files the user has already viewed on the given diff // Additionally, the database is updated asynchronously if files have changed since the last review -func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions, files ...string) error { +func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions) (*pull_model.ReviewState, error) { review, err := pull_model.GetNewestReviewState(ctx, userID, pull.ID) - if err != nil || review == nil || review.UpdatedFiles == nil { - return err + if err != nil { + return nil, err + } + if review == nil || len(review.UpdatedFiles) == 0 { + return review, nil } latestCommit := opts.AfterCommitID @@ -1393,11 +1396,11 @@ outer: err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff) if err != nil { log.Warn("Could not update review for user %d, pull %d, commit %s and the changed files %v: %v", review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff, err) - return err + return nil, err } } - return nil + return review, err } // CommentAsDiff returns c.Patch as *Diff diff --git a/web_src/js/components/DiffFileTree.vue b/web_src/js/components/DiffFileTree.vue index 381a1c3ca4..5426a672cb 100644 --- a/web_src/js/components/DiffFileTree.vue +++ b/web_src/js/components/DiffFileTree.vue @@ -1,21 +1,14 @@