Skip to content

Conversation

@adamdickmeiss
Copy link
Contributor

just a call for each of the 8 initial NCIP types.

just a call for each of the 8 initial NCIP types.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a synchronous NCIP (NISO Circulation Interchange Protocol) client implementation that supports 8 NCIP message types: LookupUser, AcceptItem, DeleteItem, RequestItem, CancelRequestItem, CheckInItem, CheckOutItem, and CreateUserFiscalTransaction.

  • Implements the NcipClient interface with methods for all 8 NCIP operations
  • Adds comprehensive test coverage for all operations including success, failure, and edge cases
  • Extends Directory schema to support NCIP configuration with operation modes (auto/manual/disabled)

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
broker/ncipclient/ncipclient.go Defines the NcipClient interface with 8 NCIP operations and custom error type
broker/ncipclient/ncipclient_impl.go Implements the NcipClient interface with HTTP communication and response handling
broker/ncipclient/ncipclient_test.go Comprehensive test suite covering all operations with various scenarios
illmock/directory/directory_api.yaml Adds NCIP schema definition with configuration fields and operation modes
illmock/dirmock/directories.json Adds sample NCIP configuration for mock testing
go.work.sum Updates Go workspace checksums for dependencies

Copy link
Contributor

@jakub-id jakub-id left a comment

Choose a reason for hiding this comment

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

Looks good to me

@adamdickmeiss adamdickmeiss merged commit 41a4422 into main Nov 25, 2025
4 checks passed
@adamdickmeiss adamdickmeiss deleted the CROSSLINK-183-extend-directory-with-ncip-info branch November 25, 2025 11:16
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.

3 participants