From e12a3dd81823cb6f4e5fc78bd19ddf7bbfb603d4 Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:30:48 -0400 Subject: [PATCH] fix(router/scm)!: change HTTP method from GET to PATCH for sync endpoints (#994) * fix(router/scm): change HTTP method from GET to PATCH for sync endpoints * update mock --- api/scm/sync.go | 14 +++++--------- api/scm/sync_org.go | 14 +++++--------- mock/server/scm.go | 4 ++-- mock/server/server.go | 4 ++-- router/scm.go | 8 ++++---- 5 files changed, 18 insertions(+), 26 deletions(-) diff --git a/api/scm/sync.go b/api/scm/sync.go index 045c0a1b6..f268c9a92 100644 --- a/api/scm/sync.go +++ b/api/scm/sync.go @@ -17,7 +17,7 @@ import ( "github.com/sirupsen/logrus" ) -// swagger:operation GET /api/v1/scm/repos/{org}/{repo}/sync scm SyncRepo +// swagger:operation PATCH /api/v1/scm/repos/{org}/{repo}/sync scm SyncRepo // // Sync up scm service and database in the context of a specific repo // @@ -125,11 +125,9 @@ func SyncRepo(c *gin.Context) { // update webhook webhookExists, err := scm.FromContext(c).Update(ctx, u, r, lastHook.GetWebhookID()) if err != nil { - // if webhook has been manually deleted from GitHub, // set to inactive in database if !webhookExists { - r.SetActive(false) _, err := database.FromContext(c).UpdateRepo(ctx, r) @@ -144,15 +142,13 @@ func SyncRepo(c *gin.Context) { c.JSON(http.StatusOK, fmt.Sprintf("webhook not found, repo %s deactivated", r.GetFullName())) return + } - } else { - - retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err) + retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err) - util.HandleError(c, http.StatusInternalServerError, retErr) + util.HandleError(c, http.StatusInternalServerError, retErr) - return - } + return } } diff --git a/api/scm/sync_org.go b/api/scm/sync_org.go index c0b2e08fa..bf7607533 100644 --- a/api/scm/sync_org.go +++ b/api/scm/sync_org.go @@ -16,7 +16,7 @@ import ( "github.com/sirupsen/logrus" ) -// swagger:operation GET /api/v1/scm/orgs/{org}/sync scm SyncReposForOrg +// swagger:operation PATCH /api/v1/scm/orgs/{org}/sync scm SyncReposForOrg // // Sync up repos from scm service and database in a specified org // @@ -138,11 +138,9 @@ func SyncReposForOrg(c *gin.Context) { // update webhook webhookExists, err := scm.FromContext(c).Update(ctx, u, repo, lastHook.GetWebhookID()) if err != nil { - // if webhook has been manually deleted from GitHub, // set to inactive in database if !webhookExists { - repo.SetActive(false) _, err := database.FromContext(c).UpdateRepo(ctx, repo) @@ -155,15 +153,13 @@ func SyncReposForOrg(c *gin.Context) { } continue + } - } else { - - retErr := fmt.Errorf("unable to update repo webhook for %s: %w", repo.GetFullName(), err) + retErr := fmt.Errorf("unable to update repo webhook for %s: %w", repo.GetFullName(), err) - util.HandleError(c, http.StatusInternalServerError, retErr) + util.HandleError(c, http.StatusInternalServerError, retErr) - return - } + return } } } diff --git a/mock/server/scm.go b/mock/server/scm.go index 33021bdde..0dea97c37 100644 --- a/mock/server/scm.go +++ b/mock/server/scm.go @@ -11,7 +11,7 @@ import ( "github.com/go-vela/types" ) -// syncRepo has a param :repo returns mock JSON for a http GET. +// syncRepo has a param :repo returns mock JSON for a http PATCH. // // Pass "not-found" to :repo to test receiving a http 404 response. func syncRepo(c *gin.Context) { @@ -27,7 +27,7 @@ func syncRepo(c *gin.Context) { c.JSON(http.StatusOK, fmt.Sprintf("Repo %s has been synced", r)) } -// syncRepos has a param :org returns mock JSON for a http GET. +// syncRepos has a param :org returns mock JSON for a http PATCH. // // Pass "not-found" to :repo to test receiving a http 404 response. func syncRepos(c *gin.Context) { diff --git a/mock/server/server.go b/mock/server/server.go index 14734e928..699062868 100644 --- a/mock/server/server.go +++ b/mock/server/server.go @@ -83,8 +83,8 @@ func FakeHandler() http.Handler { e.DELETE("/api/v1/repos/:org/:repo", removeRepo) e.PATCH("/api/v1/repos/:org/:repo/repair", repairRepo) e.PATCH("/api/v1/repos/:org/:repo/chown", chownRepo) - e.GET("/api/v1/scm/repos/:org/:repo/sync", syncRepo) - e.GET("/api/v1/scm/orgs/:org/sync", syncRepos) + e.PATCH("/api/v1/scm/repos/:org/:repo/sync", syncRepo) + e.PATCH("/api/v1/scm/orgs/:org/sync", syncRepos) // mock endpoints for secret calls e.GET("/api/v1/secrets/:engine/:type/:org/:name/:secret", getSecret) diff --git a/router/scm.go b/router/scm.go index a8ce5745b..10c1cea81 100644 --- a/router/scm.go +++ b/router/scm.go @@ -12,8 +12,8 @@ import ( // ScmHandlers is a function that extends the provided base router group // with the API handlers for source code management functionality. // -// GET /api/v1/scm/orgs/:org/sync -// GET /api/v1/scm/repos/:org/:repo/sync . +// PATCH /api/v1/scm/orgs/:org/sync +// PATCH /api/v1/scm/repos/:org/:repo/sync . func ScmHandlers(base *gin.RouterGroup) { // SCM orgs endpoints orgs := base.Group("/scm/orgs") @@ -21,7 +21,7 @@ func ScmHandlers(base *gin.RouterGroup) { // SCM org endpoints org := orgs.Group("/:org", org.Establish()) { - org.GET("/sync", scm.SyncReposForOrg) + org.PATCH("/sync", scm.SyncReposForOrg) } // end of SCM org endpoints } // end of SCM orgs endpoints @@ -31,7 +31,7 @@ func ScmHandlers(base *gin.RouterGroup) { // SCM repo endpoints repo := repos.Group("/:org/:repo", org.Establish(), repo.Establish()) { - repo.GET("/sync", scm.SyncRepo) + repo.PATCH("/sync", scm.SyncRepo) } // end of SCM repo endpoints } // end of SCM repos endpoints }