Skip to content

Commit 663acf1

Browse files
mfenniakMathieu Fenniak
authored andcommitted
fix: ListTrackedTimes API has no defined record ordering (#10588)
API call `GET /repos/{owner}/{repo}/issues/{index}/times` has no defined ordering implemented in it, causing PostgreSQL to have intermittent test failures on `TestAPIGetTrackedTimes` which expected records to be returned in ID order. ID order is reasonable enough, so this PR adds that ordering. Fixes #10577. ## 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 - [ ] 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/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10588 Reviewed-by: Cyborus <cyborus@disroot.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
1 parent 9244ed3 commit 663acf1

2 files changed

Lines changed: 16 additions & 3 deletions

File tree

models/issues/tracked_time.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func (opts *FindTrackedTimesOptions) toSession(e db.Engine) db.Engine {
148148

149149
// GetTrackedTimes returns all tracked times that fit to the given options.
150150
func GetTrackedTimes(ctx context.Context, options *FindTrackedTimesOptions) (trackedTimes TrackedTimeList, err error) {
151-
err = options.toSession(db.GetEngine(ctx)).Find(&trackedTimes)
151+
err = options.toSession(db.GetEngine(ctx)).Asc("tracked_time.id").Find(&trackedTimes)
152152
return trackedTimes, err
153153
}
154154

tests/integration/api_issue_tracked_time_test.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,21 @@ func TestAPIGetTrackedTimes(t *testing.T) {
6161
var filterAPITimes api.TrackedTimeList
6262
DecodeJSON(t, resp, &filterAPITimes)
6363
assert.Len(t, filterAPITimes, 2)
64-
assert.Equal(t, int64(3), filterAPITimes[0].ID)
65-
assert.Equal(t, int64(6), filterAPITimes[1].ID)
64+
assert.EqualValues(t, 3, filterAPITimes[0].ID)
65+
assert.EqualValues(t, 6, filterAPITimes[1].ID)
66+
67+
// test pagination
68+
allIDs := []int64{}
69+
for _, page := range []int{1, 2, 3} {
70+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/%d/times?page=%d&limit=1", user2.Name, issue2.Repo.Name, issue2.Index, page).
71+
AddTokenAuth(token)
72+
resp = MakeRequest(t, req, http.StatusOK)
73+
var pageAPITimes api.TrackedTimeList
74+
DecodeJSON(t, resp, &pageAPITimes)
75+
require.Len(t, pageAPITimes, 1)
76+
allIDs = append(allIDs, pageAPITimes[0].ID)
77+
}
78+
assert.Equal(t, []int64{2, 3, 6}, allIDs)
6679
}
6780

6881
func TestAPIDeleteTrackedTime(t *testing.T) {

0 commit comments

Comments
 (0)