Conversation
refactor a bit to ensure we're always reading from the right side
JanisSaldabols
approved these changes
Mar 5, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors patron-request lookups and creation to ensure the code consistently reads/links requests from the correct “side” (borrowing vs lending), and to always populate requester_req_id for borrowing-side requests.
Changes:
- Ensure newly created borrowing patron requests always set
RequesterReqIDand do not persistSupplierSymbol. - Add side-aware repository lookup (
GetPatronRequestByIdAndSide) and restrict supplierSymbol/requesterReqId lookups to lending-side rows. - Update message-handler, repo SQL, and related tests to use the new side-safe methods.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| broker/test/patron_request/api/api-handler_test.go | Updates integration tests to reflect borrowing requests no longer returning supplierSymbol, and uses the explicit supplier symbol when querying lending-side actions. |
| broker/sqlc/pr_query.sql | Renames the supplierSymbol/requesterReqId query and adds side = 'lending' filtering to avoid returning borrowing-side rows. |
| broker/patron_request/service/message-handler_test.go | Updates mocks/expectations to use GetPatronRequestByIdAndSide and the lending-only supplierSymbol/requesterReqId lookup. |
| broker/patron_request/service/message-handler.go | Switches message-to-request resolution to side-aware lookups and lending-only supplierSymbol/requesterReqId lookup. |
| broker/patron_request/service/action_test.go | Extends the shared test mock repo to implement the new repo method(s) and renamed lookup. |
| broker/patron_request/db/prrepo.go | Adds GetPatronRequestByIdAndSide and renames/rewires the lending-only supplierSymbol/requesterReqId lookup. |
| broker/patron_request/api/api-handler_test.go | Extends unit tests to assert RequesterReqID is set and SupplierSymbol is unset for borrowing-side creation. |
| broker/patron_request/api/api-handler.go | Sets RequesterReqID on creation and forces borrowing-side SupplierSymbol to NULL. |
| broker/client/client_test.go | Updates the client-side test mock to satisfy the updated repo interface usage. |
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.
refactor a bit to ensure we're always reading from the right side