diff --git a/models/webhook/fixtures/TestListDefaultWebhooks/webhook.yml b/models/webhook/fixtures/TestListDefaultWebhooks/webhook.yml new file mode 100644 index 0000000000..403b11ff7e --- /dev/null +++ b/models/webhook/fixtures/TestListDefaultWebhooks/webhook.yml @@ -0,0 +1,66 @@ +- + id: 5 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook5 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: false +- + id: 6 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook6 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: false +- + id: 7 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook7 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: false +- + id: 8 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook8 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: false +- + id: 9 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook9 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: false +- + id: 10 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook10 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: true diff --git a/models/webhook/fixtures/TestListSystemWebhooks/webhook.yml b/models/webhook/fixtures/TestListSystemWebhooks/webhook.yml new file mode 100644 index 0000000000..8858141f5e --- /dev/null +++ b/models/webhook/fixtures/TestListSystemWebhooks/webhook.yml @@ -0,0 +1,66 @@ +- + id: 5 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook5 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: true +- + id: 6 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook6 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: true +- + id: 7 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook7 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: true + is_system_webhook: true +- + id: 8 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook8 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: true +- + id: 9 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook9 + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: true +- + id: 10 + repo_id: 0 + owner_id: 0 + url: http://www.example.com/systemhook + http_method: POST + type: forgejo + content_type: 1 # json + events: '{"push_only":true,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":false}}' + is_active: false + is_system_webhook: false diff --git a/models/webhook/webhook_system.go b/models/webhook/webhook_system.go index b63346635c..2e53f639dc 100644 --- a/models/webhook/webhook_system.go +++ b/models/webhook/webhook_system.go @@ -11,8 +11,8 @@ import ( ) // GetDefaultWebhooks returns all admin-default webhooks. -func GetDefaultWebhooks(ctx context.Context) ([]*Webhook, error) { - return getAdminWebhooks(ctx, false) +func GetDefaultWebhooks(ctx context.Context) ([]*Webhook, int64, error) { + return getAdminWebhooks(ctx, false, db.ListOptions{ListAll: true}) } // GetSystemOrDefaultWebhook returns admin system or default webhook by given ID. @@ -30,22 +30,24 @@ func GetSystemOrDefaultWebhook(ctx context.Context, id int64) (*Webhook, error) } // GetSystemWebhooks returns all admin system webhooks. -func GetSystemWebhooks(ctx context.Context, onlyActive bool) ([]*Webhook, error) { - return getAdminWebhooks(ctx, true, onlyActive) +func GetSystemWebhooks(ctx context.Context, listOptions db.ListOptions, onlyActive bool) ([]*Webhook, int64, error) { + return getAdminWebhooks(ctx, true, listOptions, onlyActive) } -func getAdminWebhooks(ctx context.Context, systemWebhooks bool, onlyActive ...bool) ([]*Webhook, error) { +func getAdminWebhooks(ctx context.Context, systemWebhooks bool, listOptions db.ListOptions, onlyActive ...bool) ([]*Webhook, int64, error) { webhooks := make([]*Webhook, 0, 5) + sess := db.GetEngine(ctx). + Where("repo_id=?", 0). + And("owner_id=?", 0). + And("is_system_webhook=?", systemWebhooks) if len(onlyActive) > 0 && onlyActive[0] { - return webhooks, db.GetEngine(ctx). - Where("repo_id=? AND owner_id=? AND is_system_webhook=? AND is_active = ?", 0, 0, systemWebhooks, true). - OrderBy("id"). - Find(&webhooks) + sess = sess.And("is_active=?", true) } - return webhooks, db.GetEngine(ctx). - Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, systemWebhooks). - OrderBy("id"). - Find(&webhooks) + if listOptions.Page > 0 { + sess = db.SetSessionPagination(sess, &listOptions) + } + total, err := sess.OrderBy("id").FindAndCount(&webhooks) + return webhooks, total, err } // DeleteDefaultSystemWebhook deletes an admin-configured default or system webhook (where Org and Repo ID both 0) @@ -67,7 +69,7 @@ func DeleteDefaultSystemWebhook(ctx context.Context, id int64) error { // CopyDefaultWebhooksToRepo creates copies of the default webhooks in a new repo func CopyDefaultWebhooksToRepo(ctx context.Context, repoID int64) error { - ws, err := GetDefaultWebhooks(ctx) + ws, _, err := GetDefaultWebhooks(ctx) if err != nil { return fmt.Errorf("GetDefaultWebhooks: %v", err) } diff --git a/models/webhook/webhook_system_test.go b/models/webhook/webhook_system_test.go new file mode 100644 index 0000000000..1938c8d7c1 --- /dev/null +++ b/models/webhook/webhook_system_test.go @@ -0,0 +1,65 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package webhook + +import ( + "testing" + + "forgejo.org/models/db" + "forgejo.org/models/unittest" + + "github.com/stretchr/testify/require" + "xorm.io/builder" +) + +func TestListSystemWebhooks(t *testing.T) { + defer unittest.OverrideFixtures("models/webhook/fixtures/TestListSystemWebhooks")() + require.NoError(t, unittest.PrepareTestDatabase()) + testGetAdminWebhooks(t, true) +} + +func TestListDefaultWebhooks(t *testing.T) { + defer unittest.OverrideFixtures("models/webhook/fixtures/TestListDefaultWebhooks")() + require.NoError(t, unittest.PrepareTestDatabase()) + testGetAdminWebhooks(t, false) +} + +func testGetAdminWebhooks(t *testing.T, systemHooks bool) { + hookCond := builder.Eq{"is_system_webhook": systemHooks}.And(builder.Eq{"repo_id": 0}) + unittest.AssertCountByCond(t, "webhook", hookCond, 5) + + t.Run("All hooks (Implicit)", func(t *testing.T) { + hooks, count, err := getAdminWebhooks(t.Context(), systemHooks, db.ListOptionsAll) + require.NoError(t, err) + unittest.AssertCountByCond(t, "webhook", hookCond, int(count)) + require.Len(t, hooks, int(count)) + require.Equal(t, 5, int(count)) + for i := range hooks { + require.Equal(t, systemHooks, hooks[i].IsSystemWebhook) + } + }) + + t.Run("All hooks (Explicit)", func(t *testing.T) { + hooks, count, err := getAdminWebhooks(t.Context(), systemHooks, db.ListOptionsAll, false) + require.NoError(t, err) + unittest.AssertCountByCond(t, "webhook", hookCond, int(count)) + require.Len(t, hooks, int(count)) + require.Equal(t, 5, int(count)) + for i := range hooks { + require.Equal(t, systemHooks, hooks[i].IsSystemWebhook) + } + }) + + t.Run("Active hooks", func(t *testing.T) { + hooks, count, err := getAdminWebhooks(t.Context(), systemHooks, db.ListOptionsAll, true) + require.NoError(t, err) + unittest.AssertCountByCond(t, "webhook", hookCond.And(builder.Eq{"is_active": true}), int(count)) + require.Len(t, hooks, int(count)) + require.Equal(t, 3, int(count)) + for i := range hooks { + require.Equal(t, systemHooks, hooks[i].IsSystemWebhook) + require.True(t, hooks[i].IsActive) + } + }) +} diff --git a/routers/api/v1/admin/hooks.go b/routers/api/v1/admin/hooks.go index 36ca6831e6..89977f6c6e 100644 --- a/routers/api/v1/admin/hooks.go +++ b/routers/api/v1/admin/hooks.go @@ -37,7 +37,7 @@ func ListHooks(ctx *context.APIContext) { // "200": // "$ref": "#/responses/HookList" - sysHooks, err := webhook.GetSystemWebhooks(ctx, false) + sysHooks, total, err := webhook.GetSystemWebhooks(ctx, utils.GetListOptions(ctx), false) if err != nil { ctx.Error(http.StatusInternalServerError, "GetSystemWebhooks", err) return @@ -51,6 +51,7 @@ func ListHooks(ctx *context.APIContext) { } hooks[i] = h } + ctx.SetTotalCountHeader(total) ctx.JSON(http.StatusOK, hooks) } diff --git a/routers/web/admin/hooks.go b/routers/web/admin/hooks.go index aeceffe848..c8dff64729 100644 --- a/routers/web/admin/hooks.go +++ b/routers/web/admin/hooks.go @@ -6,6 +6,7 @@ package admin import ( "net/http" + "forgejo.org/models/db" "forgejo.org/models/webhook" "forgejo.org/modules/base" "forgejo.org/modules/setting" @@ -35,7 +36,7 @@ func DefaultOrSystemWebhooks(ctx *context.Context) { sys["Title"] = ctx.Tr("admin.systemhooks") sys["Description"] = ctx.Tr("admin.systemhooks.desc", "https://forgejo.org/docs/latest/user/webhooks/") - sys["Webhooks"], err = webhook.GetSystemWebhooks(ctx, false) + sys["Webhooks"], _, err = webhook.GetSystemWebhooks(ctx, db.ListOptions{ListAll: true}, false) sys["BaseLink"] = setting.AppSubURL + "/admin/hooks" sys["BaseLinkNew"] = setting.AppSubURL + "/admin/system-hooks" sys["WebhookList"] = webhook_service.List() @@ -46,7 +47,7 @@ func DefaultOrSystemWebhooks(ctx *context.Context) { def["Title"] = ctx.Tr("admin.defaulthooks") def["Description"] = ctx.Tr("admin.defaulthooks.desc", "https://forgejo.org/docs/latest/user/webhooks/") - def["Webhooks"], err = webhook.GetDefaultWebhooks(ctx) + def["Webhooks"], _, err = webhook.GetDefaultWebhooks(ctx) def["BaseLink"] = setting.AppSubURL + "/admin/hooks" def["BaseLinkNew"] = setting.AppSubURL + "/admin/default-hooks" def["WebhookList"] = webhook_service.List() diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index ecbbfcfbd6..93a863ec49 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -241,7 +241,7 @@ func PrepareWebhooks(ctx context.Context, source EventSource, event webhook_modu } // Add any admin-defined system webhooks - systemHooks, err := webhook_model.GetSystemWebhooks(ctx, true) + systemHooks, _, err := webhook_model.GetSystemWebhooks(ctx, db.ListOptions{ListAll: true}, true) if err != nil { return fmt.Errorf("GetSystemWebhooks: %w", err) } diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 7f91ac3874..5a61ef037b 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -541,3 +541,29 @@ func TestAPIAdminDeleteUserEmailsMultiple(t *testing.T) { assert.Equal(t, "user1@example.com", remainingEmails[0].Email, "Only primary email should remain") assert.True(t, remainingEmails[0].Primary, "Remaining email should be primary") } + +func TestAPIAdminListHooksPagination(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + token := getUserToken(t, "user1", auth_model.AccessTokenScopeWriteAdmin) + + for i := range 20 { + req := NewRequestWithJSON(t, "POST", "/api/v1/admin/hooks", api.CreateHookOption{ + Config: api.CreateHookOptionConfig{ + "url": fmt.Sprintf("http://localhost/hook-%d", i), + "content_type": "json", + "is_system_webhook": "true", + }, + Type: "forgejo", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + } + + req := NewRequest(t, "GET", "/api/v1/admin/hooks?page=1&limit=10").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var hooksList []api.Hook + t.Logf("got response %s", resp.Body) + DecodeJSON(t, resp, &hooksList) + assert.Len(t, hooksList, 10, "page length should equal `limit` param") + assert.Equal(t, "20", resp.Header().Get("X-Total-Count")) +}