Skip to content

Commit 2b99a58

Browse files
Mark parent directory as viewed when all files are viewed (#33958)
Fix #25644 --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent 18a673b commit 2b99a58

File tree

15 files changed

+311
-278
lines changed

15 files changed

+311
-278
lines changed

routers/web/repo/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ func Diff(ctx *context.Context) {
369369
return
370370
}
371371

372-
ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil)
372+
ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, nil)
373373
}
374374

375375
statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)

routers/web/repo/compare.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ func PrepareCompareDiff(
639639
return false
640640
}
641641

642-
ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, nil)
642+
ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, nil)
643643
}
644644

645645
headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID)

routers/web/repo/pull.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -759,12 +759,9 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
759759
// have to load only the diff and not get the viewed information
760760
// as the viewed information is designed to be loaded only on latest PR
761761
// diff and if you're signed in.
762-
shouldGetUserSpecificDiff := false
763-
if !ctx.IsSigned || willShowSpecifiedCommit || willShowSpecifiedCommitRange {
764-
// do nothing
765-
} else {
766-
shouldGetUserSpecificDiff = true
767-
err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions, files...)
762+
var reviewState *pull_model.ReviewState
763+
if ctx.IsSigned && !willShowSpecifiedCommit && !willShowSpecifiedCommitRange {
764+
reviewState, err = gitdiff.SyncUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diff, diffOptions)
768765
if err != nil {
769766
ctx.ServerError("SyncUserSpecificDiff", err)
770767
return
@@ -823,18 +820,11 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
823820
ctx.ServerError("GetDiffTree", err)
824821
return
825822
}
826-
827-
filesViewedState := make(map[string]pull_model.ViewedState)
828-
if shouldGetUserSpecificDiff {
829-
// This sort of sucks because we already fetch this when getting the diff
830-
review, err := pull_model.GetNewestReviewState(ctx, ctx.Doer.ID, issue.ID)
831-
if err == nil && review != nil && review.UpdatedFiles != nil {
832-
// If there wasn't an error and we have a review with updated files, use that
833-
filesViewedState = review.UpdatedFiles
834-
}
823+
var filesViewedState map[string]pull_model.ViewedState
824+
if reviewState != nil {
825+
filesViewedState = reviewState.UpdatedFiles
835826
}
836-
837-
ctx.PageData["DiffFiles"] = transformDiffTreeForUI(diffTree, filesViewedState)
827+
ctx.PageData["DiffFileTree"] = transformDiffTreeForWeb(diffTree, filesViewedState)
838828
}
839829

840830
ctx.Data["Diff"] = diff

routers/web/repo/treelist.go

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package repo
55

66
import (
77
"net/http"
8+
"strings"
89

910
pull_model "code.gitea.io/gitea/models/pull"
1011
"code.gitea.io/gitea/modules/base"
@@ -57,34 +58,85 @@ func isExcludedEntry(entry *git.TreeEntry) bool {
5758
return false
5859
}
5960

60-
type FileDiffFile struct {
61-
Name string
61+
// WebDiffFileItem is used by frontend, check the field names in frontend before changing
62+
type WebDiffFileItem struct {
63+
FullName string
64+
DisplayName string
6265
NameHash string
63-
IsSubmodule bool
66+
DiffStatus string
67+
EntryMode string
6468
IsViewed bool
65-
Status string
69+
Children []*WebDiffFileItem
70+
// TODO: add icon support in the future
6671
}
6772

68-
// transformDiffTreeForUI transforms a DiffTree into a slice of FileDiffFile for UI rendering
73+
// WebDiffFileTree is used by frontend, check the field names in frontend before changing
74+
type WebDiffFileTree struct {
75+
TreeRoot WebDiffFileItem
76+
}
77+
78+
// transformDiffTreeForWeb transforms a gitdiff.DiffTree into a WebDiffFileTree for Web UI rendering
6979
// it also takes a map of file names to their viewed state, which is used to mark files as viewed
70-
func transformDiffTreeForUI(diffTree *gitdiff.DiffTree, filesViewedState map[string]pull_model.ViewedState) []FileDiffFile {
71-
files := make([]FileDiffFile, 0, len(diffTree.Files))
80+
func transformDiffTreeForWeb(diffTree *gitdiff.DiffTree, filesViewedState map[string]pull_model.ViewedState) (dft WebDiffFileTree) {
81+
dirNodes := map[string]*WebDiffFileItem{"": &dft.TreeRoot}
82+
addItem := func(item *WebDiffFileItem) {
83+
var parentPath string
84+
pos := strings.LastIndexByte(item.FullName, '/')
85+
if pos == -1 {
86+
item.DisplayName = item.FullName
87+
} else {
88+
parentPath = item.FullName[:pos]
89+
item.DisplayName = item.FullName[pos+1:]
90+
}
91+
parentNode, parentExists := dirNodes[parentPath]
92+
if !parentExists {
93+
parentNode = &dft.TreeRoot
94+
fields := strings.Split(parentPath, "/")
95+
for idx, field := range fields {
96+
nodePath := strings.Join(fields[:idx+1], "/")
97+
node, ok := dirNodes[nodePath]
98+
if !ok {
99+
node = &WebDiffFileItem{EntryMode: "tree", DisplayName: field, FullName: nodePath}
100+
dirNodes[nodePath] = node
101+
parentNode.Children = append(parentNode.Children, node)
102+
}
103+
parentNode = node
104+
}
105+
}
106+
parentNode.Children = append(parentNode.Children, item)
107+
}
72108

73109
for _, file := range diffTree.Files {
74-
nameHash := git.HashFilePathForWebUI(file.HeadPath)
75-
isSubmodule := file.HeadMode == git.EntryModeCommit
76-
isViewed := filesViewedState[file.HeadPath] == pull_model.Viewed
77-
78-
files = append(files, FileDiffFile{
79-
Name: file.HeadPath,
80-
NameHash: nameHash,
81-
IsSubmodule: isSubmodule,
82-
IsViewed: isViewed,
83-
Status: file.Status,
84-
})
110+
item := &WebDiffFileItem{FullName: file.HeadPath, DiffStatus: file.Status}
111+
item.IsViewed = filesViewedState[item.FullName] == pull_model.Viewed
112+
item.NameHash = git.HashFilePathForWebUI(item.FullName)
113+
114+
switch file.HeadMode {
115+
case git.EntryModeTree:
116+
item.EntryMode = "tree"
117+
case git.EntryModeCommit:
118+
item.EntryMode = "commit" // submodule
119+
default:
120+
// default to empty, and will be treated as "blob" file because there is no "symlink" support yet
121+
}
122+
addItem(item)
85123
}
86124

87-
return files
125+
var mergeSingleDir func(node *WebDiffFileItem)
126+
mergeSingleDir = func(node *WebDiffFileItem) {
127+
if len(node.Children) == 1 {
128+
if child := node.Children[0]; child.EntryMode == "tree" {
129+
node.FullName = child.FullName
130+
node.DisplayName = node.DisplayName + "/" + child.DisplayName
131+
node.Children = child.Children
132+
mergeSingleDir(node)
133+
}
134+
}
135+
}
136+
for _, node := range dft.TreeRoot.Children {
137+
mergeSingleDir(node)
138+
}
139+
return dft
88140
}
89141

90142
func TreeViewNodes(ctx *context.Context) {

routers/web/repo/treelist_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package repo
5+
6+
import (
7+
"testing"
8+
9+
pull_model "code.gitea.io/gitea/models/pull"
10+
"code.gitea.io/gitea/modules/git"
11+
"code.gitea.io/gitea/services/gitdiff"
12+
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func TestTransformDiffTreeForWeb(t *testing.T) {
17+
ret := transformDiffTreeForWeb(&gitdiff.DiffTree{Files: []*gitdiff.DiffTreeRecord{
18+
{
19+
Status: "changed",
20+
HeadPath: "dir-a/dir-a-x/file-deep",
21+
HeadMode: git.EntryModeBlob,
22+
},
23+
{
24+
Status: "added",
25+
HeadPath: "file1",
26+
HeadMode: git.EntryModeBlob,
27+
},
28+
}}, map[string]pull_model.ViewedState{
29+
"dir-a/dir-a-x/file-deep": pull_model.Viewed,
30+
})
31+
32+
assert.Equal(t, WebDiffFileTree{
33+
TreeRoot: WebDiffFileItem{
34+
Children: []*WebDiffFileItem{
35+
{
36+
EntryMode: "tree",
37+
DisplayName: "dir-a/dir-a-x",
38+
FullName: "dir-a/dir-a-x",
39+
Children: []*WebDiffFileItem{
40+
{
41+
EntryMode: "",
42+
DisplayName: "file-deep",
43+
FullName: "dir-a/dir-a-x/file-deep",
44+
NameHash: "4acf7eef1c943a09e9f754e93ff190db8583236b",
45+
DiffStatus: "changed",
46+
IsViewed: true,
47+
},
48+
},
49+
},
50+
{
51+
EntryMode: "",
52+
DisplayName: "file1",
53+
FullName: "file1",
54+
NameHash: "60b27f004e454aca81b0480209cce5081ec52390",
55+
DiffStatus: "added",
56+
},
57+
},
58+
},
59+
}, ret)
60+
}

services/gitdiff/gitdiff.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,10 +1337,13 @@ func GetDiffShortStat(gitRepo *git.Repository, beforeCommitID, afterCommitID str
13371337

13381338
// SyncUserSpecificDiff inserts user-specific data such as which files the user has already viewed on the given diff
13391339
// Additionally, the database is updated asynchronously if files have changed since the last review
1340-
func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions, files ...string) error {
1340+
func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.PullRequest, gitRepo *git.Repository, diff *Diff, opts *DiffOptions) (*pull_model.ReviewState, error) {
13411341
review, err := pull_model.GetNewestReviewState(ctx, userID, pull.ID)
1342-
if err != nil || review == nil || review.UpdatedFiles == nil {
1343-
return err
1342+
if err != nil {
1343+
return nil, err
1344+
}
1345+
if review == nil || len(review.UpdatedFiles) == 0 {
1346+
return review, nil
13441347
}
13451348

13461349
latestCommit := opts.AfterCommitID
@@ -1393,11 +1396,11 @@ outer:
13931396
err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff)
13941397
if err != nil {
13951398
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)
1396-
return err
1399+
return nil, err
13971400
}
13981401
}
13991402

1400-
return nil
1403+
return review, err
14011404
}
14021405

14031406
// CommentAsDiff returns c.Patch as *Diff

web_src/js/components/DiffFileTree.vue

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,14 @@
11
<script lang="ts" setup>
22
import DiffFileTreeItem from './DiffFileTreeItem.vue';
33
import {toggleElem} from '../utils/dom.ts';
4-
import {diffTreeStore} from '../modules/stores.ts';
4+
import {diffTreeStore} from '../modules/diff-file.ts';
55
import {setFileFolding} from '../features/file-fold.ts';
6-
import {computed, onMounted, onUnmounted} from 'vue';
7-
import {pathListToTree, mergeChildIfOnlyOneDir} from '../utils/filetree.ts';
6+
import {onMounted, onUnmounted} from 'vue';
87
98
const LOCAL_STORAGE_KEY = 'diff_file_tree_visible';
109
1110
const store = diffTreeStore();
1211
13-
const fileTree = computed(() => {
14-
const result = pathListToTree(store.files);
15-
mergeChildIfOnlyOneDir(result); // mutation
16-
return result;
17-
});
18-
1912
onMounted(() => {
2013
// Default to true if unset
2114
store.fileTreeIsVisible = localStorage.getItem(LOCAL_STORAGE_KEY) !== 'false';
@@ -50,7 +43,7 @@ function toggleVisibility() {
5043
5144
function updateVisibility(visible: boolean) {
5245
store.fileTreeIsVisible = visible;
53-
localStorage.setItem(LOCAL_STORAGE_KEY, store.fileTreeIsVisible);
46+
localStorage.setItem(LOCAL_STORAGE_KEY, store.fileTreeIsVisible.toString());
5447
updateState(store.fileTreeIsVisible);
5548
}
5649
@@ -69,7 +62,7 @@ function updateState(visible: boolean) {
6962
<template>
7063
<div v-if="store.fileTreeIsVisible" class="diff-file-tree-items">
7164
<!-- only render the tree if we're visible. in many cases this is something that doesn't change very often -->
72-
<DiffFileTreeItem v-for="item in fileTree" :key="item.name" :item="item"/>
65+
<DiffFileTreeItem v-for="item in store.diffFileTree.TreeRoot.Children" :key="item.FullName" :item="item"/>
7366
</div>
7467
</template>
7568

0 commit comments

Comments
 (0)