From 1879ce8efeac1d60ee490d571674107b85f6d610 Mon Sep 17 00:00:00 2001 From: Robert Wolff Date: Tue, 17 Jun 2025 23:33:56 +0200 Subject: [PATCH 01/21] fix(ui): issue comment anchor on time stamp --- options/locale/locale_en-US.ini | 22 +++++++++---------- .../repo/issue/view_content/comments.tmpl | 17 +++++++------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 7479ab80af..057edda3a5 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1693,15 +1693,13 @@ issues.close_comment_issue = Close with comment issues.reopen_issue = Reopen issues.reopen_comment_issue = Reopen with comment issues.create_comment = Comment -issues.closed_at = `closed this issue %[2]s` -issues.reopened_at = `reopened this issue %[2]s` -issues.commit_ref_at = `referenced this issue from a commit %[2]s` -issues.ref_issue_from = `referenced this issue %[4]s %[2]s` -issues.ref_pull_from = `referenced this pull request %[4]s %[2]s` -issues.ref_closing_from = `referenced this issue from a pull request %[4]s that will close it, %[2]s` -issues.ref_reopening_from = `referenced this issue from a pull request %[4]s that will reopen it, %[2]s` -issues.ref_closed_from = `closed this issue %[4]s %[2]s` -issues.ref_reopened_from = `reopened this issue %[4]s %[2]s` +issues.closed_at = `closed this issue %s` +issues.reopened_at = `reopened this issue %s` +issues.commit_ref_at = `referenced this issue from a commit %s` +issues.ref_issue_from = `referenced this issue %[3]s %[1]s` +issues.ref_pull_from = `referenced this pull request %[3]s %[1]s` +issues.ref_closing_from = `referenced this issue from a pull request %[3]s that will close it, %[1]s` +issues.ref_reopening_from = `referenced this issue from a pull request %[3]s that will reopen it, %[1]s` issues.ref_from = `from %[1]s` issues.author = Author issues.author.tooltip.issue = This user is the author of this issue. @@ -2013,9 +2011,9 @@ pulls.update_branch_success = Branch update was successful pulls.update_not_allowed = You are not allowed to update branch pulls.outdated_with_base_branch = This branch is out-of-date with the base branch pulls.close = Close pull request -pulls.closed_at = `closed this pull request %[2]s` -pulls.reopened_at = `reopened this pull request %[2]s` -pulls.commit_ref_at = `referenced this pull request from a commit %[2]s` +pulls.closed_at = `closed this pull request %s` +pulls.reopened_at = `reopened this pull request %s` +pulls.commit_ref_at = `referenced this pull request from a commit %s` pulls.cmd_instruction_hint = View command line instructions pulls.cmd_instruction_checkout_title = Checkout pulls.cmd_instruction_checkout_desc = From your project repository, check out a new branch and test the changes. diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 9eb9307f9f..aca77a92e5 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -1,7 +1,7 @@ {{template "base/alert"}} {{range .Issue.Comments}} {{if call $.ShouldShowCommentType .Type}} - {{$createdStr:= DateUtils.TimeSince .CreatedUnix}} + {{$createdStr := HTMLFormat `%s` .EventTag .HashTag (DateUtils.TimeSince .CreatedUnix)}} If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8248 Reviewed-by: Earl Warren Co-authored-by: Renovate Bot Co-committed-by: Renovate Bot --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 87755b206a..bb2be827eb 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/gliderlabs/ssh v0.3.8 github.com/go-ap/activitypub v0.0.0-20231114162308-e219254dc5c9 github.com/go-ap/jsonld v0.0.0-20221030091449-f2a191312c73 - github.com/go-chi/chi/v5 v5.2.1 + github.com/go-chi/chi/v5 v5.2.2 github.com/go-chi/cors v1.2.1 github.com/go-co-op/gocron v1.37.0 github.com/go-enry/go-enry/v2 v2.9.2 diff --git a/go.sum b/go.sum index 54710930e8..639880e2ce 100644 --- a/go.sum +++ b/go.sum @@ -213,8 +213,8 @@ github.com/go-ap/jsonld v0.0.0-20221030091449-f2a191312c73/go.mod h1:jyveZeGw5La github.com/go-asn1-ber/asn1-ber v1.5.5 h1:MNHlNMBDgEKD4TcKr36vQN68BA00aDfjIt3/bD50WnA= github.com/go-asn1-ber/asn1-ber v1.5.5/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= github.com/go-chi/chi/v5 v5.0.1/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= -github.com/go-chi/chi/v5 v5.2.1 h1:KOIHODQj58PmL80G2Eak4WdvUzjSJSm0vG72crDCqb8= -github.com/go-chi/chi/v5 v5.2.1/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= +github.com/go-chi/chi/v5 v5.2.2 h1:CMwsvRVTbXVytCk1Wd72Zy1LAsAh9GxMmSNWLHCG618= +github.com/go-chi/chi/v5 v5.2.2/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= github.com/go-chi/cors v1.2.1 h1:xEC8UT3Rlp2QuWNEr4Fs/c2EAGVKBwy/1vHx3bppil4= github.com/go-chi/cors v1.2.1/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= github.com/go-co-op/gocron v1.37.0 h1:ZYDJGtQ4OMhTLKOKMIch+/CY70Brbb1dGdooLEhh7b0= From 1c0e9d8015309d61359e7666e1468c7f5fb4aa64 Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Sat, 21 Jun 2025 11:50:39 +0200 Subject: [PATCH 11/21] chore(ci): downgrade playwright temporarily and allow running all e2e tests (#8245) In https://codeberg.org/forgejo/forgejo/pulls/7906#issuecomment-5511884, I noticed that the e2e tests were failing without obvious reasons. I was able to reproduce locally with Mobile Chrome, but the error doesn't make sense to me. So I tried to downgrade playwright to the previous version, and it works fine. I actually suspect a bug in playwright, but I currently lack the capacity to reach out to upstream with a reproducer (I also found conversation with the playwright team a little difficult, unless you have absolutely convincing arguments that the flakiness you observe is really their fault, which is very hard to prove). Tests pass with this version of playwright. In order to detect such cases earlier, I added a way to run all playwright tests (which I thought I had added with the changed files patch, but apparently forgot). All tests are triggered by an explicit label (but only after firing a next event, because the testing workflows don't listen to label changes) and when the PR title contains "playwright", which should cover at least renovate dependency updates of playwright and the axe framework (both contain playwright). Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8245 Reviewed-by: Earl Warren Reviewed-by: Michael Kriese Reviewed-by: Beowulf Co-authored-by: Otto Richter Co-committed-by: Otto Richter --- .forgejo/workflows/testing.yml | 6 ++++++ package-lock.json | 24 ++++++++++++------------ package.json | 2 +- tests/e2e/changes.go | 9 +++++++-- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index 86e74591f1..7a93bb66a8 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -115,6 +115,11 @@ jobs: run: | su forgejo -c 'make deps-frontend frontend' - uses: ./.forgejo/workflows-composite/build-backend + - name: Decide to run all tests + id: run-all + if: contains(github.event.pull_request.labels.*.name, 'run-all-playwright-tests') || contains(github.event.pull_request.title, 'playwright') + run: | + echo "all=1" >> "$GITHUB_OUTPUT" - name: Get changed files id: changed-files uses: https://data.forgejo.org/tj-actions/changed-files@v46 @@ -127,6 +132,7 @@ jobs: USE_REPO_TEST_DIR: 1 PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1 CHANGED_FILES: ${{steps.changed-files.outputs.all_changed_files}} + RUN_ALL: ${{steps.run-all.all}} - name: Upload test artifacts on failure if: failure() uses: https://data.forgejo.org/forgejo/upload-artifact@v4 diff --git a/package-lock.json b/package-lock.json index 179c0e496c..1637629a83 100644 --- a/package-lock.json +++ b/package-lock.json @@ -62,7 +62,7 @@ "devDependencies": { "@axe-core/playwright": "4.10.2", "@eslint-community/eslint-plugin-eslint-comments": "4.5.0", - "@playwright/test": "1.53.0", + "@playwright/test": "1.52.0", "@stoplight/spectral-cli": "6.15.0", "@stylistic/eslint-plugin": "4.4.1", "@stylistic/stylelint-plugin": "3.1.2", @@ -2143,13 +2143,13 @@ } }, "node_modules/@playwright/test": { - "version": "1.53.0", - "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.53.0.tgz", - "integrity": "sha512-15hjKreZDcp7t6TL/7jkAo6Df5STZN09jGiv5dbP9A6vMVncXRqE7/B2SncsyOwrkZRBH2i6/TPOL8BVmm3c7w==", + "version": "1.52.0", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.52.0.tgz", + "integrity": "sha512-uh6W7sb55hl7D6vsAeA+V2p5JnlAqzhqFyF0VcJkKZXkgnFcVG9PziERRHQfPLfNGx1C292a4JqbWzhR8L4R1g==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright": "1.53.0" + "playwright": "1.52.0" }, "bin": { "playwright": "cli.js" @@ -11859,13 +11859,13 @@ } }, "node_modules/playwright": { - "version": "1.53.0", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.53.0.tgz", - "integrity": "sha512-ghGNnIEYZC4E+YtclRn4/p6oYbdPiASELBIYkBXfaTVKreQUYbMUYQDwS12a8F0/HtIjr/CkGjtwABeFPGcS4Q==", + "version": "1.52.0", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.52.0.tgz", + "integrity": "sha512-JAwMNMBlxJ2oD1kce4KPtMkDeKGHQstdpFPcPH3maElAXon/QZeTvtsfXmTMRyO9TslfoYOXkSsvao2nE1ilTw==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright-core": "1.53.0" + "playwright-core": "1.52.0" }, "bin": { "playwright": "cli.js" @@ -11878,9 +11878,9 @@ } }, "node_modules/playwright-core": { - "version": "1.53.0", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.53.0.tgz", - "integrity": "sha512-mGLg8m0pm4+mmtB7M89Xw/GSqoNC+twivl8ITteqvAndachozYe2ZA7srU6uleV1vEdAHYqjq+SV8SNxRRFYBw==", + "version": "1.52.0", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.52.0.tgz", + "integrity": "sha512-l2osTgLXSMeuLZOML9qYODUQoPPnUsKsb5/P6LJ2e6uPKXUdPK5WYhN4z03G+YNbWmGDY4YENauNu4ZKczreHg==", "dev": true, "license": "Apache-2.0", "bin": { diff --git a/package.json b/package.json index 409edecd95..d7c3a5f79f 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "devDependencies": { "@axe-core/playwright": "4.10.2", "@eslint-community/eslint-plugin-eslint-comments": "4.5.0", - "@playwright/test": "1.53.0", + "@playwright/test": "1.52.0", "@stoplight/spectral-cli": "6.15.0", "@stylistic/eslint-plugin": "4.4.1", "@stylistic/stylelint-plugin": "3.1.2", diff --git a/tests/e2e/changes.go b/tests/e2e/changes.go index d1d318fd06..5e9238dfa7 100644 --- a/tests/e2e/changes.go +++ b/tests/e2e/changes.go @@ -16,10 +16,15 @@ import ( var ( changesetFiles []string changesetAvailable bool - globalFullRun bool + globalFullRun = false ) func initChangedFiles() { + _, globalFullRun = os.LookupEnv("RUN_ALL") + if globalFullRun { + log.Info("Full run of all tests requested via RUN_ALL environment.") + return + } var changes string changes, changesetAvailable = os.LookupEnv("CHANGED_FILES") // the output of the Action seems to actually contain \n and not a newline literal @@ -44,7 +49,7 @@ func initChangedFiles() { for _, expr := range globalPatterns { fullRunPatterns = append(fullRunPatterns, glob.MustCompile(expr, '.', '/')) } - globalFullRun = false + for _, changedFile := range changesetFiles { for _, pattern := range fullRunPatterns { if pattern.Match(changedFile) { From 25d596d387eabcbec4eeb53a7f0312b20370b2b0 Mon Sep 17 00:00:00 2001 From: Michael Jerger Date: Sat, 21 Jun 2025 12:02:58 +0200 Subject: [PATCH 12/21] Federated user activity following: Isolated model changes (#8078) This PR is part of https://codeberg.org/forgejo/forgejo/pulls/4767 This should not have an outside impact but bring all model changes needed & bring migrations. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8078 Reviewed-by: Earl Warren Co-authored-by: Michael Jerger Co-committed-by: Michael Jerger --- .deadcode-out | 15 ++ models/activities/action.go | 37 +++-- models/activities/action_test.go | 3 +- models/activities/federated_user_activity.go | 106 ++++++++++++++ .../federated_user_activity_test.go | 24 ++++ models/forgefed/federationhost.go | 12 +- models/forgefed/nodeinfo.go | 8 +- models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v33.go | 126 +++++++++++++++++ models/forgejo_migrations/v33_test.go | 46 ++++++ models/user/federated_user.go | 17 ++- models/user/federated_user_follower.go | 30 ++++ models/user/federated_user_follower_test.go | 27 ++++ models/user/federated_user_test.go | 2 + models/user/follow.go | 1 + models/user/user_repository.go | 132 +++++++++++++++++- models/user/user_test.go | 2 +- services/federation/federation_service.go | 6 + services/feed/action.go | 56 +++++--- 19 files changed, 604 insertions(+), 48 deletions(-) create mode 100644 models/activities/federated_user_activity.go create mode 100644 models/activities/federated_user_activity_test.go create mode 100644 models/forgejo_migrations/v33.go create mode 100644 models/forgejo_migrations/v33_test.go create mode 100644 models/user/federated_user_follower.go create mode 100644 models/user/federated_user_follower_test.go diff --git a/.deadcode-out b/.deadcode-out index e63e4a3dc3..61c5bcb055 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -13,6 +13,13 @@ forgejo.org/models IsErrSHANotFound IsErrMergeDivergingFastForwardOnly +forgejo.org/models/activities + GetActivityByID + NewFederatedUserActivity + CreateUserActivity + GetFollowingFeeds + FederatedUserActivity.loadActor + forgejo.org/models/auth WebAuthnCredentials @@ -54,9 +61,17 @@ forgejo.org/models/user IsErrExternalLoginUserAlreadyExist IsErrExternalLoginUserNotExist NewFederatedUser + NewFederatedUserFollower IsErrUserSettingIsNotExist GetUserAllSettings DeleteUserSetting + GetFederatedUser + GetFederatedUserByUserID + UpdateFederatedUser + GetFollowersForUser + AddFollower + RemoveFollower + IsFollowingAp forgejo.org/modules/activitypub NewContext diff --git a/models/activities/action.go b/models/activities/action.go index 1e40546b97..8592f81414 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -442,6 +442,12 @@ func (a *Action) GetIssueContent(ctx context.Context) string { return a.Issue.Content } +func GetActivityByID(ctx context.Context, id int64) (*Action, error) { + var act Action + _, err := db.GetEngine(ctx).ID(id).Get(&act) + return &act, err +} + // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { db.ListOptions @@ -595,13 +601,14 @@ func DeleteOldActions(ctx context.Context, olderThan time.Duration) (err error) } // NotifyWatchers creates batch of actions for every watcher. -func NotifyWatchers(ctx context.Context, actions ...*Action) error { +func NotifyWatchers(ctx context.Context, actions ...*Action) ([]Action, error) { var watchers []*repo_model.Watch var repo *repo_model.Repository var err error var permCode []bool var permIssue []bool var permPR []bool + var out []Action e := db.GetEngine(ctx) @@ -612,14 +619,14 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { // Add feeds for user self and all watchers. watchers, err = repo_model.GetWatchers(ctx, act.RepoID) if err != nil { - return fmt.Errorf("get watchers: %w", err) + return nil, fmt.Errorf("get watchers: %w", err) } // Be aware that optimizing this correctly into the `GetWatchers` SQL // query is for most cases less performant than doing this. blockedDoerUserIDs, err := user_model.ListBlockedByUsersID(ctx, act.ActUserID) if err != nil { - return fmt.Errorf("user_model.ListBlockedByUsersID: %w", err) + return nil, fmt.Errorf("user_model.ListBlockedByUsersID: %w", err) } if len(blockedDoerUserIDs) > 0 { @@ -634,8 +641,9 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { // Add feed for actioner. act.UserID = act.ActUserID if _, err = e.Insert(act); err != nil { - return fmt.Errorf("insert new actioner: %w", err) + return nil, fmt.Errorf("insert new actioner: %w", err) } + out = append(out, *act) if repoChanged { act.loadRepo(ctx) @@ -643,7 +651,7 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { // check repo owner exist. if err := act.Repo.LoadOwner(ctx); err != nil { - return fmt.Errorf("can't get repo owner: %w", err) + return nil, fmt.Errorf("can't get repo owner: %w", err) } } else if act.Repo == nil { act.Repo = repo @@ -654,7 +662,7 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { act.ID = 0 act.UserID = act.Repo.Owner.ID if err = db.Insert(ctx, act); err != nil { - return fmt.Errorf("insert new actioner: %w", err) + return nil, fmt.Errorf("insert new actioner: %w", err) } } @@ -707,26 +715,29 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { } if err = db.Insert(ctx, act); err != nil { - return fmt.Errorf("insert new action: %w", err) + return nil, fmt.Errorf("insert new action: %w", err) } } } - return nil + return out, nil } // NotifyWatchersActions creates batch of actions for every watcher. -func NotifyWatchersActions(ctx context.Context, acts []*Action) error { +func NotifyWatchersActions(ctx context.Context, acts []*Action) ([]Action, error) { ctx, committer, err := db.TxContext(ctx) if err != nil { - return err + return nil, err } defer committer.Close() + var out []Action for _, act := range acts { - if err := NotifyWatchers(ctx, act); err != nil { - return err + as, err := NotifyWatchers(ctx, act) + if err != nil { + return nil, err } + out = append(out, as...) } - return committer.Commit() + return out, committer.Commit() } // DeleteIssueActions delete all actions related with issueID diff --git a/models/activities/action_test.go b/models/activities/action_test.go index bcc9c98cec..47dbd8ac2d 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -197,7 +197,8 @@ func TestNotifyWatchers(t *testing.T) { RepoID: 1, OpType: activities_model.ActionStarRepo, } - require.NoError(t, activities_model.NotifyWatchers(db.DefaultContext, action)) + _, err := activities_model.NotifyWatchers(db.DefaultContext, action) + require.NoError(t, err) // One watchers are inactive, thus action is only created for user 8, 1, 4, 11 unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ diff --git a/models/activities/federated_user_activity.go b/models/activities/federated_user_activity.go new file mode 100644 index 0000000000..1ff3a855d0 --- /dev/null +++ b/models/activities/federated_user_activity.go @@ -0,0 +1,106 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package activities + +import ( + "context" + "fmt" + + "forgejo.org/models/db" + user_model "forgejo.org/models/user" + "forgejo.org/modules/json" + "forgejo.org/modules/log" + "forgejo.org/modules/timeutil" + "forgejo.org/modules/validation" + + ap "github.com/go-ap/activitypub" +) + +type FederatedUserActivity struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL"` + ActorID int64 + ActorURI string + Actor *user_model.User `xorm:"-"` // transient + NoteContent string `xorm:"TEXT"` + NoteURL string `xorm:"VARCHAR(255)"` + OriginalNote string `xorm:"TEXT"` + Created timeutil.TimeStamp `xorm:"created"` +} + +func init() { + db.RegisterModel(new(FederatedUserActivity)) +} + +func NewFederatedUserActivity(userID, actorID int64, actorURI, noteContent, noteURL string, originalNote ap.Activity) (FederatedUserActivity, error) { + jsonString, err := json.Marshal(originalNote) + if err != nil { + return FederatedUserActivity{}, err + } + result := FederatedUserActivity{ + UserID: userID, + ActorID: actorID, + ActorURI: actorURI, + NoteContent: noteContent, + NoteURL: noteURL, + OriginalNote: string(jsonString), + } + if valid, err := validation.IsValid(result); !valid { + return FederatedUserActivity{}, err + } + return result, nil +} + +func (federatedUserActivity FederatedUserActivity) Validate() []string { + var result []string + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.UserID, "UserID")...) + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.ActorID, "ActorID")...) + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.ActorURI, "ActorURI")...) + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.NoteContent, "NoteContent")...) + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.NoteURL, "NoteURL")...) + result = append(result, validation.ValidateNotEmpty(federatedUserActivity.OriginalNote, "OriginalNote")...) + return result +} + +func CreateUserActivity(ctx context.Context, federatedUserActivity *FederatedUserActivity) error { + if valid, err := validation.IsValid(federatedUserActivity); !valid { + return err + } + _, err := db.GetEngine(ctx).Insert(federatedUserActivity) + return err +} + +type GetFollowingFeedsOptions struct { + db.ListOptions +} + +func GetFollowingFeeds(ctx context.Context, actorID int64, opts GetFollowingFeedsOptions) ([]*FederatedUserActivity, int64, error) { + log.Debug("user_id = %s", actorID) + sess := db.GetEngine(ctx).Where("user_id = ?", actorID) + opts.SetDefaultValues() + sess = db.SetSessionPagination(sess, &opts) + + actions := make([]*FederatedUserActivity, 0, opts.PageSize) + count, err := sess.FindAndCount(&actions) + if err != nil { + return nil, 0, fmt.Errorf("FindAndCount: %w", err) + } + for _, act := range actions { + if err := act.loadActor(ctx); err != nil { + return nil, 0, err + } + } + return actions, count, err +} + +func (federatedUserActivity *FederatedUserActivity) loadActor(ctx context.Context) error { + log.Debug("for activity %s", federatedUserActivity) + actorUser, _, err := user_model.GetFederatedUserByUserID(ctx, federatedUserActivity.ActorID) + if err != nil { + return err + } + federatedUserActivity.Actor = actorUser + + return nil +} diff --git a/models/activities/federated_user_activity_test.go b/models/activities/federated_user_activity_test.go new file mode 100644 index 0000000000..9bf4f77984 --- /dev/null +++ b/models/activities/federated_user_activity_test.go @@ -0,0 +1,24 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package activities + +import ( + "testing" + + "forgejo.org/modules/validation" +) + +func Test_FederatedUserActivityValidation(t *testing.T) { + sut := FederatedUserActivity{} + sut.UserID = 13 + sut.ActorID = 33 + sut.ActorURI = "33" + sut.NoteContent = "Any content!" + sut.NoteURL = "https://example.org/note/17" + sut.OriginalNote = "federatedUserActivityNote-17" + + if res, _ := validation.IsValid(sut); !res { + t.Errorf("sut expected to be valid: %v\n", sut.Validate()) + } +} diff --git a/models/forgefed/federationhost.go b/models/forgefed/federationhost.go index 29f1b7d28e..978847bd95 100644 --- a/models/forgefed/federationhost.go +++ b/models/forgefed/federationhost.go @@ -6,6 +6,7 @@ package forgefed import ( "database/sql" "fmt" + "net/url" "strings" "time" @@ -17,9 +18,9 @@ import ( // swagger:model type FederationHost struct { ID int64 `xorm:"pk autoincr"` - HostFqdn string `xorm:"host_fqdn UNIQUE INDEX VARCHAR(255) NOT NULL"` + HostFqdn string `xorm:"host_fqdn UNIQUE(federation_host) INDEX VARCHAR(255) NOT NULL"` + HostPort uint16 `xorm:" UNIQUE(federation_host) INDEX NOT NULL DEFAULT 443"` NodeInfo NodeInfo `xorm:"extends NOT NULL"` - HostPort uint16 `xorm:"NOT NULL DEFAULT 443"` HostSchema string `xorm:"NOT NULL DEFAULT 'https'"` LatestActivity time.Time `xorm:"NOT NULL"` KeyID sql.NullString `xorm:"key_id UNIQUE"` @@ -42,6 +43,13 @@ func NewFederationHost(hostFqdn string, nodeInfo NodeInfo, port uint16, schema s return result, nil } +func (host FederationHost) AsURL() url.URL { + return url.URL{ + Scheme: host.HostSchema, + Host: fmt.Sprintf("%v:%v", host.HostFqdn, host.HostPort), + } +} + // Validate collects error strings in a slice and returns this func (host FederationHost) Validate() []string { var result []string diff --git a/models/forgefed/nodeinfo.go b/models/forgefed/nodeinfo.go index 2461b5e499..38f51304c5 100644 --- a/models/forgefed/nodeinfo.go +++ b/models/forgefed/nodeinfo.go @@ -17,12 +17,14 @@ type ( ) const ( - ForgejoSourceType SoftwareNameType = "forgejo" - GiteaSourceType SoftwareNameType = "gitea" + ForgejoSourceType SoftwareNameType = "forgejo" + GiteaSourceType SoftwareNameType = "gitea" + MastodonSourceType SoftwareNameType = "mastodon" + GoToSocialSourceType SoftwareNameType = "gotosocial" ) var KnownSourceTypes = []any{ - ForgejoSourceType, GiteaSourceType, + ForgejoSourceType, GiteaSourceType, MastodonSourceType, GoToSocialSourceType, } // ------------------------------------------------ NodeInfoWellKnown ------------------------------------------------ diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 21a2077d06..684ef2ed0e 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -103,6 +103,8 @@ var migrations = []*Migration{ NewMigration("Normalize repository.topics to empty slice instead of null", SetTopicsAsEmptySlice), // v31 -> v32 NewMigration("Migrate maven package name concatenation", ChangeMavenArtifactConcatenation), + // v32 -> v33 + NewMigration("Add federated user activity tables, update the `federated_user` table & add indexes", FederatedUserActivityMigration), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v33.go b/models/forgejo_migrations/v33.go new file mode 100644 index 0000000000..272035fc23 --- /dev/null +++ b/models/forgejo_migrations/v33.go @@ -0,0 +1,126 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package forgejo_migrations //nolint:revive + +import ( + "fmt" + + "forgejo.org/modules/log" + "forgejo.org/modules/timeutil" + + "xorm.io/xorm" +) + +func dropOldFederationHostIndexes(x *xorm.Engine) { + // drop unique index on HostFqdn + type FederationHost struct { + ID int64 `xorm:"pk autoincr"` + HostFqdn string `xorm:"host_fqdn UNIQUE INDEX VARCHAR(255) NOT NULL"` + } + + err := x.DropIndexes(FederationHost{}) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } +} + +func addFederatedUserActivityTables(x *xorm.Engine) { + type FederatedUserActivity struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL INDEX user_id"` + ActorID int64 + ActorURI string + NoteContent string `xorm:"TEXT"` + NoteURL string `xorm:"VARCHAR(255)"` + OriginalNote string `xorm:"TEXT"` + Created timeutil.TimeStamp `xorm:"created"` + } + + // add unique index on HostFqdn+HostPort + type FederationHost struct { + ID int64 `xorm:"pk autoincr"` + HostFqdn string `xorm:"host_fqdn UNIQUE(federation_host) INDEX VARCHAR(255) NOT NULL"` + HostPort uint16 `xorm:"UNIQUE(federation_host) INDEX NOT NULL DEFAULT 443"` + } + + type FederatedUserFollower struct { + ID int64 `xorm:"pk autoincr"` + + FollowedUserID int64 `xorm:"NOT NULL unique(fuf_rel)"` + FollowingUserID int64 `xorm:"NOT NULL unique(fuf_rel)"` + } + + // Add InboxPath to FederatedUser & add index fo UserID + type FederatedUser struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL INDEX user_id"` + InboxPath string + } + + var err error + + err = x.Sync(&FederationHost{}) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + + err = x.Sync(&FederatedUserActivity{}) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + + err = x.Sync(&FederatedUserFollower{}) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + + err = x.Sync(&FederatedUser{}) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + + // Migrate + sessMigration := x.NewSession() + defer sessMigration.Close() + if err := sessMigration.Begin(); err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + federatedUsers := make([]*FederatedUser, 0) + err = sessMigration.OrderBy("id").Find(&federatedUsers) + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + + for _, federatedUser := range federatedUsers { + if federatedUser.InboxPath != "" { + log.Info("migration[33]: This user was already migrated: %v", federatedUser) + } else { + // Migrate User.InboxPath + sql := "UPDATE `federated_user` SET `inbox_path` = ? WHERE `id` = ?" + if _, err := sessMigration.Exec(sql, fmt.Sprintf("/api/v1/activitypub/user-id/%v/inbox", federatedUser.UserID), federatedUser.ID); err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } + } + } + + err = sessMigration.Commit() + if err != nil { + log.Warn("migration[33]: There was an issue: %v", err) + return + } +} + +func FederatedUserActivityMigration(x *xorm.Engine) error { + dropOldFederationHostIndexes(x) + addFederatedUserActivityTables(x) + return nil +} diff --git a/models/forgejo_migrations/v33_test.go b/models/forgejo_migrations/v33_test.go new file mode 100644 index 0000000000..664c704bbc --- /dev/null +++ b/models/forgejo_migrations/v33_test.go @@ -0,0 +1,46 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations //nolint:revive + +import ( + "testing" + "time" + + migration_tests "forgejo.org/models/migrations/test" + "forgejo.org/modules/log" + ft "forgejo.org/modules/test" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_FederatedUserActivityMigration(t *testing.T) { + lc, cl := ft.NewLogChecker(log.DEFAULT, log.WARN) + lc.Filter("migration[33]") + defer cl() + + // intentionally conflicting definition + type FederatedUser struct { + ID int64 `xorm:"pk autoincr"` + UserID string + } + + // Prepare TestEnv + x, deferable := migration_tests.PrepareTestEnv(t, 0, + new(FederatedUser), + ) + sessTest := x.NewSession() + sessTest.Insert(FederatedUser{UserID: "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" + + "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"}) + sessTest.Commit() + defer deferable() + if x == nil || t.Failed() { + return + } + + require.NoError(t, FederatedUserActivityMigration(x)) + logFiltered, _ := lc.Check(5 * time.Second) + assert.NotEmpty(t, logFiltered) +} diff --git a/models/user/federated_user.go b/models/user/federated_user.go index c1833c7de3..d2a9c34c9e 100644 --- a/models/user/federated_user.go +++ b/models/user/federated_user.go @@ -11,19 +11,21 @@ import ( type FederatedUser struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"NOT NULL"` + UserID int64 `xorm:"NOT NULL INDEX user_id"` ExternalID string `xorm:"UNIQUE(federation_user_mapping) NOT NULL"` FederationHostID int64 `xorm:"UNIQUE(federation_user_mapping) NOT NULL"` KeyID sql.NullString `xorm:"key_id UNIQUE"` PublicKey sql.Null[sql.RawBytes] `xorm:"BLOB"` - NormalizedOriginalURL string // This field is just to keep original information. Pls. do not use for search or as ID! + InboxPath string + NormalizedOriginalURL string // This field is just to keep original information. Pls. do not use for search or as ID! } -func NewFederatedUser(userID int64, externalID string, federationHostID int64, normalizedOriginalURL string) (FederatedUser, error) { +func NewFederatedUser(userID int64, externalID string, federationHostID int64, inboxPath, normalizedOriginalURL string) (FederatedUser, error) { result := FederatedUser{ UserID: userID, ExternalID: externalID, FederationHostID: federationHostID, + InboxPath: inboxPath, NormalizedOriginalURL: normalizedOriginalURL, } if valid, err := validation.IsValid(result); !valid { @@ -32,10 +34,11 @@ func NewFederatedUser(userID int64, externalID string, federationHostID int64, n return result, nil } -func (user FederatedUser) Validate() []string { +func (federatedUser FederatedUser) Validate() []string { var result []string - result = append(result, validation.ValidateNotEmpty(user.UserID, "UserID")...) - result = append(result, validation.ValidateNotEmpty(user.ExternalID, "ExternalID")...) - result = append(result, validation.ValidateNotEmpty(user.FederationHostID, "FederationHostID")...) + result = append(result, validation.ValidateNotEmpty(federatedUser.UserID, "UserID")...) + result = append(result, validation.ValidateNotEmpty(federatedUser.ExternalID, "ExternalID")...) + result = append(result, validation.ValidateNotEmpty(federatedUser.FederationHostID, "FederationHostID")...) + result = append(result, validation.ValidateNotEmpty(federatedUser.InboxPath, "InboxPath")...) return result } diff --git a/models/user/federated_user_follower.go b/models/user/federated_user_follower.go new file mode 100644 index 0000000000..db72c9b5ce --- /dev/null +++ b/models/user/federated_user_follower.go @@ -0,0 +1,30 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import "forgejo.org/modules/validation" + +type FederatedUserFollower struct { + ID int64 `xorm:"pk autoincr"` + FollowedUserID int64 `xorm:"NOT NULL unique(fuf_rel)"` + FollowingUserID int64 `xorm:"NOT NULL unique(fuf_rel)"` +} + +func NewFederatedUserFollower(followedUserID, federatedUserID int64) (FederatedUserFollower, error) { + result := FederatedUserFollower{ + FollowedUserID: followedUserID, + FollowingUserID: federatedUserID, + } + if valid, err := validation.IsValid(result); !valid { + return FederatedUserFollower{}, err + } + return result, nil +} + +func (user FederatedUserFollower) Validate() []string { + var result []string + result = append(result, validation.ValidateNotEmpty(user.FollowedUserID, "FollowedUserID")...) + result = append(result, validation.ValidateNotEmpty(user.FollowingUserID, "FollowingUserID")...) + return result +} diff --git a/models/user/federated_user_follower_test.go b/models/user/federated_user_follower_test.go new file mode 100644 index 0000000000..e57ba01308 --- /dev/null +++ b/models/user/federated_user_follower_test.go @@ -0,0 +1,27 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "testing" + + "forgejo.org/modules/validation" + + "github.com/stretchr/testify/assert" +) + +func Test_FederatedUserFollowerValidation(t *testing.T) { + sut := FederatedUserFollower{ + FollowedUserID: 12, + FollowingUserID: 1, + } + res, err := validation.IsValid(sut) + assert.Truef(t, res, "sut should be valid but was %q", err) + + sut = FederatedUserFollower{ + FollowedUserID: 1, + } + res, _ = validation.IsValid(sut) + assert.False(t, res, "sut should be invalid") +} diff --git a/models/user/federated_user_test.go b/models/user/federated_user_test.go index 542798c9bc..be18339670 100644 --- a/models/user/federated_user_test.go +++ b/models/user/federated_user_test.go @@ -14,6 +14,7 @@ func Test_FederatedUserValidation(t *testing.T) { UserID: 12, ExternalID: "12", FederationHostID: 1, + InboxPath: "/api/v1/activitypub/user-id/12/inbox", } if res, err := validation.IsValid(sut); !res { t.Errorf("sut should be valid but was %q", err) @@ -22,6 +23,7 @@ func Test_FederatedUserValidation(t *testing.T) { sut = FederatedUser{ ExternalID: "12", FederationHostID: 1, + InboxPath: "/api/v1/activitypub/user-id/12/inbox", } if res, _ := validation.IsValid(sut); res { t.Error("sut should be invalid") diff --git a/models/user/follow.go b/models/user/follow.go index 5be0f73c35..e32c226385 100644 --- a/models/user/follow.go +++ b/models/user/follow.go @@ -11,6 +11,7 @@ import ( ) // Follow represents relations of user and their followers. +// TODO: We should unify Activity-pub-following and classical following (see models/user/user_repository.go#IsFollowingAp) type Follow struct { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"UNIQUE(follow)"` diff --git a/models/user/user_repository.go b/models/user/user_repository.go index 299d3af64a..3f24efb1fb 100644 --- a/models/user/user_repository.go +++ b/models/user/user_repository.go @@ -1,4 +1,4 @@ -// Copyright 2024 The Forgejo Authors. All rights reserved. +// Copyright 2024, 2025 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package user @@ -8,12 +8,14 @@ import ( "fmt" "forgejo.org/models/db" + "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/validation" ) func init() { db.RegisterModel(new(FederatedUser)) + db.RegisterModel(new(FederatedUserFollower)) } func CreateFederatedUser(ctx context.Context, user *User, federatedUser *FederatedUser) error { @@ -30,7 +32,12 @@ func CreateFederatedUser(ctx context.Context, user *User, federatedUser *Federat if err != nil { return err } - defer committer.Close() + defer func() { + err := committer.Close() + if err != nil { + log.Error("Error closing committer: %v", err) + } + }() if err := CreateUser(ctx, user, &overwrite); err != nil { return err @@ -50,6 +57,14 @@ func CreateFederatedUser(ctx context.Context, user *User, federatedUser *Federat return committer.Commit() } +func (federatedUser *FederatedUser) UpdateFederatedUser(ctx context.Context) error { + if _, err := validation.IsValid(federatedUser); err != nil { + return err + } + _, err := db.GetEngine(ctx).ID(federatedUser.ID).Cols("inbox_path").Update(federatedUser) + return err +} + func FindFederatedUser(ctx context.Context, externalID string, federationHostID int64) (*User, *FederatedUser, error) { federatedUser := new(FederatedUser) user := new(User) @@ -75,6 +90,41 @@ func FindFederatedUser(ctx context.Context, externalID string, federationHostID return user, federatedUser, nil } +func GetFederatedUser(ctx context.Context, externalID string, federationHostID int64) (*User, *FederatedUser, error) { + user, federatedUser, err := FindFederatedUser(ctx, externalID, federationHostID) + if err != nil { + return nil, nil, err + } else if federatedUser == nil { + return nil, nil, fmt.Errorf("FederatedUser for externalId = %v and federationHostId = %v does not exist", externalID, federationHostID) + } + return user, federatedUser, nil +} + +func GetFederatedUserByUserID(ctx context.Context, userID int64) (*User, *FederatedUser, error) { + federatedUser := new(FederatedUser) + user := new(User) + has, err := db.GetEngine(ctx).Where("user_id=?", userID).Get(federatedUser) + if err != nil { + return nil, nil, err + } else if !has { + return nil, nil, fmt.Errorf("Federated user %v does not exist", federatedUser.UserID) + } + has, err = db.GetEngine(ctx).ID(federatedUser.UserID).Get(user) + if err != nil { + return nil, nil, err + } else if !has { + return nil, nil, fmt.Errorf("User %v for federated user is missing", federatedUser.UserID) + } + + if res, err := validation.IsValid(*user); !res { + return nil, nil, err + } + if res, err := validation.IsValid(*federatedUser); !res { + return nil, nil, err + } + return user, federatedUser, nil +} + func FindFederatedUserByKeyID(ctx context.Context, keyID string) (*User, *FederatedUser, error) { federatedUser := new(FederatedUser) user := new(User) @@ -101,7 +151,85 @@ func FindFederatedUserByKeyID(ctx context.Context, keyID string) (*User, *Federa return user, federatedUser, nil } +func UpdateFederatedUser(ctx context.Context, federatedUser *FederatedUser) error { + if res, err := validation.IsValid(federatedUser); !res { + return err + } + _, err := db.GetEngine(ctx).ID(federatedUser.ID).Update(federatedUser) + return err +} + func DeleteFederatedUser(ctx context.Context, userID int64) error { _, err := db.GetEngine(ctx).Delete(&FederatedUser{UserID: userID}) return err } + +func GetFollowersForUser(ctx context.Context, user *User) ([]*FederatedUserFollower, error) { + if res, err := validation.IsValid(user); !res { + return nil, err + } + followers := make([]*FederatedUserFollower, 0, 8) + + err := db.GetEngine(ctx). + Where("followed_user_id = ?", user.ID). + Find(&followers) + if err != nil { + return nil, err + } + for _, element := range followers { + if res, err := validation.IsValid(*element); !res { + return nil, err + } + } + return followers, nil +} + +func AddFollower(ctx context.Context, followedUser *User, followingUser *FederatedUser) (*FederatedUserFollower, error) { + if res, err := validation.IsValid(followedUser); !res { + return nil, err + } + if res, err := validation.IsValid(followingUser); !res { + return nil, err + } + + federatedUserFollower, err := NewFederatedUserFollower(followedUser.ID, followingUser.UserID) + if err != nil { + return nil, err + } + _, err = db.GetEngine(ctx).Insert(&federatedUserFollower) + if err != nil { + return nil, err + } + + return &federatedUserFollower, err +} + +func RemoveFollower(ctx context.Context, followedUser *User, followingUser *FederatedUser) error { + if res, err := validation.IsValid(followedUser); !res { + return err + } + if res, err := validation.IsValid(followingUser); !res { + return err + } + + _, err := db.GetEngine(ctx).Delete(&FederatedUserFollower{ + FollowedUserID: followedUser.ID, + FollowingUserID: followingUser.UserID, + }) + return err +} + +// TODO: We should unify Activity-pub-following and classical following (see models/user/follow.go) +func IsFollowingAp(ctx context.Context, followedUser *User, followingUser *FederatedUser) (bool, error) { + if res, err := validation.IsValid(followedUser); !res { + return false, err + } + if res, err := validation.IsValid(followingUser); !res { + return false, err + } + + return db.GetEngine(ctx).Get(&FederatedUserFollower{ + FollowedUserID: followedUser.ID, + FollowingUserID: followingUser.UserID, + }) +} diff --git a/models/user/user_test.go b/models/user/user_test.go index 2a9e652a35..fd9d05653f 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -148,7 +148,7 @@ func TestAPActorID_APActorID(t *testing.T) { assert.Equal(t, expected, url) } -func TestAPActorKeyID(t *testing.T) { +func TestKeyID(t *testing.T) { user := user_model.User{ID: 1} url := user.APActorKeyID() expected := "https://try.gitea.io/api/v1/activitypub/user-id/1#main-key" diff --git a/services/federation/federation_service.go b/services/federation/federation_service.go index 174c175f86..a3b719d1a7 100644 --- a/services/federation/federation_service.go +++ b/services/federation/federation_service.go @@ -211,6 +211,11 @@ func CreateUserFromAP(ctx context.Context, personID fm.PersonID, federationHostI return nil, nil, err } + inbox, err := url.ParseRequestURI(person.Inbox.GetLink().String()) + if err != nil { + return nil, nil, err + } + newUser := user.User{ LowerName: strings.ToLower(name), Name: name, @@ -227,6 +232,7 @@ func CreateUserFromAP(ctx context.Context, personID fm.PersonID, federationHostI federatedUser := user.FederatedUser{ ExternalID: personID.ID, FederationHostID: federationHostID, + InboxPath: inbox.Path, NormalizedOriginalURL: personID.AsURI(), } diff --git a/services/feed/action.go b/services/feed/action.go index a2cd0551a3..7d179bd1c8 100644 --- a/services/feed/action.go +++ b/services/feed/action.go @@ -39,6 +39,24 @@ func NewNotifier() notify_service.Notifier { return &actionNotifier{} } +func notifyAll(ctx context.Context, action *activities_model.Action) error { + _, err := activities_model.NotifyWatchers(ctx, action) + if err != nil { + return err + } + return err + // return federation_service.NotifyActivityPubFollowers(ctx, out) +} + +func notifyAllActions(ctx context.Context, acts []*activities_model.Action) error { + _, err := activities_model.NotifyWatchersActions(ctx, acts) + if err != nil { + return err + } + return nil + // return federation_service.NotifyActivityPubFollowers(ctx, out) +} + func (a *actionNotifier) NewIssue(ctx context.Context, issue *issues_model.Issue, mentions []*user_model.User) { if err := issue.LoadPoster(ctx); err != nil { log.Error("issue.LoadPoster: %v", err) @@ -50,7 +68,7 @@ func (a *actionNotifier) NewIssue(ctx context.Context, issue *issues_model.Issue } repo := issue.Repo - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: issue.Poster.ID, ActUser: issue.Poster, OpType: activities_model.ActionCreateIssue, @@ -91,7 +109,7 @@ func (a *actionNotifier) IssueChangeStatus(ctx context.Context, doer *user_model } // Notify watchers for whatever action comes in, ignore if no action type. - if err := activities_model.NotifyWatchers(ctx, act); err != nil { + if err := notifyAll(ctx, act); err != nil { log.Error("NotifyWatchers: %v", err) } } @@ -127,7 +145,7 @@ func (a *actionNotifier) CreateIssueComment(ctx context.Context, doer *user_mode } // Notify watchers for whatever action comes in, ignore if no action type. - if err := activities_model.NotifyWatchers(ctx, act); err != nil { + if err := notifyAll(ctx, act); err != nil { log.Error("NotifyWatchers: %v", err) } } @@ -146,7 +164,7 @@ func (a *actionNotifier) NewPullRequest(ctx context.Context, pull *issues_model. return } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: pull.Issue.Poster.ID, ActUser: pull.Issue.Poster, OpType: activities_model.ActionCreatePullRequest, @@ -160,7 +178,7 @@ func (a *actionNotifier) NewPullRequest(ctx context.Context, pull *issues_model. } func (a *actionNotifier) RenameRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldRepoName string) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionRenameRepo, @@ -174,7 +192,7 @@ func (a *actionNotifier) RenameRepository(ctx context.Context, doer *user_model. } func (a *actionNotifier) TransferRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldOwnerName string) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionTransferRepo, @@ -188,7 +206,7 @@ func (a *actionNotifier) TransferRepository(ctx context.Context, doer *user_mode } func (a *actionNotifier) CreateRepository(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionCreateRepo, @@ -201,7 +219,7 @@ func (a *actionNotifier) CreateRepository(ctx context.Context, doer, u *user_mod } func (a *actionNotifier) ForkRepository(ctx context.Context, doer *user_model.User, oldRepo, repo *repo_model.Repository) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionCreateRepo, @@ -266,13 +284,13 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model actions = append(actions, action) } - if err := activities_model.NotifyWatchersActions(ctx, actions); err != nil { + if err := notifyAllActions(ctx, actions); err != nil { log.Error("notify watchers '%d/%d': %v", review.Reviewer.ID, review.Issue.RepoID, err) } } func (*actionNotifier) MergePullRequest(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionMergePullRequest, @@ -286,7 +304,7 @@ func (*actionNotifier) MergePullRequest(ctx context.Context, doer *user_model.Us } func (*actionNotifier) AutoMergePullRequest(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionAutoMergePullRequest, @@ -304,7 +322,7 @@ func (*actionNotifier) NotifyPullRevieweDismiss(ctx context.Context, doer *user_ if len(review.OriginalAuthor) > 0 { reviewerName = review.OriginalAuthor } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionPullReviewDismissed, @@ -342,7 +360,7 @@ func (a *actionNotifier) PushCommits(ctx context.Context, pusher *user_model.Use opType = activities_model.ActionDeleteBranch } - if err = activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err = notifyAll(ctx, &activities_model.Action{ ActUserID: pusher.ID, ActUser: pusher, OpType: opType, @@ -362,7 +380,7 @@ func (a *actionNotifier) CreateRef(ctx context.Context, doer *user_model.User, r // has sent same action in `PushCommits`, so skip it. return } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: opType, @@ -381,7 +399,7 @@ func (a *actionNotifier) DeleteRef(ctx context.Context, doer *user_model.User, r // has sent same action in `PushCommits`, so skip it. return } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: doer.ID, ActUser: doer, OpType: opType, @@ -405,7 +423,7 @@ func (a *actionNotifier) SyncPushCommits(ctx context.Context, pusher *user_model return } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: repo.OwnerID, ActUser: repo.MustOwner(ctx), OpType: activities_model.ActionMirrorSyncPush, @@ -420,7 +438,7 @@ func (a *actionNotifier) SyncPushCommits(ctx context.Context, pusher *user_model } func (a *actionNotifier) SyncCreateRef(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, refFullName git.RefName, refID string) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: repo.OwnerID, ActUser: repo.MustOwner(ctx), OpType: activities_model.ActionMirrorSyncCreate, @@ -434,7 +452,7 @@ func (a *actionNotifier) SyncCreateRef(ctx context.Context, doer *user_model.Use } func (a *actionNotifier) SyncDeleteRef(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, refFullName git.RefName) { - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: repo.OwnerID, ActUser: repo.MustOwner(ctx), OpType: activities_model.ActionMirrorSyncDelete, @@ -452,7 +470,7 @@ func (a *actionNotifier) NewRelease(ctx context.Context, rel *repo_model.Release log.Error("LoadAttributes: %v", err) return } - if err := activities_model.NotifyWatchers(ctx, &activities_model.Action{ + if err := notifyAll(ctx, &activities_model.Action{ ActUserID: rel.PublisherID, ActUser: rel.Publisher, OpType: activities_model.ActionPublishRelease, From 9e6f722f94aec81bc41549aa485a255d31268796 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sat, 21 Jun 2025 12:15:38 +0200 Subject: [PATCH 13/21] fix: only send Forgejo Actions notifications to one user (#8227) - If the run was attributed to a system user (which is the case for scheduled runs for instance), ignore it and fallback to the mail of the owner. System users may have email addresses, but they are not to be used. - If the owner is a system user or an organization with no email associated with it, do nothing. - If a user with an email exists, check if they did not disable notifications and send the email. Refs: https://codeberg.org/forgejo/forgejo/issues/8187 Refs: https://codeberg.org/forgejo/forgejo/issues/8233 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. ### Documentation - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8227 Reviewed-by: Christopher Besch Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- models/user/user_system.go | 7 + routers/web/repo/issue.go | 2 +- services/mailer/mail_actions.go | 19 +- services/mailer/mail_actions_now_done_test.go | 270 ++++++++++++------ services/mailer/main_test.go | 8 +- 5 files changed, 201 insertions(+), 105 deletions(-) diff --git a/models/user/user_system.go b/models/user/user_system.go index 82805cc8ee..11f54591b7 100644 --- a/models/user/user_system.go +++ b/models/user/user_system.go @@ -12,6 +12,13 @@ import ( "forgejo.org/modules/structs" ) +// IsSystem returns true if the user has a fixed +// negative ID, is never stored in the database and +// is generated on the fly when needed. +func (u *User) IsSystem() bool { + return u.IsGhost() || u.IsActions() +} + const ( GhostUserID = -1 GhostUserName = "Ghost" diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index d8f3bd8d9f..3f17e30cec 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1308,7 +1308,7 @@ func roleDescriptor(ctx stdCtx.Context, repo *repo_model.Repository, poster *use } // Special user that can't have associated contributions and permissions in the repo. - if poster.IsGhost() || poster.IsActions() || poster.IsAPServerActor() { + if poster.IsSystem() || poster.IsAPServerActor() { return roleDescriptor, nil } diff --git a/services/mailer/mail_actions.go b/services/mailer/mail_actions.go index 7c63603a98..ad2bd7dfab 100644 --- a/services/mailer/mail_actions.go +++ b/services/mailer/mail_actions.go @@ -23,19 +23,20 @@ func MailActionRun(run *actions_model.ActionRun, priorStatus actions_model.Statu return nil } - if run.TriggerUser.Email != "" && run.TriggerUser.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { - if err := sendMailActionRun(run.TriggerUser, run, priorStatus, lastRun); err != nil { - return err - } + user := run.TriggerUser + // this happens e.g. when this is a scheduled run + if user.IsSystem() { + user = run.Repo.Owner + } + if user.IsSystem() || user.Email == "" { + return nil } - if run.Repo.Owner.Email != "" && run.Repo.Owner.Email != run.TriggerUser.Email && run.Repo.Owner.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { - if err := sendMailActionRun(run.Repo.Owner, run, priorStatus, lastRun); err != nil { - return err - } + if user.EmailNotificationsPreference == user_model.EmailNotificationsDisabled { + return nil } - return nil + return sendMailActionRun(user, run, priorStatus, lastRun) } func sendMailActionRun(to *user_model.User, run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error { diff --git a/services/mailer/mail_actions_now_done_test.go b/services/mailer/mail_actions_now_done_test.go index 0d832f2b36..19c6d9670b 100644 --- a/services/mailer/mail_actions_now_done_test.go +++ b/services/mailer/mail_actions_now_done_test.go @@ -4,42 +4,53 @@ package mailer import ( + "slices" "testing" actions_model "forgejo.org/models/actions" "forgejo.org/models/db" + organization_model "forgejo.org/models/organization" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" + "forgejo.org/modules/optional" "forgejo.org/modules/setting" + "forgejo.org/modules/test" notify_service "forgejo.org/services/notify" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func getActionsNowDoneTestUsers(t *testing.T) []*user_model.User { +func getActionsNowDoneTestUser(t *testing.T, name, email, notifications string) *user_model.User { t.Helper() - newTriggerUser := new(user_model.User) - newTriggerUser.Name = "new_trigger_user" - newTriggerUser.Language = "en_US" - newTriggerUser.IsAdmin = false - newTriggerUser.Email = "new_trigger_user@example.com" - newTriggerUser.LastLoginUnix = 1693648327 - newTriggerUser.CreatedUnix = 1693648027 - newTriggerUser.EmailNotificationsPreference = user_model.EmailNotificationsEnabled - require.NoError(t, user_model.CreateUser(db.DefaultContext, newTriggerUser)) + user := new(user_model.User) + user.Name = name + user.Language = "en_US" + user.IsAdmin = false + user.Email = email + user.LastLoginUnix = 1693648327 + user.CreatedUnix = 1693648027 + opts := user_model.CreateUserOverwriteOptions{ + AllowCreateOrganization: optional.Some(true), + EmailNotificationsPreference: ¬ifications, + } + require.NoError(t, user_model.AdminCreateUser(db.DefaultContext, user, &opts)) + return user +} - newOwner := new(user_model.User) - newOwner.Name = "new_owner" - newOwner.Language = "en_US" - newOwner.IsAdmin = false - newOwner.Email = "new_owner@example.com" - newOwner.LastLoginUnix = 1693648329 - newOwner.CreatedUnix = 1693648029 - newOwner.EmailNotificationsPreference = user_model.EmailNotificationsEnabled - require.NoError(t, user_model.CreateUser(db.DefaultContext, newOwner)) - - return []*user_model.User{newTriggerUser, newOwner} +func getActionsNowDoneTestOrg(t *testing.T, name, email string, owner *user_model.User) *user_model.User { + t.Helper() + org := new(organization_model.Organization) + org.Name = name + org.Language = "en_US" + org.IsAdmin = false + // contact email for the organization, for display purposes but otherwise not used as of v12 + org.Email = email + org.LastLoginUnix = 1693648327 + org.CreatedUnix = 1693648027 + org.Email = email + require.NoError(t, organization_model.CreateOrganization(db.DefaultContext, org, owner)) + return (*user_model.User)(org) } func assertTranslatedLocaleMailActionsNowDone(t *testing.T, msgBody string) { @@ -49,98 +60,169 @@ func assertTranslatedLocaleMailActionsNowDone(t *testing.T, msgBody string) { func TestActionRunNowDoneNotificationMail(t *testing.T) { ctx := t.Context() - users := getActionsNowDoneTestUsers(t) - defer CleanUpUsers(ctx, users) - triggerUser := users[0] - ownerUser := users[1] + defer test.MockVariableValue(&setting.Admin.DisableRegularOrgCreation, false)() + + actionsUser := user_model.NewActionsUser() + require.NotEmpty(t, actionsUser.Email) repo := repo_model.Repository{ Name: "some repo", Description: "rockets are cool", - Owner: ownerUser, - OwnerID: ownerUser.ID, } // Do some funky stuff with the action run's ids: // The run with the larger ID finished first. // This is odd but something that must work. - run1 := &actions_model.ActionRun{ID: 2, Repo: &repo, RepoID: repo.ID, Title: "some workflow", TriggerUser: triggerUser, TriggerUserID: triggerUser.ID, Status: actions_model.StatusFailure, Stopped: 1745821796, TriggerEvent: "workflow_dispatch"} - run2 := &actions_model.ActionRun{ID: 1, Repo: &repo, RepoID: repo.ID, Title: "some workflow", TriggerUser: triggerUser, TriggerUserID: triggerUser.ID, Status: actions_model.StatusSuccess, Stopped: 1745822796, TriggerEvent: "push"} + run1 := &actions_model.ActionRun{ID: 2, Repo: &repo, RepoID: repo.ID, Title: "some workflow", Status: actions_model.StatusFailure, Stopped: 1745821796, TriggerEvent: "workflow_dispatch"} + run2 := &actions_model.ActionRun{ID: 1, Repo: &repo, RepoID: repo.ID, Title: "some workflow", Status: actions_model.StatusSuccess, Stopped: 1745822796, TriggerEvent: "push"} + + assignUsers := func(triggerUser, owner *user_model.User) { + for _, run := range []*actions_model.ActionRun{run1, run2} { + run.TriggerUser = triggerUser + run.TriggerUserID = triggerUser.ID + } + repo.Owner = owner + repo.OwnerID = owner.ID + } notify_service.RegisterNotifier(NewNotifier()) + orgOwner := getActionsNowDoneTestUser(t, "org_owner", "org_owner@example.com", "disabled") + defer CleanUpUsers(ctx, []*user_model.User{orgOwner}) + t.Run("DontSendNotificationEmailOnFirstActionSuccess", func(t *testing.T) { + user := getActionsNowDoneTestUser(t, "new_user", "new_user@example.com", "enabled") + defer CleanUpUsers(ctx, []*user_model.User{user}) + assignUsers(user, user) defer MockMailSettings(func(msgs ...*Message) { assert.Fail(t, "no mail should be sent") })() notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, nil) }) - t.Run("SendNotificationEmailOnActionRunFailed", func(t *testing.T) { - mailSentToOwner := false - mailSentToTriggerUser := false - defer MockMailSettings(func(msgs ...*Message) { - assert.LessOrEqual(t, len(msgs), 2) - for _, msg := range msgs { - switch msg.To { - case triggerUser.EmailTo(): - assert.False(t, mailSentToTriggerUser, "sent mail twice") - mailSentToTriggerUser = true - case ownerUser.EmailTo(): - assert.False(t, mailSentToOwner, "sent mail twice") - mailSentToOwner = true - default: - assert.Fail(t, "sent mail to unknown sender", msg.To) - } - assert.Contains(t, msg.Body, triggerUser.HTMLURL()) - assert.Contains(t, msg.Body, triggerUser.Name) - // what happened - assert.Contains(t, msg.Body, "failed") - // new status of run - assert.Contains(t, msg.Body, "failure") - // prior status of this run - assert.Contains(t, msg.Body, "waiting") - assertTranslatedLocaleMailActionsNowDone(t, msg.Body) - } - })() - notify_service.ActionRunNowDone(ctx, run1, actions_model.StatusWaiting, nil) - assert.True(t, mailSentToOwner) - assert.True(t, mailSentToTriggerUser) - }) + for _, testCase := range []struct { + name string + triggerUser *user_model.User + owner *user_model.User + expected string + expectMail bool + }{ + { + // if the action is assigned a trigger user in a repository + // owned by a regular user, the mail is sent to the trigger user + name: "RegularTriggerUser", + triggerUser: getActionsNowDoneTestUser(t, "new_trigger_user0", "new_trigger_user0@example.com", user_model.EmailNotificationsEnabled), + owner: getActionsNowDoneTestUser(t, "new_owner0", "new_owner0@example.com", user_model.EmailNotificationsEnabled), + expected: "trigger", + expectMail: true, + }, + { + // if the action is assigned to a system user (e.g. ActionsUser) + // in a repository owned by a regular user, the mail is sent to + // the user that owns the repository + name: "SystemTriggerUserAndRegularOwner", + triggerUser: actionsUser, + owner: getActionsNowDoneTestUser(t, "new_owner1", "new_owner1@example.com", user_model.EmailNotificationsEnabled), + expected: "owner", + expectMail: true, + }, + { + // if the action is assigned a trigger user with disabled notifications in a repository + // owned by a regular user, no mail is sent + name: "RegularTriggerUserNotificationsDisabled", + triggerUser: getActionsNowDoneTestUser(t, "new_trigger_user2", "new_trigger_user2@example.com", user_model.EmailNotificationsDisabled), + owner: getActionsNowDoneTestUser(t, "new_owner2", "new_owner2@example.com", user_model.EmailNotificationsEnabled), + expectMail: false, + }, + { + // if the action is assigned to a system user (e.g. ActionsUser) + // owned by a regular user with disabled notifications, no mail is sent + name: "SystemTriggerUserAndRegularOwnerNotificationsDisabled", + triggerUser: actionsUser, + owner: getActionsNowDoneTestUser(t, "new_owner3", "new_owner3@example.com", user_model.EmailNotificationsDisabled), + expectMail: false, + }, + { + // if the action is assigned to a system user (e.g. ActionsUser) + // in a repository owned by an organization with an email contact, the mail is sent to + // this email contact + name: "SystemTriggerUserAndOrgOwner", + triggerUser: actionsUser, + owner: getActionsNowDoneTestOrg(t, "new_org1", "new_org_owner0@example.com", orgOwner), + expected: "owner", + expectMail: true, + }, + { + // if the action is assigned to a system user (e.g. ActionsUser) + // in a repository owned by an organization without an email contact, no mail is sent + name: "SystemTriggerUserAndNoMailOrgOwner", + triggerUser: actionsUser, + owner: getActionsNowDoneTestOrg(t, "new_org2", "", orgOwner), + expectMail: false, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + assignUsers(testCase.triggerUser, testCase.owner) + defer CleanUpUsers(ctx, slices.DeleteFunc([]*user_model.User{testCase.triggerUser, testCase.owner}, func(user *user_model.User) bool { + return user.IsSystem() + })) - t.Run("SendNotificationEmailOnActionRunRecovered", func(t *testing.T) { - mailSentToOwner := false - mailSentToTriggerUser := false - defer MockMailSettings(func(msgs ...*Message) { - assert.LessOrEqual(t, len(msgs), 2) - for _, msg := range msgs { - switch msg.To { - case triggerUser.EmailTo(): - assert.False(t, mailSentToTriggerUser, "sent mail twice") - mailSentToTriggerUser = true - case ownerUser.EmailTo(): - assert.False(t, mailSentToOwner, "sent mail twice") - mailSentToOwner = true - default: - assert.Fail(t, "sent mail to unknown sender", msg.To) - } - assert.Contains(t, msg.Body, triggerUser.HTMLURL()) - assert.Contains(t, msg.Body, triggerUser.Name) - // what happened - assert.Contains(t, msg.Body, "recovered") - // old status of run - assert.Contains(t, msg.Body, "failure") - // new status of run - assert.Contains(t, msg.Body, "success") - // prior status of this run - assert.Contains(t, msg.Body, "running") - assertTranslatedLocaleMailActionsNowDone(t, msg.Body) - } - })() - assert.NotNil(t, setting.MailService) + t.Run("SendNotificationEmailOnActionRunFailed", func(t *testing.T) { + mailSent := false + defer MockMailSettings(func(msgs ...*Message) { + assert.Len(t, msgs, 1) + msg := msgs[0] + assert.False(t, mailSent, "sent mail twice") + expectedEmail := testCase.triggerUser.Email + if testCase.expected == "owner" { // otherwise "trigger" + expectedEmail = testCase.owner.Email + } + require.Contains(t, msg.To, expectedEmail, "sent mail to unknown sender") + mailSent = true + assert.Contains(t, msg.Body, testCase.triggerUser.HTMLURL()) + assert.Contains(t, msg.Body, testCase.triggerUser.Name) + // what happened + assert.Contains(t, msg.Body, "failed") + // new status of run + assert.Contains(t, msg.Body, "failure") + // prior status of this run + assert.Contains(t, msg.Body, "waiting") + assertTranslatedLocaleMailActionsNowDone(t, msg.Body) + })() + require.NotNil(t, setting.MailService) - notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, run1) - assert.True(t, mailSentToOwner) - assert.True(t, mailSentToTriggerUser) - }) + notify_service.ActionRunNowDone(ctx, run1, actions_model.StatusWaiting, nil) + assert.Equal(t, testCase.expectMail, mailSent) + }) + + t.Run("SendNotificationEmailOnActionRunRecovered", func(t *testing.T) { + mailSent := false + defer MockMailSettings(func(msgs ...*Message) { + assert.Len(t, msgs, 1) + msg := msgs[0] + assert.False(t, mailSent, "sent mail twice") + expectedEmail := testCase.triggerUser.Email + if testCase.expected == "owner" { // otherwise "trigger" + expectedEmail = testCase.owner.Email + } + require.Contains(t, msg.To, expectedEmail, "sent mail to unknown sender") + mailSent = true + assert.Contains(t, msg.Body, testCase.triggerUser.HTMLURL()) + assert.Contains(t, msg.Body, testCase.triggerUser.Name) + // what happened + assert.Contains(t, msg.Body, "recovered") + // old status of run + assert.Contains(t, msg.Body, "failure") + // new status of run + assert.Contains(t, msg.Body, "success") + // prior status of this run + assert.Contains(t, msg.Body, "running") + })() + require.NotNil(t, setting.MailService) + + notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, run1) + assert.Equal(t, testCase.expectMail, mailSent) + }) + }) + } } diff --git a/services/mailer/main_test.go b/services/mailer/main_test.go index 47e5d5d175..5e9cbe3e99 100644 --- a/services/mailer/main_test.go +++ b/services/mailer/main_test.go @@ -8,6 +8,7 @@ import ( "testing" "forgejo.org/models/db" + organization_model "forgejo.org/models/organization" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" "forgejo.org/modules/setting" @@ -51,6 +52,11 @@ func MockMailSettings(send func(msgs ...*Message)) func() { func CleanUpUsers(ctx context.Context, users []*user_model.User) { for _, u := range users { - db.DeleteByID[user_model.User](ctx, u.ID) + if u.IsOrganization() { + organization_model.DeleteOrganization(ctx, (*organization_model.Organization)(u)) + } else { + db.DeleteByID[user_model.User](ctx, u.ID) + db.DeleteByBean(ctx, &user_model.EmailAddress{UID: u.ID}) + } } } From b2c4fc9f94e3ba2a5b5259dca4cf38b04701e574 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sat, 21 Jun 2025 13:11:01 +0200 Subject: [PATCH 14/21] bug: Forgejo Actions email notifications are opt-in (#8242) * Add the `notify-email` column / NotifyEmail to ActionRun and set it: * services/actions/workflows.go `Dispatch` * services/actions/schedule_tasks.go `CreateScheduleTask` * services/actions/notifier_helper.go `handleWorkflows` * Only send an email if the workflow has `enable-email-notifications: true` by having `MailActionRun` return immediately if `NotifyEmail` is false. * Ignore or silently fail on `enable-email-notifications: true` parsing errors. Reporting such errors belongs in workflow validation, not when it is evaluated for the notifications. * Add unit and integration tests. Refs: https://codeberg.org/forgejo/forgejo/issues/8187 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. ### Documentation - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8242 Reviewed-by: Christopher Besch Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- models/actions/run.go | 1 + models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v34.go | 14 ++ services/actions/notifier_helper.go | 8 ++ services/actions/schedule_tasks.go | 12 ++ services/actions/schedule_tasks_test.go | 121 ++++++++++++++++++ services/actions/workflows.go | 6 + services/mailer/mail_actions.go | 4 + services/mailer/mail_actions_now_done_test.go | 12 ++ .../integration/actions_notifications_test.go | 88 +++++++++++++ .../actions_run_now_done_notification_test.go | 6 + 11 files changed, 274 insertions(+) create mode 100644 models/forgejo_migrations/v34.go create mode 100644 services/actions/schedule_tasks_test.go create mode 100644 tests/integration/actions_notifications_test.go diff --git a/models/actions/run.go b/models/actions/run.go index 48756b7a08..55def805ed 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -55,6 +55,7 @@ type ActionRun struct { PreviousDuration time.Duration Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` + NotifyEmail bool } func init() { diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 684ef2ed0e..6a6922ec4e 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -105,6 +105,8 @@ var migrations = []*Migration{ NewMigration("Migrate maven package name concatenation", ChangeMavenArtifactConcatenation), // v32 -> v33 NewMigration("Add federated user activity tables, update the `federated_user` table & add indexes", FederatedUserActivityMigration), + // v33 -> v34 + NewMigration("Add `notify-email` column to `action_run` table", AddNotifyEmailToActionRun), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v34.go b/models/forgejo_migrations/v34.go new file mode 100644 index 0000000000..9e958b934f --- /dev/null +++ b/models/forgejo_migrations/v34.go @@ -0,0 +1,14 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations //nolint:revive + +import "xorm.io/xorm" + +func AddNotifyEmailToActionRun(x *xorm.Engine) error { + type ActionRun struct { + ID int64 + NotifyEmail bool + } + return x.Sync(new(ActionRun)) +} diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 9654186fbb..e240c996b5 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -345,6 +345,14 @@ func handleWorkflows( Status: actions_model.StatusWaiting, } + if workflow, err := model.ReadWorkflow(bytes.NewReader(dwf.Content)); err == nil { + notifications, err := workflow.Notifications() + if err != nil { + log.Error("Notifications: %w", err) + } + run.NotifyEmail = notifications + } + need, err := ifNeedApproval(ctx, run, input.Repo, input.Doer) if err != nil { log.Error("check if need approval for repo %d with user %d: %v", input.Repo.ID, input.Doer.ID, err) diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index 3ec0807d5f..cf8b29ead7 100644 --- a/services/actions/schedule_tasks.go +++ b/services/actions/schedule_tasks.go @@ -4,6 +4,7 @@ package actions import ( + "bytes" "context" "errors" "fmt" @@ -18,6 +19,7 @@ import ( webhook_module "forgejo.org/modules/webhook" "github.com/nektos/act/pkg/jobparser" + act_model "github.com/nektos/act/pkg/model" "xorm.io/builder" ) @@ -140,6 +142,16 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule) return err } + workflow, err := act_model.ReadWorkflow(bytes.NewReader(cron.Content)) + if err != nil { + return err + } + notifications, err := workflow.Notifications() + if err != nil { + return err + } + run.NotifyEmail = notifications + // Parse the workflow specification from the cron schedule workflows, err := jobparser.Parse(cron.Content, jobparser.WithVars(vars)) if err != nil { diff --git a/services/actions/schedule_tasks_test.go b/services/actions/schedule_tasks_test.go new file mode 100644 index 0000000000..7073985252 --- /dev/null +++ b/services/actions/schedule_tasks_test.go @@ -0,0 +1,121 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + + actions_model "forgejo.org/models/actions" + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + webhook_module "forgejo.org/modules/webhook" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCreateScheduleTask(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: 2}) + + assertConstant := func(t *testing.T, cron *actions_model.ActionSchedule, run *actions_model.ActionRun) { + t.Helper() + assert.Equal(t, cron.Title, run.Title) + assert.Equal(t, cron.RepoID, run.RepoID) + assert.Equal(t, cron.OwnerID, run.OwnerID) + assert.Equal(t, cron.WorkflowID, run.WorkflowID) + assert.Equal(t, cron.TriggerUserID, run.TriggerUserID) + assert.Equal(t, cron.Ref, run.Ref) + assert.Equal(t, cron.CommitSHA, run.CommitSHA) + assert.Equal(t, cron.Event, run.Event) + assert.Equal(t, cron.EventPayload, run.EventPayload) + assert.Equal(t, cron.ID, run.ScheduleID) + assert.Equal(t, actions_model.StatusWaiting, run.Status) + } + + assertMutable := func(t *testing.T, expected, run *actions_model.ActionRun) { + t.Helper() + assert.Equal(t, expected.NotifyEmail, run.NotifyEmail) + } + + testCases := []struct { + name string + cron actions_model.ActionSchedule + want []actions_model.ActionRun + }{ + { + name: "simple", + cron: actions_model.ActionSchedule{ + Title: "scheduletitle1", + RepoID: repo.ID, + OwnerID: repo.OwnerID, + WorkflowID: "some.yml", + TriggerUserID: repo.OwnerID, + Ref: "branch", + CommitSHA: "fakeSHA", + Event: webhook_module.HookEventSchedule, + EventPayload: "fakepayload", + Content: []byte( + ` +name: test +on: push +jobs: + job2: + runs-on: ubuntu-latest + steps: + - run: true +`), + }, + want: []actions_model.ActionRun{ + { + Title: "scheduletitle1", + NotifyEmail: false, + }, + }, + }, + { + name: "enable-email-notifications is true", + cron: actions_model.ActionSchedule{ + Title: "scheduletitle2", + RepoID: repo.ID, + OwnerID: repo.OwnerID, + WorkflowID: "some.yml", + TriggerUserID: repo.OwnerID, + Ref: "branch", + CommitSHA: "fakeSHA", + Event: webhook_module.HookEventSchedule, + EventPayload: "fakepayload", + Content: []byte( + ` +name: test +enable-email-notifications: true +on: push +jobs: + job2: + runs-on: ubuntu-latest + steps: + - run: true +`), + }, + want: []actions_model.ActionRun{ + { + Title: "scheduletitle2", + NotifyEmail: true, + }, + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.NoError(t, CreateScheduleTask(t.Context(), &testCase.cron)) + require.Equal(t, len(testCase.want), unittest.GetCount(t, actions_model.ActionRun{RepoID: repo.ID})) + for _, expected := range testCase.want { + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{Title: expected.Title}) + assertConstant(t, &testCase.cron, run) + assertMutable(t, &expected, run) + } + unittest.AssertSuccessfulDelete(t, actions_model.ActionRun{RepoID: repo.ID}) + }) + } +} diff --git a/services/actions/workflows.go b/services/actions/workflows.go index 7ec7c3abed..fbba3fd667 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -111,6 +111,11 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette return nil, nil, err } + notifications, err := wf.Notifications() + if err != nil { + return nil, nil, err + } + run := &actions_model.ActionRun{ Title: title, RepoID: repo.ID, @@ -125,6 +130,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette EventPayload: string(p), TriggerEvent: string(webhook.HookEventWorkflowDispatch), Status: actions_model.StatusWaiting, + NotifyEmail: notifications, } vars, err := actions_model.GetVariablesOfRun(ctx, run) diff --git a/services/mailer/mail_actions.go b/services/mailer/mail_actions.go index ad2bd7dfab..09763e164e 100644 --- a/services/mailer/mail_actions.go +++ b/services/mailer/mail_actions.go @@ -23,6 +23,10 @@ func MailActionRun(run *actions_model.ActionRun, priorStatus actions_model.Statu return nil } + if !run.NotifyEmail { + return nil + } + user := run.TriggerUser // this happens e.g. when this is a scheduled run if user.IsSystem() { diff --git a/services/mailer/mail_actions_now_done_test.go b/services/mailer/mail_actions_now_done_test.go index 19c6d9670b..6a01ea7631 100644 --- a/services/mailer/mail_actions_now_done_test.go +++ b/services/mailer/mail_actions_now_done_test.go @@ -80,6 +80,7 @@ func TestActionRunNowDoneNotificationMail(t *testing.T) { for _, run := range []*actions_model.ActionRun{run1, run2} { run.TriggerUser = triggerUser run.TriggerUserID = triggerUser.ID + run.NotifyEmail = true } repo.Owner = owner repo.OwnerID = owner.ID @@ -100,6 +101,17 @@ func TestActionRunNowDoneNotificationMail(t *testing.T) { notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, nil) }) + t.Run("WorkflowEnableEmailNotificationIsFalse", func(t *testing.T) { + user := getActionsNowDoneTestUser(t, "new_user1", "new_user1@example.com", "enabled") + defer CleanUpUsers(ctx, []*user_model.User{user}) + assignUsers(user, user) + defer MockMailSettings(func(msgs ...*Message) { + assert.Fail(t, "no mail should be sent") + })() + run2.NotifyEmail = false + notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, nil) + }) + for _, testCase := range []struct { name string triggerUser *user_model.User diff --git a/tests/integration/actions_notifications_test.go b/tests/integration/actions_notifications_test.go new file mode 100644 index 0000000000..e47cb64b83 --- /dev/null +++ b/tests/integration/actions_notifications_test.go @@ -0,0 +1,88 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package integration + +import ( + "fmt" + "net/url" + "testing" + + actions_model "forgejo.org/models/actions" + auth_model "forgejo.org/models/auth" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/setting" + "forgejo.org/tests" + + "github.com/stretchr/testify/assert" +) + +func TestActionNotifications(t *testing.T) { + if !setting.Database.Type.IsSQLite3() { + t.Skip() + } + + testCases := []struct { + name string + treePath string + fileContent string + notifyEmail bool + }{ + { + name: "enabled", + treePath: ".forgejo/workflows/enabled.yml", + fileContent: `name: enabled +on: + push: +enable-email-notifications: true +jobs: + job1: + runs-on: ubuntu-latest + steps: + - run: echo job1 +`, + notifyEmail: true, + }, + { + name: "disabled", + treePath: ".forgejo/workflows/disabled.yml", + fileContent: `name: disabled +on: + push: +jobs: + job1: + runs-on: ubuntu-latest + steps: + - run: echo job1 +`, + notifyEmail: false, + }, + } + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user2.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + apiRepo := createActionsTestRepo(t, token, testCase.name, false) + runner := newMockRunner() + runner.registerAsRepoRunner(t, user2.Name, apiRepo.Name, "mock-runner", []string{"ubuntu-latest"}) + opts := getWorkflowCreateFileOptions(user2, apiRepo.DefaultBranch, fmt.Sprintf("create %s", testCase.treePath), testCase.fileContent) + createWorkflowFile(t, token, user2.Name, apiRepo.Name, testCase.treePath, opts) + + task := runner.fetchTask(t) + actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: task.Id}) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: actionTask.JobID}) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: actionRunJob.RunID}) + assert.Equal(t, testCase.notifyEmail, actionRun.NotifyEmail) + + httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteRepository) + doAPIDeleteRepository(httpContext)(t) + }) + } + }) +} diff --git a/tests/integration/actions_run_now_done_notification_test.go b/tests/integration/actions_run_now_done_notification_test.go index 9a2e118701..d5142096c5 100644 --- a/tests/integration/actions_run_now_done_notification_test.go +++ b/tests/integration/actions_run_now_done_notification_test.go @@ -44,30 +44,35 @@ func (m *mockNotifier) ActionRunNowDone(ctx context.Context, run *actions_model. assert.Equal(m.t, actions_model.StatusSuccess, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) assert.Nil(m.t, lastRun) + assert.True(m.t, run.NotifyEmail) case 1: assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusFailure, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) assert.Equal(m.t, m.lastRunID, lastRun.ID) assert.Equal(m.t, actions_model.StatusSuccess, lastRun.Status) + assert.True(m.t, run.NotifyEmail) case 2: assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusCancelled, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) assert.Equal(m.t, m.lastRunID, lastRun.ID) assert.Equal(m.t, actions_model.StatusFailure, lastRun.Status) + assert.True(m.t, run.NotifyEmail) case 3: assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusSuccess, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) assert.Equal(m.t, m.lastRunID, lastRun.ID) assert.Equal(m.t, actions_model.StatusCancelled, lastRun.Status) + assert.True(m.t, run.NotifyEmail) case 4: assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusSuccess, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) assert.Equal(m.t, m.lastRunID, lastRun.ID) assert.Equal(m.t, actions_model.StatusSuccess, lastRun.Status) + assert.True(m.t, run.NotifyEmail) default: assert.Fail(m.t, "too many notifications") } @@ -101,6 +106,7 @@ func TestActionNowDoneNotification(t *testing.T) { TreePath: ".forgejo/workflows/dispatch.yml", ContentReader: strings.NewReader( "name: test\n" + + "enable-email-notifications: true\n" + "on: [workflow_dispatch]\n" + "jobs:\n" + " test:\n" + From 690532efb8022b8b8445b5a6296896b1b319024f Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Sat, 21 Jun 2025 14:42:35 +0200 Subject: [PATCH 15/21] add model viewer for `.glb` (GLTF) model in file view (#8111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation The GLTF (`.gltf`, `.glb`) 3D model format is very popular for game development and visual productions. For an indie game studio, it would be convenient for a team to view textured 3D models directly from the Forgejo interface (otherwise they need to be downloaded and opened). [Perforce](https://www.perforce.com/products/helix-dam), [Diversion](https://www.diversion.dev/), and GitHub all have this capability to differing extents. Some discussion on 3D file support here: https://codeberg.org/forgejo/forgejo/issues/5188 ## Changes Adds a model viewer similar to [GitHub STL viewer](https://github.com/assimp/assimp/blob/master/test/models/STL/Spider_ascii.stl) for `.glb` model files, and lays some groundwork to support future files. Uses the [model-viewer](https://modelviewer.dev/) library by Google and three.js. The model viewer is interactive and can be rotated and scaled. ![Screen Recording 2025-06-08 at 15.27.15](/attachments/84c63dea-a0ce-45f9-b48b-c80867636639) ## How to Test 1) Create a new repository or use an existing one. 2) Upload a `.glb` file such as `tests/testdata/data/viewer/Unicode❤♻Test.glb` (CC0 1.0 Universal) 3) View the file in the repository. - Similar to image files, the 3D model should be rendered in a viewer. - Use mouse clicks to turn and zoom. ## Licenses Libraries used for this change include three.js and @google/model-viewer, which are MIT and Apache-2.0 licenses respectively. Both of these are compatible with Forgejo's GPL3.0 license. ## Future Plans 1) `.gltf` was not attempted because it is a multiple file format, referencing other files in the same directory. Still need to experiment with this to see if it can work. `.glb` is a single file containing a `.gltf` and all of its other file/texture dependencies so was easier to implement. 2) The PR diff still shows the model as an unviewable bin file, but clicking the "View File" button takes you to a view screen where this model viewer is used. It would be nice to view the before and after of the model in two side-by-side model viewers, akin to reviewing a change in an image. 3) Also inserted stubs for adding contexts for GLTF, STL, OBJ, and 3MF. These ultimately don't do anything yet as only `.glb` files can be detected by the type sniffer of all of these. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for checking GLB file content using the first few bytes. - [x] in their respective `typesniffer_test.go` for unit tests. ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - User Interface features - [PR](https://codeberg.org/forgejo/forgejo/pulls/8111): add model viewer for `.glb` (GLTF) model in file view Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8111 Reviewed-by: oliverpool Co-authored-by: Alex Smith Co-committed-by: Alex Smith --- modules/public/mime_types.go | 5 + modules/typesniffer/typesniffer.go | 49 +++++++- modules/typesniffer/typesniffer_test.go | 20 ++++ package-lock.json | 116 ++++++++++++++++++- package.json | 3 +- routers/web/repo/setting/lfs.go | 14 +++ routers/web/repo/view.go | 14 +++ templates/repo/settings/lfs_file.tmpl | 6 + templates/repo/view_file.tmpl | 6 + tests/testdata/data/viewer/README.md | 14 +++ tests/testdata/data/viewer/Unicode❤♻Test.glb | Bin 0 -> 8088 bytes web_src/css/repo.css | 5 + web_src/js/index.js | 2 + web_src/js/render/gltf.js | 6 + 14 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 tests/testdata/data/viewer/README.md create mode 100644 tests/testdata/data/viewer/Unicode❤♻Test.glb create mode 100644 web_src/js/render/gltf.js diff --git a/modules/public/mime_types.go b/modules/public/mime_types.go index 32bdf3bfa2..87ee2854ae 100644 --- a/modules/public/mime_types.go +++ b/modules/public/mime_types.go @@ -23,6 +23,11 @@ var wellKnownMimeTypesLower = map[string]string{ ".wasm": "application/wasm", ".webp": "image/webp", ".xml": "text/xml; charset=utf-8", + ".glb": "model/gltf-binary", + ".gltf": "model/gltf+json", + ".obj": "model/obj", + ".stl": "model/stl", + ".3mf": "model/3mf", // well, there are some types missing from the builtin list ".txt": "text/plain; charset=utf-8", diff --git a/modules/typesniffer/typesniffer.go b/modules/typesniffer/typesniffer.go index a8fc70e54c..262feb2b05 100644 --- a/modules/typesniffer/typesniffer.go +++ b/modules/typesniffer/typesniffer.go @@ -24,6 +24,16 @@ const ( AvifMimeType = "image/avif" // ApplicationOctetStream MIME type of binary files. ApplicationOctetStream = "application/octet-stream" + // GLTFMimeType MIME type of GLTF files. + GLTFMimeType = "model/gltf+json" + // GLBMimeType MIME type of GLB files. + GLBMimeType = "model/gltf-binary" + // OBJMimeType MIME type of OBJ files. + OBJMimeType = "model/obj" + // STLMimeType MIME type of STL files. + STLMimeType = "model/stl" + // 3MFMimeType MIME type of 3MF files. + ThreeMFMimeType = "model/3mf" ) var ( @@ -67,6 +77,36 @@ func (ct SniffedType) IsAudio() bool { return strings.Contains(ct.contentType, "audio/") } +// Is3DModel detects if data is a 3D format +func (ct SniffedType) Is3DModel() bool { + return strings.Contains(ct.contentType, "model/") +} + +// IsGLTFFile detects if data is an SVG image format +func (ct SniffedType) IsGLTF() bool { + return strings.Contains(ct.contentType, GLTFMimeType) +} + +// IsGLBFile detects if data is an GLB image format +func (ct SniffedType) IsGLB() bool { + return strings.Contains(ct.contentType, GLBMimeType) +} + +// IsOBJFile detects if data is an OBJ image format +func (ct SniffedType) IsOBJ() bool { + return strings.Contains(ct.contentType, OBJMimeType) +} + +// IsSTLTextFile detects if data is an STL text format +func (ct SniffedType) IsSTL() bool { + return strings.Contains(ct.contentType, STLMimeType) +} + +// Is3MFFile detects if data is an 3MF image format +func (ct SniffedType) Is3MF() bool { + return strings.Contains(ct.contentType, ThreeMFMimeType) +} + // IsRepresentableAsText returns true if file content can be represented as // plain text or is empty. func (ct SniffedType) IsRepresentableAsText() bool { @@ -75,7 +115,7 @@ func (ct SniffedType) IsRepresentableAsText() bool { // IsBrowsableBinaryType returns whether a non-text type can be displayed in a browser func (ct SniffedType) IsBrowsableBinaryType() bool { - return ct.IsImage() || ct.IsSvgImage() || ct.IsPDF() || ct.IsVideo() || ct.IsAudio() + return ct.IsImage() || ct.IsSvgImage() || ct.IsPDF() || ct.IsVideo() || ct.IsAudio() || ct.Is3DModel() } // GetMimeType returns the mime type @@ -135,6 +175,13 @@ func DetectContentType(data []byte) SniffedType { ct = "audio/ogg" // for most cases, it is used as an audio container } } + + // GLTF is unsupported by http.DetectContentType + // hexdump -n 4 -C glTF.glb + if bytes.HasPrefix(data, []byte("glTF")) { + ct = GLBMimeType + } + return SniffedType{ct} } diff --git a/modules/typesniffer/typesniffer_test.go b/modules/typesniffer/typesniffer_test.go index 8d80b4ddb4..176d3658bb 100644 --- a/modules/typesniffer/typesniffer_test.go +++ b/modules/typesniffer/typesniffer_test.go @@ -117,6 +117,14 @@ func TestIsAudio(t *testing.T) { assert.True(t, DetectContentType([]byte("ID3Toy\n====\t* hi 🌞, ..."+"🌛"[0:2])).IsText()) // test ID3 tag with incomplete UTF8 char } +func TestIsGLB(t *testing.T) { + glb, _ := hex.DecodeString("676c5446") + assert.True(t, DetectContentType(glb).IsGLB()) + assert.True(t, DetectContentType(glb).Is3DModel()) + assert.False(t, DetectContentType([]byte("plain text")).IsGLB()) + assert.False(t, DetectContentType([]byte("plain text")).Is3DModel()) +} + func TestDetectContentTypeFromReader(t *testing.T) { mp3, _ := base64.StdEncoding.DecodeString("SUQzBAAAAAABAFRYWFgAAAASAAADbWFqb3JfYnJhbmQAbXA0MgBUWFhYAAAAEQAAA21pbm9yX3Zl") st, err := DetectContentTypeFromReader(bytes.NewReader(mp3)) @@ -145,3 +153,15 @@ func TestDetectContentTypeAvif(t *testing.T) { assert.True(t, st.IsImage()) } + +func TestDetectContentTypeModelGLB(t *testing.T) { + glb, err := hex.DecodeString("676c5446") + require.NoError(t, err) + + st, err := DetectContentTypeFromReader(bytes.NewReader(glb)) + require.NoError(t, err) + + // print st for debugging + assert.Equal(t, "model/gltf-binary", st.GetMimeType()) + assert.True(t, st.IsGLB()) +} diff --git a/package-lock.json b/package-lock.json index 1637629a83..9de06a8055 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "@github/markdown-toolbar-element": "2.2.3", "@github/quote-selection": "2.1.0", "@github/text-expander-element": "2.8.0", + "@google/model-viewer": "4.1.0", "@mcaptcha/vanilla-glue": "0.1.0-alpha-3", "@primer/octicons": "19.14.0", "ansi_up": "6.0.5", @@ -1222,6 +1223,22 @@ "dom-input-range": "^1.2.0" } }, + "node_modules/@google/model-viewer": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/@google/model-viewer/-/model-viewer-4.1.0.tgz", + "integrity": "sha512-7WB/jS6wfBfRl/tWhsUUvDMKFE1KlKME97coDLlZQfvJD0nCwjhES1lJ+k7wnmf7T3zMvCfn9mIjM/mgZapuig==", + "license": "Apache-2.0", + "dependencies": { + "@monogrid/gainmap-js": "^3.1.0", + "lit": "^3.2.1" + }, + "engines": { + "node": ">=6.0.0" + }, + "peerDependencies": { + "three": "^0.172.0" + } + }, "node_modules/@humanfs/core": { "version": "0.19.1", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.1.tgz", @@ -2004,6 +2021,21 @@ "integrity": "sha512-M5UknZPHRu3DEDWoipU6sE8PdkZ6Z/S+v4dD+Ke8IaNlpdSQah50lz1KtcFBa2vsdOnwbbnxJwVM4wty6udA5w==", "license": "MIT" }, + "node_modules/@lit-labs/ssr-dom-shim": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@lit-labs/ssr-dom-shim/-/ssr-dom-shim-1.3.0.tgz", + "integrity": "sha512-nQIWonJ6eFAvUUrSlwyHDm/aE8PBDu5kRpL0vHMg6K8fK3Diq1xdPjTnsJSwxABhaZ+5eBi1btQB5ShUTKo4nQ==", + "license": "BSD-3-Clause" + }, + "node_modules/@lit/reactive-element": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@lit/reactive-element/-/reactive-element-2.1.0.tgz", + "integrity": "sha512-L2qyoZSQClcBmq0qajBVbhYEcG6iK0XfLn66ifLe/RfC0/ihpc+pl0Wdn8bJ8o+hj38cG0fGXRgSS20MuXn7qA==", + "license": "BSD-3-Clause", + "dependencies": { + "@lit-labs/ssr-dom-shim": "^1.2.0" + } + }, "node_modules/@mcaptcha/core-glue": { "version": "0.1.0-alpha-5", "resolved": "https://registry.npmjs.org/@mcaptcha/core-glue/-/core-glue-0.1.0-alpha-5.tgz", @@ -2064,6 +2096,18 @@ "langium": "3.3.1" } }, + "node_modules/@monogrid/gainmap-js": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@monogrid/gainmap-js/-/gainmap-js-3.1.0.tgz", + "integrity": "sha512-Obb0/gEd/HReTlg8ttaYk+0m62gQJmCblMOjHSMHRrBP2zdfKMHLCRbh/6ex9fSUJMKdjjIEiohwkbGD3wj2Nw==", + "license": "MIT", + "dependencies": { + "promise-worker-transferable": "^1.0.4" + }, + "peerDependencies": { + "three": ">= 0.159.0" + } + }, "node_modules/@napi-rs/wasm-runtime": { "version": "0.2.11", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-0.2.11.tgz", @@ -3493,8 +3537,7 @@ "version": "2.0.7", "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==", - "license": "MIT", - "optional": true + "license": "MIT" }, "node_modules/@types/unist": { "version": "2.0.11", @@ -8768,6 +8811,12 @@ "node": ">= 4" } }, + "node_modules/immediate": { + "version": "3.0.6", + "resolved": "https://registry.npmjs.org/immediate/-/immediate-3.0.6.tgz", + "integrity": "sha512-XXOFtyqDjNDAQxVfYxuF7g9Il/IbWmmlQg2MYKOH8ExIT1qg6xc4zyS3HaEEATgs1btfzxq15ciUiY7gjSXRGQ==", + "license": "MIT" + }, "node_modules/immer": { "version": "9.0.21", "resolved": "https://registry.npmjs.org/immer/-/immer-9.0.21.tgz", @@ -9307,6 +9356,12 @@ "dev": true, "license": "MIT" }, + "node_modules/is-promise": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/is-promise/-/is-promise-2.2.2.tgz", + "integrity": "sha512-+lP4/6lKUBfQjZ2pdxThZvLUAafmZb8OAxFb8XXtiQmS35INgr85hdOGoEs124ez1FCnZJt6jau/T+alh58QFQ==", + "license": "MIT" + }, "node_modules/is-proto-prop": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/is-proto-prop/-/is-proto-prop-3.0.1.tgz", @@ -10023,6 +10078,15 @@ "npm": ">=8" } }, + "node_modules/lie": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/lie/-/lie-3.3.0.tgz", + "integrity": "sha512-UaiMJzeWRlEujzAuw5LokY1L5ecNQYZKfmyZ9L7wDHb/p5etKaxXhohBcrw0EYby+G/NA52vRSN4N39dxHAIwQ==", + "license": "MIT", + "dependencies": { + "immediate": "~3.0.5" + } + }, "node_modules/lilconfig": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/lilconfig/-/lilconfig-3.1.3.tgz", @@ -10051,6 +10115,37 @@ "uc.micro": "^2.0.0" } }, + "node_modules/lit": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/lit/-/lit-3.3.0.tgz", + "integrity": "sha512-DGVsqsOIHBww2DqnuZzW7QsuCdahp50ojuDaBPC7jUDRpYoH0z7kHBBYZewRzer75FwtrkmkKk7iOAwSaWdBmw==", + "license": "BSD-3-Clause", + "dependencies": { + "@lit/reactive-element": "^2.1.0", + "lit-element": "^4.2.0", + "lit-html": "^3.3.0" + } + }, + "node_modules/lit-element": { + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/lit-element/-/lit-element-4.2.0.tgz", + "integrity": "sha512-MGrXJVAI5x+Bfth/pU9Kst1iWID6GHDLEzFEnyULB/sFiRLgkd8NPK/PeeXxktA3T6EIIaq8U3KcbTU5XFcP2Q==", + "license": "BSD-3-Clause", + "dependencies": { + "@lit-labs/ssr-dom-shim": "^1.2.0", + "@lit/reactive-element": "^2.1.0", + "lit-html": "^3.3.0" + } + }, + "node_modules/lit-html": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/lit-html/-/lit-html-3.3.0.tgz", + "integrity": "sha512-RHoswrFAxY2d8Cf2mm4OZ1DgzCoBKUKSPvA1fhtSELxUERq2aQQ2h05pO9j81gS1o7RIRJ+CePLogfyahwmynw==", + "license": "BSD-3-Clause", + "dependencies": { + "@types/trusted-types": "^2.0.2" + } + }, "node_modules/loader-runner": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/loader-runner/-/loader-runner-4.3.0.tgz", @@ -12363,6 +12458,16 @@ "dev": true, "license": "Unlicense" }, + "node_modules/promise-worker-transferable": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/promise-worker-transferable/-/promise-worker-transferable-1.0.4.tgz", + "integrity": "sha512-bN+0ehEnrXfxV2ZQvU2PetO0n4gqBD4ulq3MI1WOPLgr7/Mg9yRQkX5+0v1vagr74ZTsl7XtzlaYDo2EuCeYJw==", + "license": "Apache-2.0", + "dependencies": { + "is-promise": "^2.1.0", + "lie": "^3.0.2" + } + }, "node_modules/proto-list": { "version": "1.2.4", "resolved": "https://registry.npmjs.org/proto-list/-/proto-list-1.2.4.tgz", @@ -14425,6 +14530,13 @@ "node": ">=0.8" } }, + "node_modules/three": { + "version": "0.172.0", + "resolved": "https://registry.npmjs.org/three/-/three-0.172.0.tgz", + "integrity": "sha512-6HMgMlzU97MsV7D/tY8Va38b83kz8YJX+BefKjspMNAv0Vx6dxMogHOrnRl/sbMIs3BPUKijPqDqJ/+UwJbIow==", + "license": "MIT", + "peer": true + }, "node_modules/throttle-debounce": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/throttle-debounce/-/throttle-debounce-5.0.0.tgz", diff --git a/package.json b/package.json index d7c3a5f79f..f7df1b3f38 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "@github/markdown-toolbar-element": "2.2.3", "@github/quote-selection": "2.1.0", "@github/text-expander-element": "2.8.0", + "@google/model-viewer": "4.1.0", "@mcaptcha/vanilla-glue": "0.1.0-alpha-3", "@primer/octicons": "19.14.0", "ansi_up": "6.0.5", @@ -78,8 +79,8 @@ "eslint-plugin-playwright": "2.2.0", "eslint-plugin-regexp": "2.9.0", "eslint-plugin-sonarjs": "3.0.2", - "eslint-plugin-unicorn": "59.0.1", "eslint-plugin-toml": "0.12.0", + "eslint-plugin-unicorn": "59.0.1", "eslint-plugin-vitest-globals": "1.5.0", "eslint-plugin-vue": "10.2.0", "eslint-plugin-vue-scoped-css": "2.10.0", diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index 2e9c34e8a7..b9cb86bd08 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -342,6 +342,20 @@ func LFSFileGet(ctx *context.Context) { ctx.Data["IsVideoFile"] = true case st.IsAudio(): ctx.Data["IsAudioFile"] = true + case st.Is3DModel(): + ctx.Data["Is3DModelFile"] = true + switch { + case st.IsGLB(): + ctx.Data["IsGLBFile"] = true + case st.IsSTL(): + ctx.Data["IsSTLFile"] = true + case st.IsGLTF(): + ctx.Data["IsGLTFFile"] = true + case st.IsOBJ(): + ctx.Data["IsOBJFile"] = true + case st.Is3MF(): + ctx.Data["Is3MFFile"] = true + } case st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage()): ctx.Data["IsImageFile"] = true } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index d958a11f55..bb3e1388a8 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -624,6 +624,20 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["IsVideoFile"] = true case fInfo.st.IsAudio(): ctx.Data["IsAudioFile"] = true + case fInfo.st.Is3DModel(): + ctx.Data["Is3DModelFile"] = true + switch { + case fInfo.st.IsGLB(): + ctx.Data["IsGLBFile"] = true + case fInfo.st.IsSTL(): + ctx.Data["IsSTLFile"] = true + case fInfo.st.IsGLTF(): + ctx.Data["IsGLTFFile"] = true + case fInfo.st.IsOBJ(): + ctx.Data["IsOBJFile"] = true + case fInfo.st.Is3MF(): + ctx.Data["Is3MFFile"] = true + } case fInfo.st.IsImage() && (setting.UI.SVG.Enabled || !fInfo.st.IsSvgImage()): ctx.Data["IsImageFile"] = true ctx.Data["CanCopyContent"] = true diff --git a/templates/repo/settings/lfs_file.tmpl b/templates/repo/settings/lfs_file.tmpl index 3b6b763536..9864ed01d6 100644 --- a/templates/repo/settings/lfs_file.tmpl +++ b/templates/repo/settings/lfs_file.tmpl @@ -32,6 +32,12 @@ {{else if .IsPDFFile}}
+ {{else if .Is3DModelFile}} + {{if .IsGLBFile}} + + {{else}} + {{ctx.Locale.Tr "repo.file_view_raw"}}! + {{end}} {{else}} {{ctx.Locale.Tr "repo.file_view_raw"}} {{end}} diff --git a/templates/repo/view_file.tmpl b/templates/repo/view_file.tmpl index bf668e1347..2a9d7d02ba 100644 --- a/templates/repo/view_file.tmpl +++ b/templates/repo/view_file.tmpl @@ -116,6 +116,12 @@ {{else if .IsPDFFile}}
+ {{else if .Is3DModelFile}} + {{if .IsGLBFile}} + + {{else}} + {{ctx.Locale.Tr "repo.file_view_raw"}} + {{end}} {{else}} {{ctx.Locale.Tr "repo.file_view_raw"}} {{end}} diff --git a/tests/testdata/data/viewer/README.md b/tests/testdata/data/viewer/README.md new file mode 100644 index 0000000000..e26d11f3c8 --- /dev/null +++ b/tests/testdata/data/viewer/README.md @@ -0,0 +1,14 @@ +# GLTF 3D Model Viewer + +⚠️ Currently supports `.glb` files only. + +3D models with the `.glb` format are rendered in repository file view. + +## 🔨 How to Test + +1) Create a new repository or use an existing one. +2) Upload a `.glb` file such as [Unicode❤♻Test.glb](./Unicode❤♻Test.glb) (CC0 1.0 Universal). +3) View the file in the repository. + - Similar to image files, the 3D model should be rendered in a viewer. + - Use mouse clicks to turn and zoom. + diff --git a/tests/testdata/data/viewer/Unicode❤♻Test.glb b/tests/testdata/data/viewer/Unicode❤♻Test.glb new file mode 100644 index 0000000000000000000000000000000000000000..32b0f4a0e8e3f701bf47538436eee8caee863e0e GIT binary patch literal 8088 zcmeHscUV)~w(p{aDpjOIXadrEldcpgp;x6Q6a@mI_hvy+sluig5$PZxMUa4iGzA1H zp-WW|0wRQ#@PgZY_j%`@ciz3L-={Xy?K;)q{Jk6B1CxnTwvZX9wmw2Ek7P5yD%PaI23l? zlDHLdOXNR#VLV&M2EndoOHT*5XBf{h3 zj4*`xJ9&C~xR}BN+}yoket5kw9%m;%m=@dEUv&f_Kg97ZI7it_G?|7L;PU|FmE@1{3^&PN+|q2Z|MOG`YrQI{uk)EIKe(=Ad^2>|! z2>dIrUEp3maBrBmzj=rceywFCrDR2TT;KuT_tg|jQefM^17Dve=_vj(SKO2|Bs=62PXaB zW&Jmp)UP7&@N#ndo!Wn<$e+>p<)0VK{Lhg65)$|EcKfB`|7ZFAnS*}t0E7!Z*!Z0y z0t^P9vI2mno)Lam0ssR1`+R>of1TfnezpGbBUeWSG=b_N_HXzOv73gW1^~QD zq&#&Z!S~61bZz_qK!EoAO_1hc!U6y+7`5R9Vj|y;R1p$&5 zh@$_vPoQql1%OMO0NODLAl`!+=$854>Hlt6*h5aurmx+aIfm#-faRLMI_W%C%9lfM zfub)&iia#}PH#60N%ygQJGpIh&kQUVz6i~@xz0?B2JQ9dW!el!;czj+@g-7v)1~o zce-D);O+#`yAkyJrE(I*_uNnu5a@V^QQglPH+Nj3AR}j>FHGd)ZF4w62|&OZLq%IT z&y)$KPGwQGkH61oP;b3REL9Ikr@72)#LH_WP_21fV6Pm-r&dL;26o&{qv46T{|FMb z;PjJ@H}2>Sf?)BFmD*gWbUw3O0Ml2keve3%5gGAN&a&}JCf!6_LvA(|7x4ArWYm>N zb2Q$t%apFvsf-kYN5wN^n0JUF^7zr`?=SJ=9X^sE0mFg^poICT0-~+rZH2A)Tmx-jMQl}{F%%*&cVkvnKI5r48HYez!xql{Co-8dudubXt#wW1)ggIr_i~f1a5~!9AC6st!;r+CO{<&w-ofE)F zcDwY0)7l))fs1~p*DCx{#uYn9Cm!^VQ+#=?fhLT?mOJKbhfl-%$(KFCu=GE+*{3a8 z;r*?h_C8ebO^;D#nXJ2gDFtqN9eW-f3}D;2^|JM3O1II#OPzfoXC!=|C#;%=aH|SU zj*7fS#mma!PX$cHPf(OM`XXw^0;?sb$RyyGPzM}dO%e>-Q5@H6S_TV`Nq2oA8cGUd zwDX~vqFnb$YQUZsgomx7^CxNREpjTwBc6yHD@PcwnP#7myP*tpnqu0%4k-k6#XRpzElkpv#rgAzkcWIF7ZA9OgWh z20b!3g;r3uBgeT{+cq=u^=uAY-|u9bQx_lX+Tsnp*68RVu^eF}B^10$pyVtn|(YS0?6`HXP!xQ_Y_Ay_)94CDzbgPk zBKA2=?F?slJn`$FOUokMxwjuW=ftpp z0S9r*E$`+@6XG%Y&?M?6RBMmxqK6xch{RV7rJG)Z`5tI(8opNfM>TZ;b*tzR?Th|r z6AaTepLZ_LJ#$nWcD~{vE+>IFea z$;rnK)a15o5|!1#Z%c{6{h}{%2Sh`HHLwsPNDnI`)_w<-r<<6Z~astGHOx~ zvwTr828v-smks%%dM}S-_3ys<3?euVZ}DzQukux_sXtaVGddn59lg&DlLf{Zm(pc3 zGpSeEZSSK~;h8es;o*=--Q=Tg#AbKr&AfZKOwPAvN^@m{TjlxA*pw;18kWKh+}!2O zSN(x!DI=EHxDG{E42PT`wyokTl5x6dHgy#3J}psP3342iZ=egPLvc1KiCz{AeG1>q zZ`M?%BKg_j_;jtRYo*4TlFk?IP*P=EIxSsMagsqi@gP=hBhuu9WL1>_1v&whEW613 z3rf9(&dc<4FYjhy+|*~(Vkp$-@|ea(K5T({x{rOepKIjQsUCiPmPuXa$m%3}t+y?( z`T@M`2W#99jinxcQ2HDoINCG7PxIR8gI9Z<}O*y=_14xed>^6w-#-ARPMuG#*igwC-4NclwzdH3CkdoR~)(h`_ zt9tiZb4sB5)^l?EEslF7?VvgD>UMRT%!7;A+Ssc>@*&$|yN?>@*uKS)`-%-k=DpY< z4@0q9H62m+9dn5OT72kuh*>EKt>}XQAm-Y_}%fS^WtU&Rt>MElQL{&Ju<(u(9nyx4JUGS z)H4q8HHc{@7Y}$)(Jfp>zm$`My-Ar?Gle>vOAdO`V_s|@VExqlWo0Y2%SlnJC&Tih z75U(}WkR;b<0};e3LDp^<83+7-qa?AD!%X@){p9PVr`G$4>vX>Rw3nfrw`j#ndh-# zzQRV|+=fn1Jh#$oylxh0Ebb`gWo>2=Eo|81Vn@8I#wzHEYKZGtqqzm}+uLKH@KAi5 zER2oE!nY~S#%QaH?J`mJJDg~F!mcuNbQV#=CqkveCHcMXA>8#pP@BO7pXTR91RGRL zgC1ZOOH0<|{crX&9*Q}+mSd8*tVS9;p|SuhVuk6k=Dyly)eBPFShfQO_^cU&YZr7; zu?A7g>M!zYvyG(i(U0pSj#uP%Bg{g!=@S8j;qWmx=t9(i$9H%Fo%Ptf&~ppWEpj}X zI!ab`wMwPHR&_v@BzE&k%VEtr8#hr!>^z2OAh|qlPEBxIk4?0A<7a!#8N%-AAEaM{ zMQF-ApwvSF zgZjJ}Mf=r~hEr$6T2Di6o!R9Id-*gORk|mgMF*h{{3nY2)Pz-HhL*NL zld0N0DotYT26%=*P>_51<)^^UYS{gnV}?0OB}9QxGa^w^T4E*)`eXy7v0Or@+Ov1X zi$cG@Vc}CCRqD_jsDC)tImDidopsJU3JHlE*RM-h+hXgU;b$>uBmxSq-F&29y1mbS zdp)g`Te?{+d*S*l_O#l>u~?Jx2LL9 z6$w__2jRVqb`s z6{{KXzOg_kg6)SPT&Y0pZdp^w-kJ~p>Jnvza#ZmHlOxRZ2W$Y7fAbak>E!NWe(pm1 zof?WTdkSLt_gK@l$Ti)!-yt*>DK6S{L9>(&%J~G^fX}q~uGX3lU%16G&hX2%)tPOD z5s!6k{I9BN@7bF90m(16i0|&xMog;~$Z)ke{Dz>axSU~c{j{zb?%P`Jo7t_bGf)|x zDEGLNnE)8E!QJNb(%DM9tCSaG^arI^yk%7GMg)=k-Kg4u)CI6j1koBf1<$^9Y`<8(_!Uc8-Nskebhr6X_RZKV{;w#Znm2quOax z*u=>>EUEnKh^D80W?U*OABa9~fAd~vG-?l<^?0@)yi839q6FgeV?Mq@2Xc=YfpWfm z-@&mRKoY8O^IF~Ls|*;enyGT7xa~HRAPko^SM}R|yRcR(rL!(Y< z63{5zBh82h8y1v3K|6B5G6~S#4}6gSoc$q<9tsXOZc(WxiljafhsT%U?Bn{M(70`2 z!D(22|FeVo0L+4+c)XUok5=>A^xAax+b`w1IzuEaaG%y{#O~@=MtgT?EP|Ho0hJUP zvlIX}0y$2+GK>P<)sBK%6>(hZpJ6_5D6zqgarK+;Pjce_Jr-{XuJ0eWo&`7^-D&E~ z@6r+TI1n{*7EXS65jplMH(b=yO;B923uw?9P)~N|VumjMcwfG?1q1J(#J-S4h}zKd znsC2XCR=mNcyAeXR6u2^7QclP*g)gf1#C6=jSrsb@5)Jt%82tCeF!Wvlq3tz?mh{D zWycT!V4^WD%eQoj(8apBR@sLGJAMZM zNutEk`DIoq9}kytvGLZ09$G>tLauMMvh2;e&yPuy9MsZLm>4sAlOc@ki`oq7*V{<{ z%!F%hhgrP9NZ7JT0VnYWbqYLVG%%f66#%4wx1NPd9L-=NCi(4?7jLPGNh+1Z9#`Qw zjj{F8cGyK>7$)lVHtde0p@@U3g>z+!}1eD|!v z;GTAopEG|#ZppjG)Z%2@7;Q4nJJ@*6EcTq>V=iiLUQrrNyQFH{r3Sexp#H+WXG~Nd zbFzCdjB-=TJpzj#1iG|Sp4?f1$4v2EQ>6~RY?r$c=37EuDI-&k3{#U@K9jP?7BmZ> z4phoA#%nUI6DN05waXPvUDgtUk4W9VoI3R8UVIk5D8necksfyi?M1H^dR=Y6PBk>k z;0617G$biv$_~f2-i4G}ao}c?2T?)JN`zDpP86Rh)-*@ZSKv}4e%z&>28fwVOK!U< z1V?)AXY6kkZ)7}vn0Yp=& zV};PABrFO@CvY#pa+rfRrt^ zgGWSU&g!umV1&93#^L9Uyb#Ct!i7g$1zZ(*;i^m{>IEyw&WjPi+kSIPKL zSml>?qP>CI`$WU~1KoL zpdyTb#i^%`oQiKzT{{q%!D^XoydiKOxGLT|Yqa@6-<@{`fqyzIC9(HyiYkZEZp?O3 zdiW{!gZErdjYkqb9n7c*cU?8Uuz>dXCvo$ctHPk^-QW-Vu3qMXWPjYJ_hPFju=;RE>+J-$k=F9`i3(7d#O>0w>wx zNn=^adi~F|AoSt|(kWt0_{Y)Bdy2aT^#&GvGH=@7F?@^;++Sx(h#FiRr*WkHI@TDX zU@9EEKv&VoI~OI~7yWa5R5J%#A;!<_b)K?A`#>niwEo)cWqVV>u^Fg+&Vo+GaYEm% zJw(AVfA&PO6=%`b*Fe~y?`3*Payu0-|M#^x(NXuR-z69mMwG(WpLy*WC2ePw&LEV92oC}p6kA(voVZHisA=_#c* z+2nln34QQ|$ytbYj?v8hPmC|ss~pArF?)9*!55xY*lk;=G1pj=7vEa;M50sEQ!@tc zFq;t%-`g$c^2a~1KYTkJsu4~}XlRV7&l`TbL8^0>7|izG;CArTy4@Am==uPE4{oXX z?+&BmM1VWUInvZE<}g^y5_v5|Nsxh+^T!dA>a{&bzS1FoVvg<%Xzax#fEZ_7DYMhv z%rq{=g{`go?CZ~mrpVE|Z?(@<8z;({Ggaz8utPGyEs9o}JC?AC2#$bR=4toqNZY~5 zv?yP946|dYRn+Pq`t(~9{*`PR-4+>S`f%cWe{Tu0UfgT!rXbQ8(az`?C5uiWUhWvAN$NWJTA0H { initUserAuth(); initRepoDiffView(); initPdfViewer(); + initGltfViewer(); initScopedAccessTokenCategories(); initColorPickers(); }); diff --git a/web_src/js/render/gltf.js b/web_src/js/render/gltf.js new file mode 100644 index 0000000000..2d48e9f8e6 --- /dev/null +++ b/web_src/js/render/gltf.js @@ -0,0 +1,6 @@ +export async function initGltfViewer() { + const els = document.querySelectorAll('model-viewer'); + if (!els.length) return; + + await import(/* webpackChunkName: "@google/model-viewer" */'@google/model-viewer'); +} From b6dd1dd799ab77ebcdee6e7e2781bd9ae48e2319 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Sat, 21 Jun 2025 18:42:42 +0200 Subject: [PATCH 16/21] Update renovate to v41 (forgejo) (major) (#8253) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8253 Reviewed-by: Michael Kriese Co-authored-by: Renovate Bot Co-committed-by: Renovate Bot --- .forgejo/workflows/renovate.yml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.forgejo/workflows/renovate.yml b/.forgejo/workflows/renovate.yml index 98b93b5757..590eab762e 100644 --- a/.forgejo/workflows/renovate.yml +++ b/.forgejo/workflows/renovate.yml @@ -28,7 +28,7 @@ jobs: runs-on: docker container: - image: data.forgejo.org/renovate/renovate:40.57.1 + image: data.forgejo.org/renovate/renovate:41.1.3 steps: - name: Load renovate repo cache diff --git a/Makefile b/Makefile index 852c85ccd6..f31c1732be 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasour GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go DEADCODE_PACKAGE ?= golang.org/x/tools/cmd/deadcode@v0.34.0 # renovate: datasource=go GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.5.2 # renovate: datasource=go -RENOVATE_NPM_PACKAGE ?= renovate@40.57.1 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate +RENOVATE_NPM_PACKAGE ?= renovate@41.1.3 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate # https://github.com/disposable-email-domains/disposable-email-domains/commits/main/ DISPOSABLE_EMAILS_SHA ?= 0c27e671231d27cf66370034d7f6818037416989 # renovate: ... From d8ad592d4e8aab8f7c7610549ac342c7d951b368 Mon Sep 17 00:00:00 2001 From: John Veness Date: Sun, 22 Jun 2025 08:51:01 +0200 Subject: [PATCH 17/21] Fix sentence structure mentioning cooldown period (#8197) The text should be two sentences, or at the very least separated by a semicolon rather than a comma. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8197 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Earl Warren Co-authored-by: John Veness Co-committed-by: John Veness --- options/locale/locale_en-US.ini | 8 ++++---- tests/integration/user_redirect_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ac841db6d5..0bb128f8b3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -768,8 +768,8 @@ update_profile_success = Your profile has been updated. change_username = Your username has been changed. change_username_prompt = Note: Changing your username also changes your account URL. change_username_redirect_prompt = The old username will redirect until someone claims it. -change_username_redirect_prompt.with_cooldown.one = The old username will be available to everyone after a cooldown period of %[1]d day, you can still reclaim the old username during the cooldown period. -change_username_redirect_prompt.with_cooldown.few = The old username will be available to everyone after a cooldown period of %[1]d days, you can still reclaim the old username during the cooldown period. +change_username_redirect_prompt.with_cooldown.one = The old username will be available to everyone after a cooldown period of %[1]d day. You can still reclaim the old username during the cooldown period. +change_username_redirect_prompt.with_cooldown.few = The old username will be available to everyone after a cooldown period of %[1]d days. You can still reclaim the old username during the cooldown period. continue = Continue cancel = Cancel language = Language @@ -2931,8 +2931,8 @@ settings.update_settings = Update settings settings.update_setting_success = Organization settings have been updated. settings.change_orgname_prompt = Note: Changing the organization name will also change your organization's URL and free the old name. settings.change_orgname_redirect_prompt = The old name will redirect until it is claimed. -settings.change_orgname_redirect_prompt.with_cooldown.one = The old organization name will be available to everyone after a cooldown period of %[1]d day, you can still reclaim the old name during the cooldown period. -settings.change_orgname_redirect_prompt.with_cooldown.few = The old organization name will be available to everyone after a cooldown period of %[1]d days, you can still reclaim the old name during the cooldown period. +settings.change_orgname_redirect_prompt.with_cooldown.one = The old organization name will be available to everyone after a cooldown period of %[1]d day. You can still reclaim the old name during the cooldown period. +settings.change_orgname_redirect_prompt.with_cooldown.few = The old organization name will be available to everyone after a cooldown period of %[1]d days. You can still reclaim the old name during the cooldown period. settings.update_avatar_success = The organization's avatar has been updated. settings.delete = Delete organization settings.delete_account = Delete this organization diff --git a/tests/integration/user_redirect_test.go b/tests/integration/user_redirect_test.go index 8e59699d66..ad17b57713 100644 --- a/tests/integration/user_redirect_test.go +++ b/tests/integration/user_redirect_test.go @@ -143,7 +143,7 @@ func TestUserRedirect(t *testing.T) { defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 8)() defer tests.PrintCurrentTest(t)() - assert.Contains(t, getPrompt(t), "The old username will be available to everyone after a cooldown period of 8 days, you can still reclaim the old username during the cooldown period.") + assert.Contains(t, getPrompt(t), "The old username will be available to everyone after a cooldown period of 8 days. You can still reclaim the old username during the cooldown period.") }) }) @@ -167,7 +167,7 @@ func TestUserRedirect(t *testing.T) { defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 8)() defer tests.PrintCurrentTest(t)() - assert.Contains(t, getPrompt(t), "The old organization name will be available to everyone after a cooldown period of 8 days, you can still reclaim the old name during the cooldown period.") + assert.Contains(t, getPrompt(t), "The old organization name will be available to everyone after a cooldown period of 8 days. You can still reclaim the old name during the cooldown period.") }) }) } From b58cebc2d9c3c4a997d9092e9fba1995e023cd02 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Mon, 23 Jun 2025 07:40:04 +0200 Subject: [PATCH 18/21] Update renovate to v41.1.4 (forgejo) (#8256) Co-authored-by: Renovate Bot Co-committed-by: Renovate Bot --- .forgejo/workflows/renovate.yml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.forgejo/workflows/renovate.yml b/.forgejo/workflows/renovate.yml index 590eab762e..5aa6c8cd98 100644 --- a/.forgejo/workflows/renovate.yml +++ b/.forgejo/workflows/renovate.yml @@ -28,7 +28,7 @@ jobs: runs-on: docker container: - image: data.forgejo.org/renovate/renovate:41.1.3 + image: data.forgejo.org/renovate/renovate:41.1.4 steps: - name: Load renovate repo cache diff --git a/Makefile b/Makefile index f31c1732be..e770f2a989 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasour GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go DEADCODE_PACKAGE ?= golang.org/x/tools/cmd/deadcode@v0.34.0 # renovate: datasource=go GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.5.2 # renovate: datasource=go -RENOVATE_NPM_PACKAGE ?= renovate@41.1.3 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate +RENOVATE_NPM_PACKAGE ?= renovate@41.1.4 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate # https://github.com/disposable-email-domains/disposable-email-domains/commits/main/ DISPOSABLE_EMAILS_SHA ?= 0c27e671231d27cf66370034d7f6818037416989 # renovate: ... From cf4d0e6c34a8ead79540a9f661901d9c4cb0ed8e Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Mon, 23 Jun 2025 07:54:32 +0200 Subject: [PATCH 19/21] bug: unify RepoActionRun and ActionRun structs (#8250) Two pull requests were merged at the same time - https://codeberg.org/forgejo/forgejo/pulls/7699 - https://codeberg.org/forgejo/forgejo/pulls/7508 And added conflicting structs ActionRun modules/structs. That broke the forgejo development branch and a quick fix was made to resolve the name conflict. - https://codeberg.org/forgejo/forgejo/pulls/8066 However that creates an undesirable duplication of two structures that serve the same purpose but are different. - Remove RepoActionRun and replace it with ActionRun - convert.ToActionRun has one more argument, the doer, because it is determined differently in the context of webhooks or API ### Tests - No need because the two pull requests involved already have good coverage. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8250 Reviewed-by: Michael Kriese Reviewed-by: klausfyhn Reviewed-by: Christopher Besch Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- modules/structs/action.go | 6 + modules/structs/repo_actions.go | 20 -- routers/api/v1/repo/action.go | 23 +-- routers/api/v1/swagger/repo.go | 16 +- services/convert/action.go | 9 +- services/convert/convert.go | 23 --- services/webhook/notifier.go | 11 +- templates/swagger/v1_json.tmpl | 209 ++++++++++++++------- tests/integration/api_repo_actions_test.go | 10 +- 9 files changed, 187 insertions(+), 140 deletions(-) diff --git a/modules/structs/action.go b/modules/structs/action.go index 2c42365c19..f47b228d75 100644 --- a/modules/structs/action.go +++ b/modules/structs/action.go @@ -78,3 +78,9 @@ type ActionRun struct { // the url of this action run HTMLURL string `json:"html_url"` } + +// ListActionRunResponse return a list of ActionRun +type ListActionRunResponse struct { + Entries []*ActionRun `json:"workflow_runs"` + TotalCount int64 `json:"total_count"` +} diff --git a/modules/structs/repo_actions.go b/modules/structs/repo_actions.go index 505367336c..b13f344738 100644 --- a/modules/structs/repo_actions.go +++ b/modules/structs/repo_actions.go @@ -32,23 +32,3 @@ type ActionTaskResponse struct { Entries []*ActionTask `json:"workflow_runs"` TotalCount int64 `json:"total_count"` } - -// ActionRun represents an ActionRun -type RepoActionRun struct { - ID int64 `json:"id"` - Name string `json:"name"` - RunNumber int64 `json:"run_number"` - Event string `json:"event"` - Status string `json:"status"` - HeadBranch string `json:"head_branch"` - HeadSHA string `json:"head_sha"` - WorkflowID string `json:"workflow_id"` - URL string `json:"url"` - TriggeringActor *User `json:"triggering_actor"` -} - -// ListActionRunResponse return a list of ActionRun -type ListRepoActionRunResponse struct { - Entries []*RepoActionRun `json:"workflow_runs"` - TotalCount int64 `json:"total_count"` -} diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 03089a18d3..dbc4933de6 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -748,7 +748,7 @@ func ListActionRuns(ctx *context.APIContext) { // type: string // responses: // "200": - // "$ref": "#/responses/RepoActionRunList" + // "$ref": "#/responses/ActionRunList" // "400": // "$ref": "#/responses/error" // "403": @@ -779,16 +779,16 @@ func ListActionRuns(ctx *context.APIContext) { return } - res := new(api.ListRepoActionRunResponse) + res := new(api.ListActionRunResponse) res.TotalCount = total - res.Entries = make([]*api.RepoActionRun, len(runs)) + res.Entries = make([]*api.ActionRun, len(runs)) for i, r := range runs { - cr, err := convert.ToRepoActionRun(ctx, r) - if err != nil { - ctx.Error(http.StatusInternalServerError, "ToActionRun", err) + if err := r.LoadAttributes(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return } + cr := convert.ToActionRun(ctx, r, ctx.Doer) res.Entries[i] = cr } @@ -821,7 +821,7 @@ func GetActionRun(ctx *context.APIContext) { // required: true // responses: // "200": - // "$ref": "#/responses/RepoActionRun" + // "$ref": "#/responses/ActionRun" // "400": // "$ref": "#/responses/error" // "403": @@ -839,16 +839,17 @@ func GetActionRun(ctx *context.APIContext) { return } + // Action runs lives in its own table, therefore we check that the + // run with the requested ID is owned by the repository if ctx.Repo.Repository.ID != run.RepoID { ctx.Error(http.StatusNotFound, "GetRunById", util.ErrNotExist) return } - res, err := convert.ToRepoActionRun(ctx, run) - if err != nil { - ctx.Error(http.StatusInternalServerError, "ToRepoActionRun", err) + if err := run.LoadAttributes(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return } - ctx.JSON(http.StatusOK, res) + ctx.JSON(http.StatusOK, convert.ToActionRun(ctx, run, ctx.Doer)) } diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index bde0efea4e..cd4832e15f 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -463,16 +463,16 @@ type swaggerSyncForkInfo struct { Body []api.SyncForkInfo `json:"body"` } -// RepoActionRunList -// swagger:response RepoActionRunList -type swaggerRepoActionRunList struct { +// ActionRunList +// swagger:response ActionRunList +type swaggerActionRunList struct { // in:body - Body api.ListRepoActionRunResponse `json:"body"` + Body api.ListActionRunResponse `json:"body"` } -// RepoActionRun -// swagger:response RepoActionRun -type swaggerRepoActionRun struct { +// ActionRun +// swagger:response ActionRun +type swaggerActionRun struct { // in:body - Body api.RepoActionRun `json:"body"` + Body api.ActionRun `json:"body"` } diff --git a/services/convert/action.go b/services/convert/action.go index 5e17172b45..703c1f1261 100644 --- a/services/convert/action.go +++ b/services/convert/action.go @@ -8,22 +8,17 @@ import ( actions_model "forgejo.org/models/actions" access_model "forgejo.org/models/perm/access" + user_model "forgejo.org/models/user" api "forgejo.org/modules/structs" ) // ToActionRun convert actions_model.User to api.ActionRun // the run needs all attributes loaded -func ToActionRun(ctx context.Context, run *actions_model.ActionRun) *api.ActionRun { +func ToActionRun(ctx context.Context, run *actions_model.ActionRun, doer *user_model.User) *api.ActionRun { if run == nil { return nil } - // The doer is the one whose perspective is used to view this ActionRun. - // In the best case we use the user that created the webhook. - // Unfortunately we don't know who that was. - // So instead we use the repo owner, who is able to create webhooks and allow others to do so by making them repo admins. - // This is pretty close to perfect. - doer := run.Repo.Owner permissionInRepo, _ := access_model.GetUserRepoPermission(ctx, run.Repo, doer) return &api.ActionRun{ diff --git a/services/convert/convert.go b/services/convert/convert.go index 48da9d7623..2ea24a1b51 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -222,29 +222,6 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action }, nil } -// ToRepoActionRun convert a actions_model.ActionRun to an api.RepoActionRun -func ToRepoActionRun(ctx context.Context, r *actions_model.ActionRun) (*api.RepoActionRun, error) { - if err := r.LoadAttributes(ctx); err != nil { - return nil, err - } - - url := strings.TrimSuffix(setting.AppURL, "/") + r.Link() - actor := ToUser(ctx, r.TriggerUser, nil) - - return &api.RepoActionRun{ - ID: r.ID, - Name: r.Title, - HeadBranch: r.PrettyRef(), - HeadSHA: r.CommitSHA, - RunNumber: r.Index, - Event: r.TriggerEvent, - Status: r.Status.String(), - WorkflowID: r.WorkflowID, - URL: url, - TriggeringActor: actor, - }, nil -} - // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification { verif := asymkey_model.ParseCommitWithSignature(ctx, c) diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index b3201e5d10..009efc994f 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -894,9 +894,16 @@ func (m *webhookNotifier) ActionRunNowDone(ctx context.Context, run *actions_mod Owner: run.TriggerUser, } + // The doer is the one whose perspective is used to view this ActionRun. + // In the best case we use the user that created the webhook. + // Unfortunately we don't know who that was. + // So instead we use the repo owner, who is able to create webhooks and allow others to do so by making them repo admins. + // This is pretty close to perfect. + doer := run.Repo.Owner + payload := &api.ActionPayload{ - Run: convert.ToActionRun(ctx, run), - LastRun: convert.ToActionRun(ctx, lastRun), + Run: convert.ToActionRun(ctx, run, doer), + LastRun: convert.ToActionRun(ctx, lastRun, doer), PriorStatus: priorStatus.String(), } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index bba4ac4949..3ef712c464 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4985,7 +4985,7 @@ ], "responses": { "200": { - "$ref": "#/responses/RepoActionRunList" + "$ref": "#/responses/ActionRunList" }, "400": { "$ref": "#/responses/error" @@ -5032,7 +5032,7 @@ ], "responses": { "200": { - "$ref": "#/responses/RepoActionRun" + "$ref": "#/responses/ActionRun" }, "400": { "$ref": "#/responses/error" @@ -21120,6 +21120,129 @@ }, "x-go-package": "forgejo.org/modules/structs" }, + "ActionRun": { + "description": "ActionRun represents an action run", + "type": "object", + "properties": { + "ScheduleID": { + "description": "the cron id for the schedule trigger", + "type": "integer", + "format": "int64" + }, + "approved_by": { + "description": "who approved this action run", + "type": "integer", + "format": "int64", + "x-go-name": "ApprovedBy" + }, + "commit_sha": { + "description": "the commit sha the action run ran on", + "type": "string", + "x-go-name": "CommitSHA" + }, + "created": { + "description": "when the action run was created", + "type": "string", + "format": "date-time", + "x-go-name": "Created" + }, + "duration": { + "$ref": "#/definitions/Duration" + }, + "event": { + "description": "the webhook event that causes the workflow to run", + "type": "string", + "x-go-name": "Event" + }, + "event_payload": { + "description": "the payload of the webhook event that causes the workflow to run", + "type": "string", + "x-go-name": "EventPayload" + }, + "html_url": { + "description": "the url of this action run", + "type": "string", + "x-go-name": "HTMLURL" + }, + "id": { + "description": "the action run id", + "type": "integer", + "format": "int64", + "x-go-name": "ID" + }, + "index_in_repo": { + "description": "a unique number for each run of a repository", + "type": "integer", + "format": "int64", + "x-go-name": "Index" + }, + "is_fork_pull_request": { + "description": "If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow.", + "type": "boolean", + "x-go-name": "IsForkPullRequest" + }, + "is_ref_deleted": { + "description": "has the commit/tag/… the action run ran on been deleted", + "type": "boolean", + "x-go-name": "IsRefDeleted" + }, + "need_approval": { + "description": "may need approval if it's a fork pull request", + "type": "boolean", + "x-go-name": "NeedApproval" + }, + "prettyref": { + "description": "the commit/tag/… the action run ran on", + "type": "string", + "x-go-name": "PrettyRef" + }, + "repository": { + "$ref": "#/definitions/Repository" + }, + "started": { + "description": "when the action run was started", + "type": "string", + "format": "date-time", + "x-go-name": "Started" + }, + "status": { + "description": "the current status of this run", + "type": "string", + "x-go-name": "Status" + }, + "stopped": { + "description": "when the action run was stopped", + "type": "string", + "format": "date-time", + "x-go-name": "Stopped" + }, + "title": { + "description": "the action run's title", + "type": "string", + "x-go-name": "Title" + }, + "trigger_event": { + "description": "the trigger event defined in the `on` configuration of the triggered workflow", + "type": "string", + "x-go-name": "TriggerEvent" + }, + "trigger_user": { + "$ref": "#/definitions/User" + }, + "updated": { + "description": "when the action run was last updated", + "type": "string", + "format": "date-time", + "x-go-name": "Updated" + }, + "workflow_id": { + "description": "the name of workflow file", + "type": "string", + "x-go-name": "WorkflowID" + } + }, + "x-go-package": "forgejo.org/modules/structs" + }, "ActionRunJob": { "description": "ActionRunJob represents a job of a run", "type": "object", @@ -23610,6 +23733,12 @@ }, "x-go-package": "forgejo.org/modules/structs" }, + "Duration": { + "description": "A Duration represents the elapsed time between two instants\nas an int64 nanosecond count. The representation limits the\nlargest representable duration to approximately 290 years.", + "type": "integer", + "format": "int64", + "x-go-package": "time" + }, "EditAttachmentOptions": { "description": "EditAttachmentOptions options for editing attachments", "type": "object", @@ -25576,7 +25705,7 @@ }, "x-go-package": "forgejo.org/modules/structs" }, - "ListRepoActionRunResponse": { + "ListActionRunResponse": { "description": "ListActionRunResponse return a list of ActionRun", "type": "object", "properties": { @@ -25588,7 +25717,7 @@ "workflow_runs": { "type": "array", "items": { - "$ref": "#/definitions/RepoActionRun" + "$ref": "#/definitions/ActionRun" }, "x-go-name": "Entries" } @@ -27353,54 +27482,6 @@ }, "x-go-package": "forgejo.org/modules/structs" }, - "RepoActionRun": { - "description": "ActionRun represents an ActionRun", - "type": "object", - "properties": { - "event": { - "type": "string", - "x-go-name": "Event" - }, - "head_branch": { - "type": "string", - "x-go-name": "HeadBranch" - }, - "head_sha": { - "type": "string", - "x-go-name": "HeadSHA" - }, - "id": { - "type": "integer", - "format": "int64", - "x-go-name": "ID" - }, - "name": { - "type": "string", - "x-go-name": "Name" - }, - "run_number": { - "type": "integer", - "format": "int64", - "x-go-name": "RunNumber" - }, - "status": { - "type": "string", - "x-go-name": "Status" - }, - "triggering_actor": { - "$ref": "#/definitions/User" - }, - "url": { - "type": "string", - "x-go-name": "URL" - }, - "workflow_id": { - "type": "string", - "x-go-name": "WorkflowID" - } - }, - "x-go-package": "forgejo.org/modules/structs" - }, "RepoCollaboratorPermission": { "description": "RepoCollaboratorPermission to get repository permission for a collaborator", "type": "object", @@ -28847,6 +28928,18 @@ } } }, + "ActionRun": { + "description": "ActionRun", + "schema": { + "$ref": "#/definitions/ActionRun" + } + }, + "ActionRunList": { + "description": "ActionRunList", + "schema": { + "$ref": "#/definitions/ListActionRunResponse" + } + }, "ActionVariable": { "description": "ActionVariable", "schema": { @@ -29616,18 +29709,6 @@ } } }, - "RepoActionRun": { - "description": "RepoActionRun", - "schema": { - "$ref": "#/definitions/RepoActionRun" - } - }, - "RepoActionRunList": { - "description": "RepoActionRunList", - "schema": { - "$ref": "#/definitions/ListRepoActionRunResponse" - } - }, "RepoCollaboratorPermission": { "description": "RepoCollaboratorPermission", "schema": { diff --git a/tests/integration/api_repo_actions_test.go b/tests/integration/api_repo_actions_test.go index 34d603487c..af2aa10cfd 100644 --- a/tests/integration/api_repo_actions_test.go +++ b/tests/integration/api_repo_actions_test.go @@ -169,7 +169,7 @@ func TestAPIGetListActionRun(t *testing.T) { req.AddTokenAuth(token) res := MakeRequest(t, req, http.StatusOK) - apiRuns := new(api.ListRepoActionRunResponse) + apiRuns := new(api.ListActionRunResponse) DecodeJSON(t, res, apiRuns) assert.Equal(t, int64(len(tt.expectedIDs)), apiRuns.TotalCount) @@ -231,13 +231,13 @@ func TestAPIGetActionRun(t *testing.T) { } dbRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: tt.runID}) - apiRun := new(api.RepoActionRun) + apiRun := new(api.ActionRun) DecodeJSON(t, res, apiRun) - assert.Equal(t, dbRun.Index, apiRun.RunNumber) + assert.Equal(t, dbRun.Index, apiRun.Index) assert.Equal(t, dbRun.Status.String(), apiRun.Status) - assert.Equal(t, dbRun.CommitSHA, apiRun.HeadSHA) - assert.Equal(t, dbRun.TriggerUserID, apiRun.TriggeringActor.ID) + assert.Equal(t, dbRun.CommitSHA, apiRun.CommitSHA) + assert.Equal(t, dbRun.TriggerUserID, apiRun.TriggerUser.ID) }) } } From debd74e1b688f02293848db89948f5be6b12740a Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Mon, 23 Jun 2025 08:00:18 +0200 Subject: [PATCH 20/21] fix: add an index to the ActionRun.stopped column (#8252) The models/actions/run.go:GetRunBefore function sorts ActionRun rows to get the most recently stopped. Since the ActionRun rows do not expire, the cost will keep increasing over time. The index is meant to ensure the execution time of the associated query does not grow linearly with the number of rows in the ActionRun table. Ref https://codeberg.org/forgejo/forgejo/pulls/7491/files#issuecomment-5495441 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8252 Reviewed-by: Christopher Besch Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- models/forgejo_migrations/migrate.go | 2 ++ models/forgejo_migrations/v35.go | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 models/forgejo_migrations/v35.go diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 6a6922ec4e..737350b019 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -107,6 +107,8 @@ var migrations = []*Migration{ NewMigration("Add federated user activity tables, update the `federated_user` table & add indexes", FederatedUserActivityMigration), // v33 -> v34 NewMigration("Add `notify-email` column to `action_run` table", AddNotifyEmailToActionRun), + // v34 -> v35 + NewMigration("Add index to `stopped` column in `action_run` table", AddIndexToActionRunStopped), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v35.go b/models/forgejo_migrations/v35.go new file mode 100644 index 0000000000..0fb3b43e2c --- /dev/null +++ b/models/forgejo_migrations/v35.go @@ -0,0 +1,19 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations //nolint:revive + +import ( + "forgejo.org/modules/timeutil" + + "xorm.io/xorm" +) + +func AddIndexToActionRunStopped(x *xorm.Engine) error { + type ActionRun struct { + ID int64 + Stopped timeutil.TimeStamp `xorm:"index"` + } + + return x.Sync(&ActionRun{}) +} From 39e6785da00ddd3f1f01f846aab8bf0a5c31f96b Mon Sep 17 00:00:00 2001 From: Danko Aleksejevs Date: Mon, 23 Jun 2025 15:21:15 +0200 Subject: [PATCH 21/21] fix(ui): erroneous list continuation on Cmd+Enter on macOS (#8153) (#8170) The line continuation code in the Markdown editor ignored Enter presses if Ctrl, Alt or Shift were being held. This now also accounts for Cmd on macOS (which browsers represent as metaKey). ### Tests - Use Safari (on macOS) - create a new issue in a repository - start writing a list (with - one[enter]- two) - now press Cmd+Enter - verify that while the form is being submitted, no new line got visually added *** The visual evidence of this bug is a race condition (form is being edited while also being submitted) and I don't think a reliable cross-browser E2E test is possible. Bugged and fixed behavior verified manually on an actual Macbook in current Safari. Specifically, * Before the fix: pressing Cmd+Enter submits the form *and* inserts a newline + list prefix in the markdown editor. Reporter had the changed content submitted, I only saw it submit the original (but the change was visible during submission). * After the fix: Cmd+Enter only submits the form. Enter with no modifiers inserts newlines/prefixes as before. ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8170 Reviewed-by: Beowulf Co-authored-by: Danko Aleksejevs Co-committed-by: Danko Aleksejevs --- web_src/js/features/comp/ComboMarkdownEditor.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index 5f452b3802..3c94274175 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -121,12 +121,12 @@ class ComboMarkdownEditor { // Prevent special keyboard handling if currently a text expander popup is open if (this.textarea.hasAttribute('aria-expanded')) return; - const noModifiers = !e.shiftKey && !e.ctrlKey && !e.altKey; + const noModifiers = !e.shiftKey && !e.ctrlKey && !e.altKey && !e.metaKey; if (e.key === 'Escape') { // Explicitly lose focus and reenable tab navigation. e.target.blur(); this.tabEnabled = false; - } else if (e.key === 'Tab' && this.tabEnabled && !e.altKey && !e.ctrlKey) { + } else if (e.key === 'Tab' && this.tabEnabled && !e.altKey && !e.ctrlKey && !e.metaKey) { if (this.indentSelection(e.shiftKey, true)) { this.options?.onContentChanged?.(this, e); e.preventDefault();