From f883715638e3a445f233045f41ea7487a8f680cb Mon Sep 17 00:00:00 2001 From: Leni Kadali Date: Sat, 8 Nov 2025 04:20:05 +0100 Subject: [PATCH] chore: Remove IsDeleted from action (activity) table (#9829) Fixes [#3525](https://codeberg.org/forgejo/forgejo/issues/3525) and supersedes [#9586](https://codeberg.org/forgejo/forgejo/pulls/9586) ## 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. - [ ] 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. - [ ] 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. ## Release notes - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/9829): chore: Remove IsDeleted from action (activity) table Co-authored-by: Mathieu Fenniak Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9829 Reviewed-by: Mathieu Fenniak Co-authored-by: Leni Kadali Co-committed-by: Leni Kadali --- models/activities/action.go | 17 ++--- models/activities/action_test.go | 3 - models/activities/user_heatmap.go | 1 - models/activities/user_heatmap_test.go | 1 - ...leted-column-from-activity-action-table.go | 34 ++++++++++ ...-column-from-activity-action-table_test.go | 63 +++++++++++++++++++ models/issues/comment.go | 6 +- routers/web/feed/profile.go | 1 - routers/web/user/home.go | 1 - routers/web/user/profile.go | 1 - 10 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table.go create mode 100644 models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table_test.go diff --git a/models/activities/action.go b/models/activities/action.go index 5067109545..7ac050d79f 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -154,7 +154,6 @@ type Action struct { CommentID int64 `xorm:"INDEX"` Comment *issues_model.Comment `xorm:"-"` Issue *issues_model.Issue `xorm:"-"` // get the issue id from content - IsDeleted bool `xorm:"NOT NULL DEFAULT false"` RefName string IsPrivate bool `xorm:"NOT NULL DEFAULT false"` Content string `xorm:"TEXT"` @@ -167,14 +166,14 @@ func init() { // TableIndices implements xorm's TableIndices interface func (a *Action) TableIndices() []*schemas.Index { - repoIndex := schemas.NewIndex("r_u_d", schemas.IndexType) - repoIndex.AddColumn("repo_id", "user_id", "is_deleted") + repoIndex := schemas.NewIndex("r_u", schemas.IndexType) + repoIndex.AddColumn("repo_id", "user_id") - actUserIndex := schemas.NewIndex("au_r_c_u_d", schemas.IndexType) - actUserIndex.AddColumn("act_user_id", "repo_id", "created_unix", "user_id", "is_deleted") + actUserIndex := schemas.NewIndex("au_r_c_u", schemas.IndexType) + actUserIndex.AddColumn("act_user_id", "repo_id", "created_unix", "user_id") - cudIndex := schemas.NewIndex("c_u_d", schemas.IndexType) - cudIndex.AddColumn("created_unix", "user_id", "is_deleted") + cudIndex := schemas.NewIndex("c_u", schemas.IndexType) + cudIndex.AddColumn("created_unix", "user_id") indices := []*schemas.Index{actUserIndex, repoIndex, cudIndex} @@ -472,7 +471,6 @@ type GetFeedsOptions struct { IncludePrivate bool // include private actions OnlyPerformedBy bool // only actions performed by requested user OnlyPerformedByActor bool // only actions performed by the original actor - IncludeDeleted bool // include deleted actions Date string // the day we want activity for: YYYY-MM-DD } @@ -588,9 +586,6 @@ func activityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder. if !opts.IncludePrivate { cond = cond.And(builder.Eq{"`action`.is_private": false}) } - if !opts.IncludeDeleted { - cond = cond.And(builder.Eq{"is_deleted": false}) - } if opts.Date != "" { dateLow, err := time.ParseInLocation("2006-01-02", opts.Date, setting.DefaultUILocation) diff --git a/models/activities/action_test.go b/models/activities/action_test.go index 6911733a09..bb72f4ed47 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -51,7 +51,6 @@ func TestGetFeeds(t *testing.T) { Actor: user, IncludePrivate: true, OnlyPerformedBy: false, - IncludeDeleted: true, }) require.NoError(t, err) if assert.Len(t, actions, 1) { @@ -127,7 +126,6 @@ func TestGetFeeds2(t *testing.T) { Actor: user, IncludePrivate: true, OnlyPerformedBy: false, - IncludeDeleted: true, }) require.NoError(t, err) assert.Len(t, actions, 1) @@ -142,7 +140,6 @@ func TestGetFeeds2(t *testing.T) { Actor: user, IncludePrivate: false, OnlyPerformedBy: false, - IncludeDeleted: true, }) require.NoError(t, err) assert.Empty(t, actions) diff --git a/models/activities/user_heatmap.go b/models/activities/user_heatmap.go index 11badb77e2..970b05f0ae 100644 --- a/models/activities/user_heatmap.go +++ b/models/activities/user_heatmap.go @@ -54,7 +54,6 @@ func getUserHeatmapData(ctx context.Context, user *user_model.User, team *organi RequestedTeam: team, Actor: doer, IncludePrivate: true, // don't filter by private, as we already filter by repo access - IncludeDeleted: true, // * Heatmaps for individual users only include actions that the user themself did. // * For organizations actions by all users that were made in owned // repositories are counted. diff --git a/models/activities/user_heatmap_test.go b/models/activities/user_heatmap_test.go index 34308cb3d4..63de0b2f0d 100644 --- a/models/activities/user_heatmap_test.go +++ b/models/activities/user_heatmap_test.go @@ -82,7 +82,6 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { Actor: doer, IncludePrivate: true, OnlyPerformedBy: true, - IncludeDeleted: true, }) require.NoError(t, err) diff --git a/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table.go b/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table.go new file mode 100644 index 0000000000..2eff0014cd --- /dev/null +++ b/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table.go @@ -0,0 +1,34 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "forgejo.org/models/gitea_migrations/base" + + "xorm.io/xorm" +) + +func init() { + registerMigration(&Migration{ + Description: "remove is_deleted column from activity action table", + Upgrade: removeIsDeletedColumnFromActivityActionTable, + }) +} + +func removeIsDeletedColumnFromActivityActionTable(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if _, err := sess.Table("action").Where("is_deleted = ?", true).Delete(); err != nil { + return err + } + + if err := base.DropTableColumns(sess, "action", "is_deleted"); err != nil { + return err + } + return sess.Commit() +} diff --git a/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table_test.go b/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table_test.go new file mode 100644 index 0000000000..8383c238d6 --- /dev/null +++ b/models/forgejo_migrations/v14a_remove-is-deleted-column-from-activity-action-table_test.go @@ -0,0 +1,63 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "testing" + + migration_tests "forgejo.org/models/gitea_migrations/test" + "forgejo.org/modules/timeutil" + + "github.com/stretchr/testify/require" + "xorm.io/xorm/schemas" +) + +func Test_removeIsDeletedColumnFromActivityActionTable(t *testing.T) { + type Action struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + ActUserID int64 // Action user id. + RepoID int64 + CommentID int64 `xorm:"INDEX"` + IsDeleted bool `xorm:"NOT NULL DEFAULT false"` + RefName string + IsPrivate bool `xorm:"NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` + } + + // Prepare TestEnv + x, deferable := migration_tests.PrepareTestEnv(t, 0, + new(Action), + ) + defer deferable() + if x == nil || t.Failed() { + return + } + + // test for expected results + getColumn := func(tn, co string) *schemas.Column { + tables, err := x.DBMetas() + require.NoError(t, err) + var table *schemas.Table + for _, elem := range tables { + if elem.Name == tn { + table = elem + break + } + } + return table.GetColumn(co) + } + + require.NotNil(t, getColumn("action", "is_deleted")) + _, err := x.Table("action").Count() + require.NoError(t, err) + + require.NoError(t, removeIsDeletedColumnFromActivityActionTable(x)) + + require.Nil(t, getColumn("action", "is_deleted")) + cnt2, err := x.Table("action").Count() + require.NoError(t, err) + require.Equal(t, int64(0), cnt2) +} diff --git a/models/issues/comment.go b/models/issues/comment.go index 6afd1623f3..06d35bb2d4 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -1215,11 +1215,9 @@ func DeleteComment(ctx context.Context, comment *Comment) error { return err } } + if _, err := e.Table("action"). - Where("comment_id = ?", comment.ID). - Update(map[string]any{ - "is_deleted": true, - }); err != nil { + Where("comment_id = ?", comment.ID).Delete(); err != nil { return err } diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index dd2fec186f..0c11a15886 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -33,7 +33,6 @@ func showUserFeed(ctx *context.Context, formatType string) { Actor: ctx.Doer, IncludePrivate: includePrivate, OnlyPerformedBy: !ctx.ContextUser.IsOrganization(), - IncludeDeleted: false, Date: ctx.FormString("date"), }) if err != nil { diff --git a/routers/web/user/home.go b/routers/web/user/home.go index d980fa393a..2f0eadf93a 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -119,7 +119,6 @@ func Dashboard(ctx *context.Context) { Actor: ctx.Doer, IncludePrivate: true, OnlyPerformedBy: false, - IncludeDeleted: false, Date: ctx.FormString("date"), ListOptions: db.ListOptions{ Page: page, diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 02cea05a47..c236c66dc5 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -195,7 +195,6 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb Actor: ctx.Doer, IncludePrivate: showPrivate, OnlyPerformedBy: true, - IncludeDeleted: false, Date: date, ListOptions: db.ListOptions{ PageSize: pagingNum,