add sort parameter to getJobs endpoint #51
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aqdas0307
force-pushed
the
feature/add-sorting-parameter-get-job
branch
3 times, most recently
from
7eab30c to
f8af1d4
Compare
lindenmckenzie
requested changes
Jan 15, 2026
domain/sort.go
Outdated
Comment on lines
1
to
18
| package domain | ||
|
|
||
| // Field represents what you are sorting by | ||
| type Field string | ||
|
|
||
| // Direction represents in which order are you sorting by | ||
| type Direction string | ||
|
|
||
| const ( | ||
| // FieldJobNumber indicates to sort via job number | ||
| FieldJobNumber Field = "job_number" | ||
| // FieldLabel indicates to sort via label | ||
| FieldLabel Field = "label" | ||
| // DirectionAsc indicates to sort in ascending order | ||
| DirectionAsc Direction = "asc" | ||
| // DirectionDesc indicates to sort in descending order | ||
| DirectionDesc Direction = "desc" | ||
| ) |
Contributor
There was a problem hiding this comment.
Prefer all of this in the api/parameters file.
api/jobs.go
Outdated
Comment on lines
70
to
73
| sortParam := r.URL.Query()["sort"] | ||
| // default direction is descending | ||
| direction := domain.Direction("desc") | ||
| field := domain.Field("job_number") |
Contributor
There was a problem hiding this comment.
Would prefer these as consts as otherwise you're still using magic strings.
api/jobs.go
Outdated
Comment on lines
75
to
91
| for _, s := range sortParam { | ||
| for _, p := range strings.Split(s, ",") { | ||
| part := strings.TrimSpace(p) | ||
| parts := strings.Split(part, ":") | ||
|
|
||
| field = domain.Field(strings.TrimSpace(parts[0])) | ||
| direction = domain.Direction(strings.TrimSpace(parts[1])) | ||
|
|
||
| if !domain.IsValidField(field) { | ||
| return nil, 0, appErrors.ErrSortFieldInvalid | ||
| } | ||
|
|
||
| if !domain.IsValidDirection(direction) { | ||
| return nil, 0, appErrors.ErrSortDirectionInvalid | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
would recommend abstracting this as a function.
mongo/job_store.go
Outdated
| } | ||
|
|
||
| // default to field being job_number | ||
| sortField := "job_number" |
mongo/job_store.go
Outdated
| // default to field being job_number | ||
| sortField := "job_number" | ||
| if field == domain.FieldLabel { | ||
| sortField = "label" |
aqdas0307
force-pushed
the
feature/add-sorting-parameter-get-job
branch
5 times, most recently
from
af8f9f1 to
6155316
Compare
aqdas0307
force-pushed
the
feature/add-sorting-parameter-get-job
branch
from
6155316 to
57d6b1b
Compare
lindenmckenzie
approved these changes
Jan 22, 2026
Contributor
lindenmckenzie
left a comment
lindenmckenzie
left a comment
There was a problem hiding this comment.
I'm not sure the structuring is quite right here (blaming on my own feedback) but equally it's not so problematic that I think this PR should be held up anymore. We'll take forward as it is and refactor as necessary if it becomes necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Add sort parameter to getJobs endpoint
Sort by:
Order by:
How to review
Sense check
Ensure tests pass
Ensure everything is okay/in place
Who can review
Not me.