Skip to content

ILLDEV-539 Use NCIP patron info - #785

Merged
jakub-id merged 2 commits into
mainfrom
ncip-patron-info
Sep 29, 2026
Merged

jakub-id merged 2 commits into
mainfrom
ncip-patron-info

Conversation

@jakub-id

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI balanced review requested due to automatic review settings September 29, 2026 23:05
@jakub-id jakub-id changed the title Use NCIP patron info ILLDEV-539 Use NCIP patron info Sep 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

NCIP responses containing patron names and addresses are persisted unsanitized in event logs.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds NCIP patron names and email addresses to borrowing requests while preserving existing patron data.

Changes:

  • Extends LMS user lookup options and results.
  • Extracts NCIP patron details and enriches ISO 18626 requests.
  • Adds adapter and enrichment tests.

Validation: Static review only; tests were not run.

File Description
broker/​patron_request/​service/​action.go Enriches patron requests from LMS lookup results.
broker/​patron_request/​service/​action_test.go Tests enrichment behavior and updated mocks.
broker/​lms/​lms_adapter.go Expands the lookup interface.
broker/​lms/​lms_adapter_test.go Tests NCIP patron information extraction.
broker/​lms/​lms_adapter_ncip.go Requests and parses NCIP patron details.
broker/​lms/​lms_adapter_manual.go Adapts manual lookup to the new interface.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread broker/patron_request/service/action.go
@jakub-id
jakub-id merged commit 5f1e164 into main Sep 29, 2026
6 checks passed
@jakub-id
jakub-id deleted the ncip-patron-info branch September 29, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants