From 2f6affa7e06bf6660cefa2ed54f0cfc452c77b33 Mon Sep 17 00:00:00 2001 From: Kurt Nordstrom Date: Mon, 28 Sep 2026 16:00:37 -0400 Subject: [PATCH 1/3] Add minimumCost field to illConfig --- directory/api.yaml | 7 ++++++ directory/api/catalog_config.go | 13 ++++++++++ directory/api/entries.go | 3 ++- directory/api/loan_config_test.go | 24 +++++++++++++++++- directory/import/db/entry.go | 1 + directory/import/db/repo_test.go | 6 ++++- directory/import/model/config.go | 1 + directory/import/service/importer_test.go | 6 +++-- .../migrations/012_minimum_cost.down.sql | 1 + directory/migrations/012_minimum_cost.up.sql | 2 ++ directory/query.sql | 7 +++--- directory/test/entries_test.go | 8 +++--- directory/test/loan_config_test.go | 25 +++++++++++++++++++ 13 files changed, 93 insertions(+), 11 deletions(-) create mode 100644 directory/migrations/012_minimum_cost.down.sql create mode 100644 directory/migrations/012_minimum_cost.up.sql diff --git a/directory/api.yaml b/directory/api.yaml index b0521746..c894b373 100644 --- a/directory/api.yaml +++ b/directory/api.yaml @@ -1757,6 +1757,7 @@ components: duplicateCheckWindowHours: { type: integer, format: int32, minimum: 0, nullable: true } defaultLoanPeriod: { type: integer, format: int32, minimum: 1, nullable: true } maxRequestsPerPatron: { type: integer, format: int32, minimum: 0, nullable: true } + minimumCost: { type: number, format: double, minimum: 0, nullable: true } ImportCatalogConfig: type: object @@ -2238,6 +2239,12 @@ components: nullable: true minimum: 0 description: Maximum number of active requests allowed per patron. 0, omitted, or null disables the limit. + minimumCost: + type: number + format: double + nullable: true + minimum: 0 + description: Minimum cost configured for ILL requests. Omitted or null means no minimum cost is configured. iso18626Url: type: string description: URL of the ISO18626 service. When set, used instead of an ISO18626 service from endpoints. diff --git a/directory/api/catalog_config.go b/directory/api/catalog_config.go index 8a4b30ce..8275b09e 100644 --- a/directory/api/catalog_config.go +++ b/directory/api/catalog_config.go @@ -252,6 +252,7 @@ func illConfigToDBParams(entryID uuid.UUID, cfg IllConfig) db.UpsertIllConfigPar DuplicateCheckWindowHours: cfg.DuplicateCheckWindowHours, DefaultLoanPeriod: nullableLoanPeriod(cfg), MaxRequestsPerPatron: nullableMaxRequestsPerPatron(cfg), + MinimumCost: nullableMinimumCost(cfg), } if cfg.Iso18626Vendor != nil { vendor := string(*cfg.Iso18626Vendor) @@ -276,6 +277,7 @@ func illConfigPatchToDBParams(entryID uuid.UUID, cfg IllConfig, original db.IllC DuplicateCheckWindowHours: original.DuplicateCheckWindowHours, DefaultLoanPeriod: original.DefaultLoanPeriod, MaxRequestsPerPatron: original.MaxRequestsPerPatron, + MinimumCost: original.MinimumCost, } params.Iso18626Url = derefOrDefaultPtr(cfg.Iso18626Url, params.Iso18626Url) @@ -300,6 +302,9 @@ func illConfigPatchToDBParams(entryID uuid.UUID, cfg IllConfig, original db.IllC if cfg.MaxRequestsPerPatron.IsSpecified() { params.MaxRequestsPerPatron = nullableMaxRequestsPerPatron(cfg) } + if cfg.MinimumCost.IsSpecified() { + params.MinimumCost = nullableMinimumCost(cfg) + } return params } @@ -319,6 +324,14 @@ func nullableMaxRequestsPerPatron(cfg IllConfig) *int32 { return &value } +func nullableMinimumCost(cfg IllConfig) *float64 { + value, err := cfg.MinimumCost.Get() + if err != nil { + return nil + } + return &value +} + func boolPtr(value bool) *bool { return &value } diff --git a/directory/api/entries.go b/directory/api/entries.go index f00d1a56..33367848 100644 --- a/directory/api/entries.go +++ b/directory/api/entries.go @@ -294,7 +294,8 @@ func buildEntrySQL(whereClause string) string { 'supplierPatronPattern', i.supplier_patron_pattern, 'duplicateCheckWindowHours', i.duplicate_check_window_hours, 'defaultLoanPeriod', i.default_loan_period, - 'maxRequestsPerPatron', i.max_requests_per_patron + 'maxRequestsPerPatron', i.max_requests_per_patron, + 'minimumCost', i.minimum_cost )) FROM ill_configs i WHERE i.entry = e.id) as ill_config, ( SELECT diff --git a/directory/api/loan_config_test.go b/directory/api/loan_config_test.go index 31f5b576..56fbeee9 100644 --- a/directory/api/loan_config_test.go +++ b/directory/api/loan_config_test.go @@ -52,4 +52,26 @@ func TestMaxRequestsPerPatronPatch(t *testing.T) { assert.Nil(t, result.MaxRequestsPerPatron, "no implicit default") } -func int32Pointer(value int32) *int32 { return &value } +func TestMinimumCostPatch(t *testing.T) { + original := 1.5 + for _, tc := range []struct { + body string + want *float64 + }{ + {`{}`, &original}, + {`{"minimumCost":null}`, nil}, + {`{"minimumCost":0}`, float64Pointer(0)}, + {`{"minimumCost":2.75}`, float64Pointer(2.75)}, + } { + var config IllConfig + require.NoError(t, json.Unmarshal([]byte(tc.body), &config)) + result := illConfigPatchToDBParams(uuid.New(), config, db.IllConfig{MinimumCost: &original}) + assert.Equal(t, tc.want, result.MinimumCost) + } + + result := illConfigToDBParams(uuid.New(), IllConfig{}) + assert.Nil(t, result.MinimumCost, "no implicit default") +} + +func int32Pointer(value int32) *int32 { return &value } +func float64Pointer(value float64) *float64 { return &value } diff --git a/directory/import/db/entry.go b/directory/import/db/entry.go index a3721b43..f38e285e 100644 --- a/directory/import/db/entry.go +++ b/directory/import/db/entry.go @@ -598,6 +598,7 @@ func replaceILLConfig(ctx context.Context, queries *db.Queries, entryID uuid.UUI DuplicateCheckWindowHours: config.DuplicateCheckWindowHours, DefaultLoanPeriod: config.DefaultLoanPeriod, MaxRequestsPerPatron: config.MaxRequestsPerPatron, + MinimumCost: config.MinimumCost, }) return err } diff --git a/directory/import/db/repo_test.go b/directory/import/db/repo_test.go index fe15f7a7..664eff0d 100644 --- a/directory/import/db/repo_test.go +++ b/directory/import/db/repo_test.go @@ -152,6 +152,9 @@ func TestImportEntryCreatesCompleteAggregateWithGeneratedIDs(t *testing.T) { var maxRequestsPerPatron int32 require.NoError(t, testPool.QueryRow(context.Background(), `SELECT max_requests_per_patron FROM ill_configs WHERE entry=$1`, entryID).Scan(&maxRequestsPerPatron)) require.Zero(t, maxRequestsPerPatron) + var minimumCost float64 + require.NoError(t, testPool.QueryRow(context.Background(), `SELECT minimum_cost FROM ill_configs WHERE entry=$1`, entryID).Scan(&minimumCost)) + require.Equal(t, 1.25, minimumCost) } func TestImportEntryConflictPoliciesAndUpdateFullSynchronization(t *testing.T) { @@ -1325,6 +1328,7 @@ func completeEntryAggregate(symbol string) model.EntryAggregate { metadataMode := "replace" truth := true zero := int32(0) + minimumCost := 1.25 aggregate.Data.Endpoints = []model.ServiceEndpoint{{Name: "ISO", Type: "ISO18626", Address: "https://example.test/ill"}} aggregate.Data.Addresses = []model.Address{{Type: "Default", Components: []model.AddressComponent{{Seq: 1, Type: "Locality", Value: "Riga"}}}} aggregate.Data.Closures = []model.Closure{{StartDate: "2026-12-24", EndDate: "2026-12-26", Reason: "Holiday"}} @@ -1336,7 +1340,7 @@ func completeEntryAggregate(symbol string) model.EntryAggregate { HoldingsFormat: &model.HoldingsParserConfig{Marc: &model.MarcHoldingsParserConfig{MainField: &text}}, MetadataFormat: &model.MetadataParserConfig{Marc21: &model.MarcMetadataParserConfig{Title: &text}}, } - aggregate.Data.ILLConfig = &model.ILLConfig{ISO18626URL: &text, LendersOfLastResort: []model.SymbolRef{}, IncludeSupplierInfo: &truth, MaxRequestsPerPatron: &zero} + aggregate.Data.ILLConfig = &model.ILLConfig{ISO18626URL: &text, LendersOfLastResort: []model.SymbolRef{}, IncludeSupplierInfo: &truth, MaxRequestsPerPatron: &zero, MinimumCost: &minimumCost} aggregate.Data.HoldingsPolicy = &model.HoldingsPolicy{ Locations: []model.HoldingsLocation{{Code: "MAIN", Name: "Main", SupplyPreference: 1}}, ShelvingLocations: []model.HoldingsShelvingLocation{}, diff --git a/directory/import/model/config.go b/directory/import/model/config.go index a070ad35..c788ba69 100644 --- a/directory/import/model/config.go +++ b/directory/import/model/config.go @@ -38,6 +38,7 @@ type PatronProfile struct { type ILLConfig struct { DefaultLoanPeriod *int32 `json:"defaultLoanPeriod"` MaxRequestsPerPatron *int32 `json:"maxRequestsPerPatron"` + MinimumCost *float64 `json:"minimumCost"` ISO18626URL *string `json:"iso18626Url"` ISO18626Vendor *string `json:"iso18626Vendor"` LendersOfLastResort []SymbolRef `json:"lendersOfLastResort"` diff --git a/directory/import/service/importer_test.go b/directory/import/service/importer_test.go index d867d9b8..354aea6e 100644 --- a/directory/import/service/importer_test.go +++ b/directory/import/service/importer_test.go @@ -233,7 +233,7 @@ func TestImportAcceptsNullLMSPatronProfiles(t *testing.T) { require.Nil(t, repo.entry.Data.LMSConfig.PatronProfiles) } -func TestImportAcceptsMaxRequestsPerPatron(t *testing.T) { +func TestImportAcceptsIllConfigLimits(t *testing.T) { repo := &recordingRepo{result: model.RepoResult{Outcome: model.OutcomeImported}} record := strings.Replace(validEntryRecord(), `"illConfig":null`, validILLConfig(), 1) @@ -246,6 +246,8 @@ func TestImportAcceptsMaxRequestsPerPatron(t *testing.T) { require.NotNil(t, repo.entry.Data.ILLConfig) require.NotNil(t, repo.entry.Data.ILLConfig.MaxRequestsPerPatron) assert.Equal(t, int32(0), *repo.entry.Data.ILLConfig.MaxRequestsPerPatron) + require.NotNil(t, repo.entry.Data.ILLConfig.MinimumCost) + assert.Equal(t, 1.25, *repo.entry.Data.ILLConfig.MinimumCost) } func validLMSConfig() string { @@ -253,7 +255,7 @@ func validLMSConfig() string { } func validILLConfig() string { - return `"illConfig":{"iso18626Url":null,"iso18626Vendor":null,"lendersOfLastResort":[],"includeRequestingAgencyInfo":null,"includeSupplierInfo":null,"includeReturnInfo":null,"includeVendorNote":null,"useOfferedCosts":null,"noteFieldSeparator":null,"supplierPatronPattern":null,"duplicateCheckWindowHours":null,"maxRequestsPerPatron":0}` + return `"illConfig":{"iso18626Url":null,"iso18626Vendor":null,"lendersOfLastResort":[],"includeRequestingAgencyInfo":null,"includeSupplierInfo":null,"includeReturnInfo":null,"includeVendorNote":null,"useOfferedCosts":null,"noteFieldSeparator":null,"supplierPatronPattern":null,"duplicateCheckWindowHours":null,"maxRequestsPerPatron":0,"minimumCost":1.25}` } func TestImportAccountsForSkippedAndRepositoryFailures(t *testing.T) { diff --git a/directory/migrations/012_minimum_cost.down.sql b/directory/migrations/012_minimum_cost.down.sql new file mode 100644 index 00000000..ade1a77b --- /dev/null +++ b/directory/migrations/012_minimum_cost.down.sql @@ -0,0 +1 @@ +ALTER TABLE ill_configs DROP COLUMN minimum_cost; diff --git a/directory/migrations/012_minimum_cost.up.sql b/directory/migrations/012_minimum_cost.up.sql new file mode 100644 index 00000000..6a51b620 --- /dev/null +++ b/directory/migrations/012_minimum_cost.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE ill_configs + ADD COLUMN minimum_cost DOUBLE PRECISION CHECK (minimum_cost >= 0); diff --git a/directory/query.sql b/directory/query.sql index e5f16921..ef20e920 100644 --- a/directory/query.sql +++ b/directory/query.sql @@ -158,13 +158,13 @@ INSERT INTO ill_configs ( include_requesting_agency_info, include_supplier_info, include_return_info, include_vendor_note, use_offered_costs, note_field_separator, supplier_patron_pattern, duplicate_check_window_hours, default_loan_period, - max_requests_per_patron + max_requests_per_patron, minimum_cost ) VALUES ( @entry, @iso18626_url, @iso18626_vendor, @lenders_of_last_resort, @include_requesting_agency_info, @include_supplier_info, @include_return_info, @include_vendor_note, @use_offered_costs, @note_field_separator, @supplier_patron_pattern, @duplicate_check_window_hours, @default_loan_period, - @max_requests_per_patron + @max_requests_per_patron, @minimum_cost ) ON CONFLICT (entry) DO UPDATE SET iso18626_url = COALESCE(@iso18626_url, ill_configs.iso18626_url), @@ -179,7 +179,8 @@ ON CONFLICT (entry) DO UPDATE SET supplier_patron_pattern = COALESCE(@supplier_patron_pattern, ill_configs.supplier_patron_pattern), duplicate_check_window_hours = COALESCE(@duplicate_check_window_hours, ill_configs.duplicate_check_window_hours), default_loan_period = @default_loan_period, - max_requests_per_patron = @max_requests_per_patron + max_requests_per_patron = @max_requests_per_patron, + minimum_cost = @minimum_cost RETURNING *; -- name: GetIllConfigByEntry :one diff --git a/directory/test/entries_test.go b/directory/test/entries_test.go index a7bc44c5..22023770 100644 --- a/directory/test/entries_test.go +++ b/directory/test/entries_test.go @@ -903,7 +903,8 @@ func TestEntryDirectoryContractFieldsAndCatalogConfig(t *testing.T) { "noteFieldSeparator":" | ", "supplierPatronPattern":"PATRON-{requesterSymbol}", "duplicateCheckWindowHours":24, - "maxRequestsPerPatron":0 + "maxRequestsPerPatron":0, + "minimumCost":1.5 }, "symbols":[{"authority":"ISIL","symbol":"CONTRACT"}], "catalogConfig":{ @@ -993,7 +994,8 @@ func TestEntryDirectoryContractFieldsAndCatalogConfig(t *testing.T) { illConfig["noteFieldSeparator"] != " | " || illConfig["supplierPatronPattern"] != "PATRON-{requesterSymbol}" || illConfig["duplicateCheckWindowHours"] != float64(24) || - illConfig["maxRequestsPerPatron"] != float64(0) { + illConfig["maxRequestsPerPatron"] != float64(0) || + illConfig["minimumCost"] != 1.5 { t.Fatalf("illConfig fields did not round-trip: %#v", illConfig) } @@ -1010,7 +1012,7 @@ func TestEntryDirectoryContractFieldsAndCatalogConfig(t *testing.T) { t.Fatalf("failed to parse entry after illConfig PATCH: %v", err) } illConfig = entry["illConfig"].(map[string]any) - if illConfig["noteFieldSeparator"] != " / " || illConfig["useOfferedCosts"] != false || illConfig["iso18626Url"] != "https://iso.example.org/iso18626" || illConfig["maxRequestsPerPatron"] != float64(25) { + if illConfig["noteFieldSeparator"] != " / " || illConfig["useOfferedCosts"] != false || illConfig["iso18626Url"] != "https://iso.example.org/iso18626" || illConfig["maxRequestsPerPatron"] != float64(25) || illConfig["minimumCost"] != 1.5 { t.Fatalf("partial illConfig PATCH did not merge fields: %#v", illConfig) } if _, ok := illConfig["lendersOfLastResort"]; ok { diff --git a/directory/test/loan_config_test.go b/directory/test/loan_config_test.go index 0ac46853..af1deeb7 100644 --- a/directory/test/loan_config_test.go +++ b/directory/test/loan_config_test.go @@ -59,3 +59,28 @@ func TestMaxRequestsPerPatronPersistenceAndValidation(t *testing.T) { assert.Equal(t, tc.want, config["maxRequestsPerPatron"]) } } + +func TestMinimumCostPersistenceAndValidation(t *testing.T) { + resetDb() + headers := map[string]string{"X-Okapi-Tenant": "ANINST", "X-Okapi-Permissions": `["directory.consortium.all"]`} + path := "/entries/by-id/00000000-0000-0000-0000-000000000002" + for _, tc := range []struct { + value string + status int + want any + }{ + {"0", http.StatusNoContent, float64(0)}, + {"2.75", http.StatusNoContent, float64(2.75)}, + {"-1", http.StatusBadRequest, float64(2.75)}, + {"null", http.StatusNoContent, nil}, + } { + response, body := jsonReq(t, http.MethodPatch, path, `{"illConfig":{"minimumCost":`+tc.value+`}}`, headers) + require.Equal(t, tc.status, response.StatusCode, body) + response, body = jsonReq(t, http.MethodGet, path, "", headers) + require.Equal(t, http.StatusOK, response.StatusCode, body) + var entry map[string]any + require.NoError(t, json.Unmarshal([]byte(body), &entry)) + config := entry["illConfig"].(map[string]any) + assert.Equal(t, tc.want, config["minimumCost"]) + } +} From a7456d3376f1f217e7afde0550712fa1ff3930a0 Mon Sep 17 00:00:00 2001 From: Kurt Nordstrom Date: Tue, 29 Sep 2026 14:54:41 -0400 Subject: [PATCH 2/3] Fix issue with overlapping migration --- directory/migrations/012_minimum_cost.down.sql | 1 - directory/migrations/012_minimum_cost.up.sql | 2 -- 2 files changed, 3 deletions(-) delete mode 100644 directory/migrations/012_minimum_cost.down.sql delete mode 100644 directory/migrations/012_minimum_cost.up.sql diff --git a/directory/migrations/012_minimum_cost.down.sql b/directory/migrations/012_minimum_cost.down.sql deleted file mode 100644 index ade1a77b..00000000 --- a/directory/migrations/012_minimum_cost.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE ill_configs DROP COLUMN minimum_cost; diff --git a/directory/migrations/012_minimum_cost.up.sql b/directory/migrations/012_minimum_cost.up.sql deleted file mode 100644 index 6a51b620..00000000 --- a/directory/migrations/012_minimum_cost.up.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE ill_configs - ADD COLUMN minimum_cost DOUBLE PRECISION CHECK (minimum_cost >= 0); From 158e3d9c78b67f610a00cdbc6bf8b476520acad6 Mon Sep 17 00:00:00 2001 From: Kurt Nordstrom Date: Tue, 29 Sep 2026 14:54:58 -0400 Subject: [PATCH 3/3] Add new migrations --- directory/migrations/013_minimum_cost.down.sql | 1 + directory/migrations/013_minimum_cost.up.sql | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 directory/migrations/013_minimum_cost.down.sql create mode 100644 directory/migrations/013_minimum_cost.up.sql diff --git a/directory/migrations/013_minimum_cost.down.sql b/directory/migrations/013_minimum_cost.down.sql new file mode 100644 index 00000000..ade1a77b --- /dev/null +++ b/directory/migrations/013_minimum_cost.down.sql @@ -0,0 +1 @@ +ALTER TABLE ill_configs DROP COLUMN minimum_cost; diff --git a/directory/migrations/013_minimum_cost.up.sql b/directory/migrations/013_minimum_cost.up.sql new file mode 100644 index 00000000..6a51b620 --- /dev/null +++ b/directory/migrations/013_minimum_cost.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE ill_configs + ADD COLUMN minimum_cost DOUBLE PRECISION CHECK (minimum_cost >= 0);