Skip to content

Commit 8b9f98b

Browse files
committed
Merge remote-tracking branch 'giteaofficial/main'
* giteaofficial/main: Notify the user when the file path contains leading or trailing spaces and fix the error message for invalid file names. (go-gitea#31507) Fix wrong status of `Set up Job` when first step is skipped (go-gitea#32120) Fix bug when deleting a migrated branch (go-gitea#32075) Include collaboration repositories on dashboard source/forks/mirrors list (go-gitea#31946) Display head branch more comfortable on pull request view (go-gitea#32000) Truncate commit message during Discord webhook push events (go-gitea#31970) Fix template bug of pull request view (go-gitea#32072)
2 parents b792231 + 3269b04 commit 8b9f98b

File tree

14 files changed

+159
-41
lines changed

14 files changed

+159
-41
lines changed

models/issues/pull.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@ func (pr *PullRequest) LoadAttributes(ctx context.Context) (err error) {
268268
return nil
269269
}
270270

271+
func (pr *PullRequest) IsAgitFlow() bool {
272+
return pr.Flow == PullRequestFlowAGit
273+
}
274+
271275
// LoadHeadRepo loads the head repository, pr.HeadRepo will remain nil if it does not exist
272276
// and thus ErrRepoNotExist will never be returned
273277
func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) {

modules/actions/task_state.go

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,32 @@ func FullSteps(task *actions_model.ActionTask) []*actions_model.ActionTaskStep {
1818
return fullStepsOfEmptySteps(task)
1919
}
2020

21-
firstStep := task.Steps[0]
21+
// firstStep is the first step that has run or running, not include preStep.
22+
// For example,
23+
// 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): firstStep is step1.
24+
// 2. preStep(Success) -> step1(Skipped) -> step2(Success) -> postStep(Success): firstStep is step2.
25+
// 3. preStep(Success) -> step1(Running) -> step2(Waiting) -> postStep(Waiting): firstStep is step1.
26+
// 4. preStep(Success) -> step1(Skipped) -> step2(Skipped) -> postStep(Skipped): firstStep is nil.
27+
// 5. preStep(Success) -> step1(Cancelled) -> step2(Cancelled) -> postStep(Cancelled): firstStep is nil.
28+
var firstStep *actions_model.ActionTaskStep
29+
// lastHasRunStep is the last step that has run.
30+
// For example,
31+
// 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): lastHasRunStep is step1.
32+
// 2. preStep(Success) -> step1(Success) -> step2(Success) -> step3(Success) -> postStep(Success): lastHasRunStep is step3.
33+
// 3. preStep(Success) -> step1(Success) -> step2(Failure) -> step3 -> postStep(Waiting): lastHasRunStep is step2.
34+
// So its Stopped is the Started of postStep when there are no more steps to run.
35+
var lastHasRunStep *actions_model.ActionTaskStep
36+
2237
var logIndex int64
38+
for _, step := range task.Steps {
39+
if firstStep == nil && (step.Status.HasRun() || step.Status.IsRunning()) {
40+
firstStep = step
41+
}
42+
if step.Status.HasRun() {
43+
lastHasRunStep = step
44+
}
45+
logIndex += step.LogLength
46+
}
2347

2448
preStep := &actions_model.ActionTaskStep{
2549
Name: preStepName,
@@ -28,32 +52,17 @@ func FullSteps(task *actions_model.ActionTask) []*actions_model.ActionTaskStep {
2852
Status: actions_model.StatusRunning,
2953
}
3054

31-
if firstStep.Status.HasRun() || firstStep.Status.IsRunning() {
55+
// No step has run or is running, so preStep is equal to the task
56+
if firstStep == nil {
57+
preStep.Stopped = task.Stopped
58+
preStep.Status = task.Status
59+
} else {
3260
preStep.LogLength = firstStep.LogIndex
3361
preStep.Stopped = firstStep.Started
3462
preStep.Status = actions_model.StatusSuccess
35-
} else if task.Status.IsDone() {
36-
preStep.Stopped = task.Stopped
37-
preStep.Status = actions_model.StatusFailure
38-
if task.Status.IsSkipped() {
39-
preStep.Status = actions_model.StatusSkipped
40-
}
4163
}
4264
logIndex += preStep.LogLength
4365

44-
// lastHasRunStep is the last step that has run.
45-
// For example,
46-
// 1. preStep(Success) -> step1(Success) -> step2(Running) -> step3(Waiting) -> postStep(Waiting): lastHasRunStep is step1.
47-
// 2. preStep(Success) -> step1(Success) -> step2(Success) -> step3(Success) -> postStep(Success): lastHasRunStep is step3.
48-
// 3. preStep(Success) -> step1(Success) -> step2(Failure) -> step3 -> postStep(Waiting): lastHasRunStep is step2.
49-
// So its Stopped is the Started of postStep when there are no more steps to run.
50-
var lastHasRunStep *actions_model.ActionTaskStep
51-
for _, step := range task.Steps {
52-
if step.Status.HasRun() {
53-
lastHasRunStep = step
54-
}
55-
logIndex += step.LogLength
56-
}
5766
if lastHasRunStep == nil {
5867
lastHasRunStep = preStep
5968
}

modules/actions/task_state_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,25 @@ func TestFullSteps(t *testing.T) {
137137
{Name: postStepName, Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
138138
},
139139
},
140+
{
141+
name: "first step is skipped",
142+
task: &actions_model.ActionTask{
143+
Steps: []*actions_model.ActionTaskStep{
144+
{Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
145+
{Status: actions_model.StatusSuccess, LogIndex: 10, LogLength: 80, Started: 10010, Stopped: 10090},
146+
},
147+
Status: actions_model.StatusSuccess,
148+
Started: 10000,
149+
Stopped: 10100,
150+
LogLength: 100,
151+
},
152+
want: []*actions_model.ActionTaskStep{
153+
{Name: preStepName, Status: actions_model.StatusSuccess, LogIndex: 0, LogLength: 10, Started: 10000, Stopped: 10010},
154+
{Status: actions_model.StatusSkipped, LogIndex: 0, LogLength: 0, Started: 0, Stopped: 0},
155+
{Status: actions_model.StatusSuccess, LogIndex: 10, LogLength: 80, Started: 10010, Stopped: 10090},
156+
{Name: postStepName, Status: actions_model.StatusSuccess, LogIndex: 90, LogLength: 10, Started: 10090, Stopped: 10100},
157+
},
158+
},
140159
}
141160
for _, tt := range tests {
142161
t.Run(tt.name, func(t *testing.T) {

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1927,6 +1927,7 @@ pulls.delete.text = Do you really want to delete this pull request? (This will p
19271927
pulls.recently_pushed_new_branches = You pushed on branch <strong>%[1]s</strong> %[2]s
19281928

19291929
pull.deleted_branch = (deleted):%s
1930+
pull.agit_documentation = Review documentation about AGit
19301931

19311932
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
19321933

routers/web/repo/editor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
317317
case git.EntryModeBlob:
318318
ctx.RenderWithErr(ctx.Tr("repo.editor.directory_is_a_file", fileErr.Path), tplEditFile, &form)
319319
default:
320-
ctx.Error(http.StatusInternalServerError, err.Error())
320+
ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_invalid", fileErr.Path), tplEditFile, &form)
321321
}
322322
} else {
323323
ctx.Error(http.StatusInternalServerError, err.Error())

routers/web/repo/pull.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,19 @@ func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) {
164164
ctx.Data["HeadTarget"] = pull.MustHeadUserName(ctx) + "/" + pull.HeadRepo.Name + ":" + pull.HeadBranch
165165
}
166166
ctx.Data["BaseTarget"] = pull.BaseBranch
167-
ctx.Data["HeadBranchLink"] = pull.GetHeadBranchLink(ctx)
167+
headBranchLink := ""
168+
if pull.Flow == issues_model.PullRequestFlowGithub {
169+
b, err := git_model.GetBranch(ctx, ctx.Repo.Repository.ID, pull.HeadBranch)
170+
switch {
171+
case err == nil:
172+
if !b.IsDeleted {
173+
headBranchLink = pull.GetHeadBranchLink(ctx)
174+
}
175+
case !git_model.IsErrBranchNotExist(err):
176+
log.Error("GetBranch: %v", err)
177+
}
178+
}
179+
ctx.Data["HeadBranchLink"] = headBranchLink
168180
ctx.Data["BaseBranchLink"] = pull.GetBaseBranchLink(ctx)
169181
}
170182

services/repository/branch.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -483,22 +483,23 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R
483483
}
484484

485485
rawBranch, err := git_model.GetBranch(ctx, repo.ID, branchName)
486-
if err != nil {
486+
if err != nil && !git_model.IsErrBranchNotExist(err) {
487487
return fmt.Errorf("GetBranch: %vc", err)
488488
}
489489

490-
if rawBranch.IsDeleted {
491-
return nil
492-
}
490+
// database branch record not exist or it's a deleted branch
491+
notExist := git_model.IsErrBranchNotExist(err) || rawBranch.IsDeleted
493492

494493
commit, err := gitRepo.GetBranchCommit(branchName)
495494
if err != nil {
496495
return err
497496
}
498497

499498
if err := db.WithTx(ctx, func(ctx context.Context) error {
500-
if err := git_model.AddDeletedBranch(ctx, repo.ID, branchName, doer.ID); err != nil {
501-
return err
499+
if !notExist {
500+
if err := git_model.AddDeletedBranch(ctx, repo.ID, branchName, doer.ID); err != nil {
501+
return err
502+
}
502503
}
503504

504505
return gitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{

services/webhook/discord.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"net/url"
1212
"strconv"
1313
"strings"
14+
"unicode/utf8"
1415

1516
webhook_model "code.gitea.io/gitea/models/webhook"
1617
"code.gitea.io/gitea/modules/git"
@@ -154,8 +155,14 @@ func (d discordConvertor) Push(p *api.PushPayload) (DiscordPayload, error) {
154155
var text string
155156
// for each commit, generate attachment text
156157
for i, commit := range p.Commits {
157-
text += fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL,
158-
strings.TrimRight(commit.Message, "\r\n"), commit.Author.Name)
158+
// limit the commit message display to just the summary, otherwise it would be hard to read
159+
message := strings.TrimRight(strings.SplitN(commit.Message, "\n", 1)[0], "\r")
160+
161+
// a limit of 50 is set because GitHub does the same
162+
if utf8.RuneCountInString(message) > 50 {
163+
message = fmt.Sprintf("%.47s...", message)
164+
}
165+
text += fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL, message, commit.Author.Name)
159166
// add linebreak to each commit but the last
160167
if i < len(p.Commits)-1 {
161168
text += "\n"

services/webhook/discord_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,20 @@ func TestDiscordPayload(t *testing.T) {
8080
assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL)
8181
})
8282

83+
t.Run("PushWithLongCommitMessage", func(t *testing.T) {
84+
p := pushTestMultilineCommitMessagePayload()
85+
86+
pl, err := dc.Push(p)
87+
require.NoError(t, err)
88+
89+
assert.Len(t, pl.Embeds, 1)
90+
assert.Equal(t, "[test/repo:test] 2 new commits", pl.Embeds[0].Title)
91+
assert.Equal(t, "[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好... - user1\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好... - user1", pl.Embeds[0].Description)
92+
assert.Equal(t, p.Sender.UserName, pl.Embeds[0].Author.Name)
93+
assert.Equal(t, setting.AppURL+p.Sender.UserName, pl.Embeds[0].Author.URL)
94+
assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL)
95+
})
96+
8397
t.Run("Issue", func(t *testing.T) {
8498
p := issueTestPayload()
8599

services/webhook/general_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,17 @@ func forkTestPayload() *api.ForkPayload {
6464
}
6565

6666
func pushTestPayload() *api.PushPayload {
67+
return pushTestPayloadWithCommitMessage("commit message")
68+
}
69+
70+
func pushTestMultilineCommitMessagePayload() *api.PushPayload {
71+
return pushTestPayloadWithCommitMessage("This is a commit summary ⚠️⚠️⚠️⚠️ containing 你好 ⚠️⚠️️\n\nThis is the message body.")
72+
}
73+
74+
func pushTestPayloadWithCommitMessage(message string) *api.PushPayload {
6775
commit := &api.PayloadCommit{
6876
ID: "2020558fe2e34debb818a514715839cabd25e778",
69-
Message: "commit message",
77+
Message: message,
7078
URL: "http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778",
7179
Author: &api.PayloadUser{
7280
Name: "user1",

templates/repo/issue/view_content/sidebar.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@
109109
</a>
110110
</div>
111111
<div class="tw-flex tw-items-center tw-gap-2">
112-
<span {{if .Review.TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .Review.TooltipContent}}"{{end}}>
112+
<span {{if .TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .TooltipContent}}"{{end}}>
113113
{{svg (printf "octicon-%s" .Type.Icon) 16 (printf "text %s" (.HTMLTypeColorName))}}
114114
</span>
115115
</div>

templates/repo/issue/view_title.tmpl

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,14 @@
5050
{{if .Issue.IsPull}}
5151
{{$headHref := .HeadTarget}}
5252
{{if .HeadBranchLink}}
53-
{{$headHref = HTMLFormat `<a href="%s">%s</a>` .HeadBranchLink $headHref}}
53+
{{$headHref = HTMLFormat `<a href="%s">%s</a> <button class="btn interact-fg" data-tooltip-content="%s" data-clipboard-text="%s">%s</button>` .HeadBranchLink $headHref (ctx.Locale.Tr "copy_branch") .HeadTarget (svg "octicon-copy" 14)}}
54+
{{else}}
55+
{{if .Issue.PullRequest.IsAgitFlow}}
56+
{{$headHref = HTMLFormat `%s <a href="%s" target="_blank"><span class="ui label basic tiny" data-tooltip-content="%s">AGit</span></a>` $headHref "https://docs.gitea.com/usage/agit" (ctx.Locale.Tr "repo.pull.agit_documentation")}}
57+
{{else}}
58+
{{$headHref = HTMLFormat `<span data-tooltip-content="%s">%s</span>` (ctx.Locale.Tr "form.target_branch_not_exist") $headHref}}
59+
{{end}}
5460
{{end}}
55-
{{$headHref = HTMLFormat `%s <button class="btn interact-fg" data-tooltip-content="%s" data-clipboard-text="%s">%s</button>` $headHref (ctx.Locale.Tr "copy_branch") .HeadTarget (svg "octicon-copy" 14)}}
5661
{{$baseHref := .BaseTarget}}
5762
{{if .BaseBranchLink}}
5863
{{$baseHref = HTMLFormat `<a href="%s">%s</a>` .BaseBranchLink $baseHref}}

web_src/js/components/DashboardRepoList.vue

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ const sfc = {
7878
searchURL() {
7979
return `${this.subUrl}/repo/search?sort=updated&order=desc&uid=${this.uid}&team_id=${this.teamId}&q=${this.searchQuery
8080
}&page=${this.page}&limit=${this.searchLimit}&mode=${this.repoTypes[this.reposFilter].searchMode
81-
}${this.reposFilter !== 'all' ? '&exclusive=1' : ''
8281
}${this.archivedFilter === 'archived' ? '&archived=true' : ''}${this.archivedFilter === 'unarchived' ? '&archived=false' : ''
8382
}${this.privateFilter === 'private' ? '&is_private=true' : ''}${this.privateFilter === 'public' ? '&is_private=false' : ''
8483
}`;

web_src/js/features/repo-editor.ts

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,62 @@ export function initRepoEditor() {
7575
}
7676
filenameInput.addEventListener('input', function () {
7777
const parts = filenameInput.value.split('/');
78+
const links = Array.from(document.querySelectorAll('.breadcrumb span.section'));
79+
const dividers = Array.from(document.querySelectorAll('.breadcrumb .breadcrumb-divider'));
80+
let warningDiv = document.querySelector('.ui.warning.message.flash-message.flash-warning.space-related');
81+
let containSpace = false;
7882
if (parts.length > 1) {
7983
for (let i = 0; i < parts.length; ++i) {
8084
const value = parts[i];
85+
const trimValue = value.trim();
86+
if (trimValue === '..') {
87+
// remove previous tree path
88+
if (links.length > 0) {
89+
const link = links.pop();
90+
const divider = dividers.pop();
91+
link.remove();
92+
divider.remove();
93+
}
94+
continue;
95+
}
8196
if (i < parts.length - 1) {
82-
if (value.length) {
83-
filenameInput.before(createElementFromHTML(
97+
if (trimValue.length) {
98+
const linkElement = createElementFromHTML(
8499
`<span class="section"><a href="#">${htmlEscape(value)}</a></span>`,
85-
));
86-
filenameInput.before(createElementFromHTML(
100+
);
101+
const dividerElement = createElementFromHTML(
87102
`<div class="breadcrumb-divider">/</div>`,
88-
));
103+
);
104+
links.push(linkElement);
105+
dividers.push(dividerElement);
106+
filenameInput.before(linkElement);
107+
filenameInput.before(dividerElement);
89108
}
90109
} else {
91110
filenameInput.value = value;
92111
}
93112
this.setSelectionRange(0, 0);
113+
containSpace |= (trimValue !== value && trimValue !== '');
114+
}
115+
}
116+
containSpace |= Array.from(links).some((link) => {
117+
const value = link.querySelector('a').textContent;
118+
return value.trim() !== value;
119+
});
120+
containSpace |= parts[parts.length - 1].trim() !== parts[parts.length - 1];
121+
if (containSpace) {
122+
if (!warningDiv) {
123+
warningDiv = document.createElement('div');
124+
warningDiv.classList.add('ui', 'warning', 'message', 'flash-message', 'flash-warning', 'space-related');
125+
warningDiv.innerHTML = '<p>File path contains leading or trailing whitespace.</p>';
126+
// Add display 'block' because display is set to 'none' in formantic\build\semantic.css
127+
warningDiv.style.display = 'block';
128+
const inputContainer = document.querySelector('.repo-editor-header');
129+
inputContainer.insertAdjacentElement('beforebegin', warningDiv);
94130
}
131+
showElem(warningDiv);
132+
} else if (warningDiv) {
133+
hideElem(warningDiv);
95134
}
96135
joinTreePath();
97136
});

0 commit comments

Comments
 (0)