From 3997d686a0bd6f9311e355c708de30c872d9029c Mon Sep 17 00:00:00 2001 From: David May <49894298+wass3rw3rk@users.noreply.github.com> Date: Tue, 14 Jan 2025 15:23:06 -0600 Subject: [PATCH] fix(build): ensure count/pagination links account for filter/query params (#1242) --- api/build/compile_publish.go | 2 +- api/pagination.go | 15 ++++++++++++--- api/webhook/post.go | 2 +- database/build/count_repo.go | 4 +++- database/build/count_repo_test.go | 7 +++++-- database/build/interface.go | 2 +- database/build/list_repo.go | 2 +- database/build/list_repo_test.go | 2 +- database/integration_test.go | 2 +- 9 files changed, 26 insertions(+), 12 deletions(-) diff --git a/api/build/compile_publish.go b/api/build/compile_publish.go index 7167d1357..ce1aacacd 100644 --- a/api/build/compile_publish.go +++ b/api/build/compile_publish.go @@ -119,7 +119,7 @@ func CompileAndPublish( } // send API call to capture the number of pending or running builds for the repo - builds, err := database.CountBuildsForRepo(ctx, r, filters) + builds, err := database.CountBuildsForRepo(ctx, r, filters, time.Now().Unix(), 0) if err != nil { retErr := fmt.Errorf("%s: unable to get count of builds for repo %s", baseErr, r.GetFullName()) diff --git a/api/pagination.go b/api/pagination.go index 6abb3f3d9..126b36372 100644 --- a/api/pagination.go +++ b/api/pagination.go @@ -30,6 +30,9 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { l := []string{} r := c.Request + // grab the current query params + q := r.URL.Query() + hl := HeaderLink{ "first": 1, "last": p.TotalPages(), @@ -42,6 +45,9 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { return } + // reset per config + q.Set("per_page", strconv.Itoa(p.PerPage)) + // drop first, prev on the first page if p.Page == 1 { delete(hl, "first") @@ -54,14 +60,17 @@ func (p *Pagination) SetHeaderLink(c *gin.Context) { delete(hl, "next") } + // loop over the fields that make up the header links for rel, page := range hl { + // set the page info for the current field + q.Set("page", strconv.Itoa(page)) + ls := fmt.Sprintf( - `<%s://%s%s?per_page=%d&page=%d>; rel="%s"`, + `<%s://%s%s?%s>; rel="%s"`, resolveScheme(r), r.Host, r.URL.Path, - p.PerPage, - page, + q.Encode(), rel, ) diff --git a/api/webhook/post.go b/api/webhook/post.go index f71dd7eb2..e5f644889 100644 --- a/api/webhook/post.go +++ b/api/webhook/post.go @@ -785,7 +785,7 @@ func RenameRepository(ctx context.Context, l *logrus.Entry, db database.Interfac } // get total number of builds associated with repository - t, err = db.CountBuildsForRepo(ctx, dbR, nil) + t, err = db.CountBuildsForRepo(ctx, dbR, nil, time.Now().Unix(), 0) if err != nil { return nil, fmt.Errorf("unable to get build count for repo %s: %w", dbR.GetFullName(), err) } diff --git a/database/build/count_repo.go b/database/build/count_repo.go index df59ee2fe..a3eb4ba4b 100644 --- a/database/build/count_repo.go +++ b/database/build/count_repo.go @@ -12,7 +12,7 @@ import ( ) // CountBuildsForRepo gets the count of builds by repo ID from the database. -func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}) (int64, error) { +func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters map[string]interface{}, before, after int64) (int64, error) { e.logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), @@ -26,6 +26,8 @@ func (e *engine) CountBuildsForRepo(ctx context.Context, r *api.Repo, filters ma WithContext(ctx). Table(constants.TableBuild). Where("repo_id = ?", r.GetID()). + Where("created < ?", before). + Where("created > ?", after). Where(filters). Count(&b). Error diff --git a/database/build/count_repo_test.go b/database/build/count_repo_test.go index 77bbc69d6..10019e551 100644 --- a/database/build/count_repo_test.go +++ b/database/build/count_repo_test.go @@ -6,6 +6,7 @@ import ( "context" "reflect" "testing" + "time" "github.com/DATA-DOG/go-sqlmock" @@ -33,12 +34,14 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) { _buildOne.SetRepo(_repo) _buildOne.SetNumber(1) _buildOne.SetDeployPayload(nil) + _buildOne.SetCreated(1) _buildTwo := testutils.APIBuild() _buildTwo.SetID(2) _buildTwo.SetRepo(_repo) _buildTwo.SetNumber(2) _buildTwo.SetDeployPayload(nil) + _buildTwo.SetCreated(2) _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() @@ -47,7 +50,7 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) { _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows) + _mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1 AND created < $2 AND created > $3`).WithArgs(1, AnyArgument{}, 0).WillReturnRows(_rows) _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() @@ -88,7 +91,7 @@ func TestBuild_Engine_CountBuildsForRepo(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := test.database.CountBuildsForRepo(context.TODO(), _repo, filters) + got, err := test.database.CountBuildsForRepo(context.TODO(), _repo, filters, time.Now().Unix(), 0) if test.failure { if err == nil { diff --git a/database/build/interface.go b/database/build/interface.go index c723d838e..4b4f91f3e 100644 --- a/database/build/interface.go +++ b/database/build/interface.go @@ -35,7 +35,7 @@ type BuildInterface interface { // CountBuildsForOrg defines a function that gets the count of builds by org name. CountBuildsForOrg(context.Context, string, map[string]interface{}) (int64, error) // CountBuildsForRepo defines a function that gets the count of builds by repo ID. - CountBuildsForRepo(context.Context, *api.Repo, map[string]interface{}) (int64, error) + CountBuildsForRepo(context.Context, *api.Repo, map[string]interface{}, int64, int64) (int64, error) // CountBuildsForStatus defines a function that gets the count of builds by status. CountBuildsForStatus(context.Context, string, map[string]interface{}) (int64, error) // CreateBuild defines a function that creates a new build. diff --git a/database/build/list_repo.go b/database/build/list_repo.go index 470b448d3..3de7b2da4 100644 --- a/database/build/list_repo.go +++ b/database/build/list_repo.go @@ -27,7 +27,7 @@ func (e *engine) ListBuildsForRepo(ctx context.Context, r *api.Repo, filters map builds := []*api.Build{} // count the results - count, err := e.CountBuildsForRepo(ctx, r, filters) + count, err := e.CountBuildsForRepo(ctx, r, filters, before, after) if err != nil { return builds, 0, err } diff --git a/database/build/list_repo_test.go b/database/build/list_repo_test.go index 8071e3a67..c384405bf 100644 --- a/database/build/list_repo_test.go +++ b/database/build/list_repo_test.go @@ -55,7 +55,7 @@ func TestBuild_Engine_ListBuildsForRepo(t *testing.T) { _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) // ensure the mock expects the count query - _mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1`).WithArgs(1).WillReturnRows(_rows) + _mock.ExpectQuery(`SELECT count(*) FROM "builds" WHERE repo_id = $1 AND created < $2 AND created > $3`).WithArgs(1, AnyArgument{}, 0).WillReturnRows(_rows) // create expected query result in mock _rows = sqlmock.NewRows( diff --git a/database/integration_test.go b/database/integration_test.go index c1073df7d..a629c1e95 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -258,7 +258,7 @@ func testBuilds(t *testing.T, db Interface, resources *Resources) { methods["CountBuildsForOrg"] = true // count the builds for a repo - count, err = db.CountBuildsForRepo(context.TODO(), resources.Repos[0], nil) + count, err = db.CountBuildsForRepo(context.TODO(), resources.Repos[0], nil, time.Now().Unix(), 0) if err != nil { t.Errorf("unable to count builds for repo %d: %v", resources.Repos[0].GetID(), err) }