Pass pgcql.Query for patron requests#618
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors patron request listing/faceting to pass a parsed pgcql.Query through the repo layer (instead of passing raw CQL strings), so CQL parsing/validation happens at API boundaries and the repo only consumes validated SQL fragments/args.
Changes:
- Update
PrRepointerfaces and implementations to acceptpgcql.Queryfor listing patron requests/search view and fetching facets. - Export
ParsePatronRequestsCQLand update API handlers/tests to parse CQL before calling repo methods. - Adjust the facets SQL template to use parameterized
LIMIT/OFFSETso facet queries can safely prepend base args.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| broker/patron_request/db/prrepo.go | Repo interface + Pg repo methods now take pgcql.Query for list/search/facets. |
| broker/patron_request/db/prcql.go | Export CQL parsing helper; update CQL→SQL assembly and facets query to accept pgcql.Query. |
| broker/sqlc/pr_query.sql | Parameterize facets query LIMIT/OFFSET to support base args prepending. |
| broker/patron_request/api/api-handler.go | Parse/validate CQL before listing and faceting patron requests. |
| broker/patron_request/api/api-handler_test.go | Update tests to assert against pgcql.Query where clause instead of raw CQL strings. |
| broker/pullslip/api/api_handler.go | Parse/validate CQL before listing patron requests for PDF generation. |
| broker/pullslip/api/api_handler_test.go | Update mocks to accept pgcql.Query in ListPatronRequests. |
| broker/test/patron_request/db/prrepo_test.go | Update repo integration test to pass parsed pgcql.Query to list calls. |
| broker/patron_request/db/prcql_test.go | Update unit tests to use the exported parse function and testify assertions. |
jakub-id
approved these changes
May 27, 2026
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
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.
Should ParsePatronRequestsCQL be part of the PrRepo interface?