From 4dd2522177e8eb55a32798a2d9dc7922e008a701 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Tue, 18 Aug 2026 13:47:21 +0300 Subject: [PATCH 1/9] ILLDEV-466 Use template record for PDF pullslip --- broker/email/email.go | 206 +++++++++++ broker/email/email_test.go | 322 ++++++++++++++++++ broker/oapi/open-api.yaml | 3 + broker/patron_request/api/api-handler_test.go | 3 +- broker/patron_request/service/action.go | 25 +- .../patron_request/service/action_mapping.go | 8 + broker/patron_request/service/action_test.go | 4 +- .../patron_request/service/statemodel_test.go | 32 ++ broker/pullslip/api/api_handler_test.go | 13 + broker/pullslip/service/pdf.go | 206 ++--------- broker/pullslip/service/pdf_test.go | 266 ++++----------- .../pullslip/service/pull_slip_template.html | 120 ------- broker/scheduler/service/email_sender.go | 18 +- broker/test/pullslip/api/api_handler_test.go | 25 ++ misc/state-models.yaml | 129 +++++++ 15 files changed, 862 insertions(+), 518 deletions(-) delete mode 100644 broker/pullslip/service/pull_slip_template.html diff --git a/broker/email/email.go b/broker/email/email.go index 92aa2f8fe..cc8ca0c4b 100644 --- a/broker/email/email.go +++ b/broker/email/email.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "errors" "fmt" + "html/template" "mime" "mime/multipart" "mime/quotedprintable" @@ -12,9 +13,14 @@ import ( "net/textproto" "strings" + pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + "github.com/indexdata/crosslink/iso18626" "github.com/indexdata/go-utils/utils" ) +const DEFAULT_FOR_NO_VALUE = "n/a" +const DATE_LAYOUT = "2006-01-02" + // Environment variables for SMTP configuration. var ( SMTP_HOST = utils.GetEnv("SMTP_HOST", "") @@ -180,3 +186,203 @@ func joinAddresses(addrs []string) string { } return result } + +type PullSlipData struct { + BorrowerName string + ReqId string + PickupLocation string + Title string + Author string + DueDate string + ReturnAddress string + BarcodeBase64 string + ServiceType string + ServiceLevel string + SystemIdentifier string + Publisher string + Volume string + Issue string + Pages string + StaffNotes string + CallNumber string + LoanConditions string + PatronName string + PatronSurname string + PatronId string +} + +func GetPullSlipData(pr pr_db.PatronRequest, notes []pr_db.Notification, conditions []pr_db.Notification, barcodeData string) PullSlipData { + data := PullSlipData{ + ReqId: pr.RequesterReqID.String, + PickupLocation: getPickupLocation(pr), + Title: DEFAULT_FOR_NO_VALUE, + Author: DEFAULT_FOR_NO_VALUE, + DueDate: DEFAULT_FOR_NO_VALUE, + ReturnAddress: DEFAULT_FOR_NO_VALUE, + BarcodeBase64: barcodeData, + ServiceType: DEFAULT_FOR_NO_VALUE, + ServiceLevel: DEFAULT_FOR_NO_VALUE, + SystemIdentifier: DEFAULT_FOR_NO_VALUE, + Publisher: DEFAULT_FOR_NO_VALUE, + Volume: DEFAULT_FOR_NO_VALUE, + Issue: DEFAULT_FOR_NO_VALUE, + Pages: DEFAULT_FOR_NO_VALUE, + StaffNotes: getStaffNotes(notes), + CallNumber: getCallNumber(pr), + LoanConditions: getLoanConditions(conditions), + PatronName: DEFAULT_FOR_NO_VALUE, + PatronSurname: DEFAULT_FOR_NO_VALUE, + PatronId: DEFAULT_FOR_NO_VALUE, + } + if pr.IllRequest.BibliographicInfo.Author != "" { + data.Author = pr.IllRequest.BibliographicInfo.Author + } + if pr.IllRequest.BibliographicInfo.Title != "" { + data.Title = pr.IllRequest.BibliographicInfo.Title + } + if pr.IllRequest.BibliographicInfo.Volume != "" { + data.Volume = pr.IllRequest.BibliographicInfo.Volume + } + if pr.IllRequest.BibliographicInfo.Issue != "" { + data.Issue = pr.IllRequest.BibliographicInfo.Issue + } + if pr.IllRequest.BibliographicInfo.EstimatedNoPages != "" { + data.Pages = pr.IllRequest.BibliographicInfo.EstimatedNoPages + } + if pr.IllRequest.BibliographicInfo.SupplierUniqueRecordId != "" { + data.SystemIdentifier = pr.IllRequest.BibliographicInfo.SupplierUniqueRecordId + } + if pr.IllRequest.PublicationInfo != nil && pr.IllRequest.PublicationInfo.Publisher != "" { + data.Publisher = pr.IllRequest.PublicationInfo.Publisher + } + if pr.IllResponse.StatusInfo.DueDate != nil { + data.DueDate = pr.IllResponse.StatusInfo.DueDate.Format(DATE_LAYOUT) + } + if pr.IllResponse.ReturnInfo != nil && pr.IllResponse.ReturnInfo.PhysicalAddress != nil { + data.ReturnAddress = formatPhysicalAddress(pr.IllResponse.ReturnInfo.PhysicalAddress) + } + if pr.IllRequest.ServiceInfo != nil { + if pr.IllRequest.ServiceInfo.ServiceLevel != nil && pr.IllRequest.ServiceInfo.ServiceLevel.Text != "" { + data.ServiceLevel = pr.IllRequest.ServiceInfo.ServiceLevel.Text + } + if pr.IllRequest.ServiceInfo.ServiceType != "" { + data.ServiceType = string(pr.IllRequest.ServiceInfo.ServiceType) + } + } + if pr.IllRequest.PatronInfo != nil { + if pr.IllRequest.PatronInfo.PatronId != "" { + data.PatronId = pr.IllRequest.PatronInfo.PatronId + } + if pr.IllRequest.PatronInfo.GivenName != "" { + data.PatronName = pr.IllRequest.PatronInfo.GivenName + } + if pr.IllRequest.PatronInfo.Surname != "" { + data.PatronSurname = pr.IllRequest.PatronInfo.Surname + } + } + return data +} + +func RenderPullSlipHTMLWithTemplate(data PullSlipData, templateBody string) (string, error) { + tmpl, err := template.New("pull-slip").Parse(templateBody) + if err != nil { + return "", err + } + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return "", err + } + return buf.String(), nil +} + +func getStaffNotes(noteList []pr_db.Notification) string { + noteStrings := []string{} + for _, note := range noteList { + if note.Note.Valid { + noteStrings = append(noteStrings, note.Note.String) + } + } + notes := strings.Join(noteStrings, "\n") + if notes == "" { + return DEFAULT_FOR_NO_VALUE + } + return notes +} + +func getLoanConditions(conditionList []pr_db.Notification) string { + conditionStrings := []string{} + for _, note := range conditionList { + if note.Condition.Valid { + conditionStrings = append(conditionStrings, note.Condition.String) + } + } + conditions := strings.Join(conditionStrings, "\n") + if conditions == "" { + return DEFAULT_FOR_NO_VALUE + } + return conditions +} + +func getCallNumber(request pr_db.PatronRequest) string { + callNumberStrings := []string{} + for _, item := range request.Items { + if item.CallNumber != nil && *item.CallNumber != "" { + callNumberStrings = append(callNumberStrings, *item.CallNumber) + } + } + callNumber := strings.Join(callNumberStrings, ", ") + if callNumber == "" { + return DEFAULT_FOR_NO_VALUE + } + return callNumber +} + +func getPickupLocation(request pr_db.PatronRequest) string { + if len(request.IllRequest.RequestedDeliveryInfo) > 0 && request.IllRequest.RequestedDeliveryInfo[0].Address != nil { + address := *request.IllRequest.RequestedDeliveryInfo[0].Address + if address.PhysicalAddress != nil { + return formatPhysicalAddress(address.PhysicalAddress) + } else if address.ElectronicAddress != nil && address.ElectronicAddress.ElectronicAddressData != "" { + return address.ElectronicAddress.ElectronicAddressData + } + } + return DEFAULT_FOR_NO_VALUE +} + +func formatPhysicalAddress(a *iso18626.PhysicalAddress) string { + parts := []string{} + if a.Line1 != "" { + parts = append(parts, a.Line1) + } + if a.Line2 != "" { + parts = append(parts, a.Line2) + } + if a.Locality != "" { + parts = append(parts, a.Locality) + } + if a.PostalCode != "" { + parts = append(parts, a.PostalCode) + } + if a.Region != nil && a.Region.Text != "" { + parts = append(parts, a.Region.Text) + } + if a.Country != nil && a.Country.Text != "" { + parts = append(parts, a.Country.Text) + } + return strings.Join(parts, ", ") +} + +func GetBatchEmailData(fullCount int64, actualCount int, batchQuery string) map[string]string { + return map[string]string{ + "fullCount": fmt.Sprintf("%d", fullCount), + "actualCount": fmt.Sprintf("%d", actualCount), + "batchQuery": batchQuery, + } +} + +func RenderBatchEmailTemplate(value string, placeholders map[string]string) string { + for key, replacement := range placeholders { + value = strings.ReplaceAll(value, "{{"+key+"}}", replacement) + } + return value +} diff --git a/broker/email/email_test.go b/broker/email/email_test.go index fcb9fd3f0..14d2fde52 100644 --- a/broker/email/email_test.go +++ b/broker/email/email_test.go @@ -1,8 +1,14 @@ package email import ( + "strings" "testing" + "time" + pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + "github.com/indexdata/crosslink/iso18626" + "github.com/indexdata/go-utils/utils" + "github.com/jackc/pgx/v5/pgtype" "github.com/stretchr/testify/assert" ) @@ -86,3 +92,319 @@ func TestBuildRawMessage_WithoutAttachment(t *testing.T) { assert.NoError(t, err) assert.NotContains(t, string(raw), "application/pdf") } + +// --------------------------------------------------------------------------- +// RenderPullSlipHTMLWithTemplate +// --------------------------------------------------------------------------- + +func TestRenderPullSlipHTML(t *testing.T) { + template := "
\n Service Type: {{.ServiceType}}
\n Service Level: {{.ServiceLevel}}
\n System Identifier: {{.SystemIdentifier}}
\n Title: {{.Title}}
\n Author: {{.Author}}
\n Publisher: {{.Publisher}}
\n Volume(s): {{.Volume}}
\n Issue: {{.Issue}}
\n Pages: {{.Pages}}
\n
" + html, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ + ServiceType: "Loan", + Title: "Big Shark", + Author: "John Doe", + DueDate: "2026-01-01", + ReturnAddress: "1 Test Street", + SystemIdentifier: "abc123", + }, template) + assert.NoError(t, err) + assert.True(t, strings.Contains(html, "Loan")) + assert.True(t, strings.Contains(html, "John Doe")) + assert.True(t, strings.Contains(html, "Big Shark")) + assert.True(t, strings.Contains(html, "abc123")) +} + +func TestRenderPullSlipHTML_UsesProvidedTemplate(t *testing.T) { + html, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") + + assert.NoError(t, err) + assert.Equal(t, "
REQ-1
", html) +} + +func TestRenderPullSlipHTML_InvalidTemplate(t *testing.T) { + _, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") + assert.Error(t, err) +} + +func TestRenderPullSlipHTML_ExecuteError(t *testing.T) { + _, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") + // Execute on a struct with map-access fails + assert.Error(t, err) +} + +// --------------------------------------------------------------------------- +// GetPullSlipData +// --------------------------------------------------------------------------- + +func TestGetPullSlipData_PopulatesAllAvailableFields(t *testing.T) { + callNumber := "QA76.73.G63" + dueDate := utils.XSDDateTime{Time: time.Date(2026, 8, 15, 9, 30, 0, 0, time.UTC)} + pr := pr_db.PatronRequest{ + RequesterReqID: pgtype.Text{String: "REQ-123", Valid: true}, + Items: []pr_db.PrItem{{ID: "item-1", CallNumber: &callNumber}}, + IllRequest: iso18626.Request{ + BibliographicInfo: iso18626.BibliographicInfo{ + Author: "Jane Doe", + Title: "Distributed Libraries", + Volume: "7", + Issue: "2", + EstimatedNoPages: "18", + SupplierUniqueRecordId: "SYS-456", + }, + PublicationInfo: &iso18626.PublicationInfo{Publisher: "Index Press"}, + ServiceInfo: &iso18626.ServiceInfo{ + ServiceType: iso18626.TypeServiceTypeLoan, + ServiceLevel: &iso18626.TypeSchemeValuePair{Text: "Rush"}, + }, + RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ + {Address: &iso18626.Address{ + PhysicalAddress: &iso18626.PhysicalAddress{ + Line1: "Pickup Desk", + Locality: "Riga", + PostalCode: "LV-1050", + Country: &iso18626.TypeSchemeValuePair{Text: "LV"}, + }, + }}, + }, + PatronInfo: &iso18626.PatronInfo{ + PatronId: "P-789", + GivenName: "Ann", + Surname: "Reader", + }, + }, + IllResponse: iso18626.SupplyingAgencyMessage{ + StatusInfo: iso18626.StatusInfo{DueDate: &dueDate}, + ReturnInfo: &iso18626.ReturnInfo{PhysicalAddress: &iso18626.PhysicalAddress{ + Line1: "Return Room", + Line2: "Shelf B", + Locality: "Riga", + Country: &iso18626.TypeSchemeValuePair{Text: "LV"}, + }}, + }, + } + notes := []pr_db.Notification{ + {Note: pgtype.Text{String: "first note", Valid: true}}, + {Note: pgtype.Text{String: "ignored note", Valid: false}}, + {Note: pgtype.Text{String: "second note", Valid: true}}, + } + conditions := []pr_db.Notification{ + {Condition: pgtype.Text{String: "library use only", Valid: true}}, + {Condition: pgtype.Text{String: "no renewal", Valid: true}}, + } + + data := GetPullSlipData(pr, notes, conditions, "barcode-base64") + + assert.Equal(t, PullSlipData{ + BorrowerName: "", + ReqId: "REQ-123", + PickupLocation: "Pickup Desk, Riga, LV-1050, LV", + Title: "Distributed Libraries", + Author: "Jane Doe", + DueDate: "2026-08-15", + ReturnAddress: "Return Room, Shelf B, Riga, LV", + BarcodeBase64: "barcode-base64", + ServiceType: "Loan", + ServiceLevel: "Rush", + SystemIdentifier: "SYS-456", + Publisher: "Index Press", + Volume: "7", + Issue: "2", + Pages: "18", + StaffNotes: "first note\nsecond note", + CallNumber: "QA76.73.G63", + LoanConditions: "library use only\nno renewal", + PatronName: "Ann", + PatronSurname: "Reader", + PatronId: "P-789", + }, data) +} + +func TestGetPullSlipData_UsesDefaultsWhenOptionalFieldsAreMissing(t *testing.T) { + data := GetPullSlipData(pr_db.PatronRequest{}, nil, nil, DEFAULT_FOR_NO_VALUE) + + assert.Equal(t, PullSlipData{ + BorrowerName: "", + ReqId: "", + PickupLocation: DEFAULT_FOR_NO_VALUE, + Title: DEFAULT_FOR_NO_VALUE, + Author: DEFAULT_FOR_NO_VALUE, + DueDate: DEFAULT_FOR_NO_VALUE, + ReturnAddress: DEFAULT_FOR_NO_VALUE, + BarcodeBase64: DEFAULT_FOR_NO_VALUE, + ServiceType: DEFAULT_FOR_NO_VALUE, + ServiceLevel: DEFAULT_FOR_NO_VALUE, + SystemIdentifier: DEFAULT_FOR_NO_VALUE, + Publisher: DEFAULT_FOR_NO_VALUE, + Volume: DEFAULT_FOR_NO_VALUE, + Issue: DEFAULT_FOR_NO_VALUE, + Pages: DEFAULT_FOR_NO_VALUE, + StaffNotes: DEFAULT_FOR_NO_VALUE, + CallNumber: DEFAULT_FOR_NO_VALUE, + LoanConditions: DEFAULT_FOR_NO_VALUE, + PatronName: DEFAULT_FOR_NO_VALUE, + PatronSurname: DEFAULT_FOR_NO_VALUE, + PatronId: DEFAULT_FOR_NO_VALUE, + }, data) +} + +// ── formatPhysicalAddress ───────────────────────────────────────────────────── + +func TestFormatPhysicalAddress_Full(t *testing.T) { + a := &iso18626.PhysicalAddress{ + Line1: "1 Main St", + Line2: "Floor 2", + Locality: "Springfield", + PostalCode: "12345", + Region: &iso18626.TypeSchemeValuePair{Text: "IL"}, + Country: &iso18626.TypeSchemeValuePair{Text: "US"}, + } + assert.Equal(t, "1 Main St, Floor 2, Springfield, 12345, IL, US", formatPhysicalAddress(a)) +} + +func TestFormatPhysicalAddress_Partial(t *testing.T) { + // Only Line1 and Locality — Region/Country nil, Line2/PostalCode empty + a := &iso18626.PhysicalAddress{ + Line1: "42 Book Rd", + Locality: "Shelbyville", + } + assert.Equal(t, "42 Book Rd, Shelbyville", formatPhysicalAddress(a)) +} + +func TestFormatPhysicalAddress_EmptyRegionText(t *testing.T) { + // Region present but empty Text — should be skipped + a := &iso18626.PhysicalAddress{ + Line1: "1 St", + Region: &iso18626.TypeSchemeValuePair{Text: ""}, + Country: &iso18626.TypeSchemeValuePair{Text: ""}, + } + assert.Equal(t, "1 St", formatPhysicalAddress(a)) +} + +func TestFormatPhysicalAddress_Empty(t *testing.T) { + assert.Equal(t, "", formatPhysicalAddress(&iso18626.PhysicalAddress{})) +} + +// ── getStaffNotes ───────────────────────────────────────────────────────────── + +func TestGetStaffNotes_Empty(t *testing.T) { + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getStaffNotes([]pr_db.Notification{})) +} + +func TestGetStaffNotes_InvalidNotesSkipped(t *testing.T) { + notes := []pr_db.Notification{ + {Note: pgtype.Text{String: "valid note", Valid: true}}, + {Note: pgtype.Text{String: "ignored", Valid: false}}, + } + assert.Equal(t, "valid note", getStaffNotes(notes)) +} + +func TestGetStaffNotes_Multiple(t *testing.T) { + notes := []pr_db.Notification{ + {Note: pgtype.Text{String: "note one", Valid: true}}, + {Note: pgtype.Text{String: "note two", Valid: true}}, + } + assert.Equal(t, "note one\nnote two", getStaffNotes(notes)) +} + +// ── getLoanConditions ───────────────────────────────────────────────────────── + +func TestGetLoanConditions_Empty(t *testing.T) { + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getLoanConditions([]pr_db.Notification{})) +} + +func TestGetLoanConditions_InvalidSkipped(t *testing.T) { + conditions := []pr_db.Notification{ + {Condition: pgtype.Text{String: "library use only", Valid: true}}, + {Condition: pgtype.Text{String: "ignored", Valid: false}}, + } + assert.Equal(t, "library use only", getLoanConditions(conditions)) +} + +func TestGetLoanConditions_Multiple(t *testing.T) { + conditions := []pr_db.Notification{ + {Condition: pgtype.Text{String: "no photocopying", Valid: true}}, + {Condition: pgtype.Text{String: "in-library use", Valid: true}}, + } + assert.Equal(t, "no photocopying\nin-library use", getLoanConditions(conditions)) +} + +// ── getCallNumber ───────────────────────────────────────────────────────────── + +func TestGetCallNumber_Empty(t *testing.T) { + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr_db.PatronRequest{})) +} + +func TestGetCallNumber_NilCallNumber(t *testing.T) { + pr := pr_db.PatronRequest{Items: []pr_db.PrItem{{ID: "i1", CallNumber: nil}}} + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr)) +} + +func TestGetCallNumber_EmptyCallNumber(t *testing.T) { + empty := "" + pr := pr_db.PatronRequest{Items: []pr_db.PrItem{{ID: "i1", CallNumber: &empty}}} + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr)) +} + +func TestGetCallNumber_Multiple(t *testing.T) { + cn1, cn2 := "QA76", "PR9199" + pr := pr_db.PatronRequest{Items: []pr_db.PrItem{ + {ID: "i1", CallNumber: &cn1}, + {ID: "i2", CallNumber: &cn2}, + }} + assert.Equal(t, "QA76, PR9199", getCallNumber(pr)) +} + +// ── getPickupLocation ───────────────────────────────────────────────────────── + +func TestGetPickupLocation_NoDeliveryInfo(t *testing.T) { + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr_db.PatronRequest{})) +} + +func TestGetPickupLocation_NilAddress(t *testing.T) { + pr := pr_db.PatronRequest{ + IllRequest: iso18626.Request{ + RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{{Address: nil}}, + }, + } + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr)) +} + +func TestGetPickupLocation_PhysicalAddress(t *testing.T) { + pr := pr_db.PatronRequest{ + IllRequest: iso18626.Request{ + RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ + {Address: &iso18626.Address{ + PhysicalAddress: &iso18626.PhysicalAddress{Line1: "Pickup Desk"}, + }}, + }, + }, + } + assert.Equal(t, "Pickup Desk", getPickupLocation(pr)) +} + +func TestGetPickupLocation_ElectronicAddress(t *testing.T) { + pr := pr_db.PatronRequest{ + IllRequest: iso18626.Request{ + RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ + {Address: &iso18626.Address{ + ElectronicAddress: &iso18626.ElectronicAddress{ + ElectronicAddressData: "patron@library.org", + }, + }}, + }, + }, + } + assert.Equal(t, "patron@library.org", getPickupLocation(pr)) +} + +func TestGetPickupLocation_AddressWithNoUsableFields(t *testing.T) { + // Address present but neither PhysicalAddress nor a non-empty ElectronicAddressData + pr := pr_db.PatronRequest{ + IllRequest: iso18626.Request{ + RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ + {Address: &iso18626.Address{}}, + }, + }, + } + assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr)) +} diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index dd5476f04..7d885fda4 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -759,6 +759,9 @@ components: description: Version of the state model in SemVer selector: $ref: '#/components/schemas/StateModelSelector' + pulllslipPdfTemplateLabel: + type: string + description: Template label used to resolve the PDF pullslip template for this state model. states: type: array description: A list of all allowed states diff --git a/broker/patron_request/api/api-handler_test.go b/broker/patron_request/api/api-handler_test.go index b22103934..67af42a6f 100644 --- a/broker/patron_request/api/api-handler_test.go +++ b/broker/patron_request/api/api-handler_test.go @@ -1100,7 +1100,7 @@ func TestGetStateModelTemplates(t *testing.T) { var templates []proapi.CreateTemplate err := json.Unmarshal(rr.Body.Bytes(), &templates) assert.NoError(t, err) - assert.Len(t, templates, 5) + assert.Len(t, templates, 6) labels := make([]string, 0, len(templates)) for _, template := range templates { assert.NotEmpty(t, template.Title) @@ -1113,6 +1113,7 @@ func TestGetStateModelTemplates(t *testing.T) { "cancelled-notification", "new-supply-request-notification", "pullslip-email", + "pullslip-pdf", }, labels) } diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index c3098bc85..6382004db 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -1711,7 +1711,7 @@ func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedCo if len(recipients) == 0 { result.Note = "no recipients found for patron" } else { - sendErr := a.createAndSendEmail(ctx, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToPatron) + sendErr := a.createAndSendEmail(ctx, pr, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToPatron) if sendErr != nil { return logNotificationErrorAndReturnSuccess(ctx, pr, "error sending email to patron", sendErr) } @@ -1731,7 +1731,7 @@ func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedCo } result.Note += "no recipients found for staff" } else { - sendErr := a.createAndSendEmail(ctx, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToStaff) + sendErr := a.createAndSendEmail(ctx, pr, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToStaff) if sendErr != nil { return logNotificationErrorAndReturnSuccess(ctx, pr, "error sending email to staff", sendErr) } @@ -1742,7 +1742,7 @@ func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedCo return actionExecutionResult{status: events.EventStatusSuccess, result: &result, pr: pr} } -func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedContext, symbol string, from string, recipients []string, label string, audience proapi.ModelActionParamsSendTo) error { +func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedContext, pr pr_db.PatronRequest, symbol string, from string, recipients []string, label string, audience proapi.ModelActionParamsSendTo) error { template, err := a.prRepo.GetTemplateByPurposeAudienceLabelAndOwner(ctx, pr_db.GetTemplateByPurposeAudienceLabelAndOwnerParams{ Purpose: string(proapi.Email), Owner: symbol, @@ -1752,10 +1752,27 @@ func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedConte if err != nil { return err } + body := template.Body + if template.ContentType == string(proapi.Html) { + notes, _, ifErr := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindNote)}) + if ifErr != nil { + return ifErr + } + conditions, _, ifErr := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindCondition)}) + if ifErr != nil { + return ifErr + } + data := email.GetPullSlipData(pr, notes, conditions, email.DEFAULT_FOR_NO_VALUE) + html, ifErr := email.RenderPullSlipHTMLWithTemplate(data, template.Body) + if ifErr != nil { + return ifErr + } + body = html + } emailData := email.EmailData{ To: recipients, Subject: template.Subject.String, - Body: template.Body, + Body: body, IsHTML: template.ContentType == string(proapi.Html), IncludePdf: false, } diff --git a/broker/patron_request/service/action_mapping.go b/broker/patron_request/service/action_mapping.go index faa5965a2..f92018bb6 100644 --- a/broker/patron_request/service/action_mapping.go +++ b/broker/patron_request/service/action_mapping.go @@ -27,6 +27,14 @@ func (r *ActionMappingService) GetActionMapping(request iso18626.Request) (*Acti return mapping, err } +func (r *ActionMappingService) GetStateModelForRequest(request iso18626.Request) (*proapi.StateModel, error) { + name, _, err := r.ResolveActionMapping(request) + if err != nil { + return nil, err + } + return r.getStateModelService().GetStateModel(name) +} + // ResolveActionMapping selects the state model once and returns both the // persisted model key and the effective mapping for the request's service type. func (r *ActionMappingService) ResolveActionMapping(request iso18626.Request) (string, *ActionMapping, error) { diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 8cf4c5a67..992372d4c 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -3785,6 +3785,8 @@ func TestCreateAndSendEmail(t *testing.T) { Subject: pgtype.Text{String: "Your request", Valid: true}, } + pr := pr_db.PatronRequest{IllRequest: iso18626.Request{}} + tests := []struct { name string from string @@ -3859,7 +3861,7 @@ func TestCreateAndSendEmail(t *testing.T) { tc.setupEmail(mockEmail) svc := newActionServiceWithEmail(mockPrRepo, mockEmail) - err := svc.createAndSendEmail(appCtx, symbol, tc.from, tc.recipients, label, audience) + err := svc.createAndSendEmail(appCtx, pr, symbol, tc.from, tc.recipients, label, audience) if tc.wantErrSubstr == "" { assert.NoError(t, err) diff --git a/broker/patron_request/service/statemodel_test.go b/broker/patron_request/service/statemodel_test.go index d5b47926c..d94e5d03c 100644 --- a/broker/patron_request/service/statemodel_test.go +++ b/broker/patron_request/service/statemodel_test.go @@ -219,6 +219,38 @@ func TestDefaultIncludesLocalSupplyRequesterState(t *testing.T) { assert.NotEqual(t, -1, stateIndex) } +func TestReturnablesPullslipPdfTemplateLabel(t *testing.T) { + model, err := LoadStateModelByName("returnables") + if !assert.NoError(t, err) || !assert.NotNil(t, model) { + return + } + + if assert.NotNil(t, model.PulllslipPdfTemplateLabel) { + assert.Equal(t, "pullslip-pdf", *model.PulllslipPdfTemplateLabel) + } +} + +func TestStateModelTemplateDefaultsIncludePullslipPdf(t *testing.T) { + templates := GetStateModelTemplateDefaults() + + idx := slices.IndexFunc(templates, func(template proapi.CreateTemplate) bool { + return slices.Contains(template.Labels, "pullslip-pdf") + }) + if !assert.NotEqual(t, -1, idx) { + return + } + + template := templates[idx] + assert.Equal(t, "Pullslip PDF template", template.Title) + assert.Equal(t, proapi.Pullslip, template.Purpose) + assert.Equal(t, proapi.Html, template.ContentType) + if assert.NotNil(t, template.Audience) { + assert.Equal(t, proapi.TemplateAudienceStaff, *template.Audience) + } + assert.Contains(t, template.Body, "{{.BarcodeBase64}}") + assert.Contains(t, template.Body, "{{.ReqId}}") +} + func TestDefaultInvalidPatronStateIsEditableAndNeedsAttention(t *testing.T) { model, err := LoadStateModelByName("default") if !assert.NoError(t, err) || !assert.NotNil(t, model) { diff --git a/broker/pullslip/api/api_handler_test.go b/broker/pullslip/api/api_handler_test.go index db5f7e0bd..4c809ecef 100644 --- a/broker/pullslip/api/api_handler_test.go +++ b/broker/pullslip/api/api_handler_test.go @@ -5,12 +5,14 @@ import ( "errors" "net/http" "net/http/httptest" + "slices" "strings" "testing" "github.com/indexdata/cql-go/pgcql" "github.com/indexdata/crosslink/broker/common" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + prservice "github.com/indexdata/crosslink/broker/patron_request/service" ps_db "github.com/indexdata/crosslink/broker/pullslip/db" psoapi "github.com/indexdata/crosslink/broker/pullslip/oapi" "github.com/indexdata/crosslink/broker/tenant" @@ -52,6 +54,17 @@ func (m *MockPrRepo) GetNotificationsByPrId(ctx common.ExtendedContext, params p return args.Get(0).([]pr_db.Notification), args.Get(1).(int64), args.Error(2) } +func (m *MockPrRepo) GetTemplateByPurposeAudienceLabelAndOwner(_ common.ExtendedContext, params pr_db.GetTemplateByPurposeAudienceLabelAndOwnerParams) (pr_db.Template, error) { + for _, t := range prservice.GetStateModelTemplateDefaults() { + if slices.Contains(t.Labels, params.Label) { + return pr_db.Template{ + Body: t.Body, + }, nil + } + } + return pr_db.Template{}, nil +} + // ── helpers ─────────────────────────────────────────────────────────────────── var sym = "ISIL:TEST" diff --git a/broker/pullslip/service/pdf.go b/broker/pullslip/service/pdf.go index 9c47a7e6c..f6dc7e570 100644 --- a/broker/pullslip/service/pdf.go +++ b/broker/pullslip/service/pdf.go @@ -4,60 +4,36 @@ import ( "bytes" _ "embed" "encoding/base64" - "html/template" + "errors" "image/png" - "strings" "github.com/boombuler/barcode" "github.com/boombuler/barcode/code128" "github.com/carlos7ags/folio/document" "github.com/carlos7ags/folio/reader" "github.com/indexdata/crosslink/broker/common" + "github.com/indexdata/crosslink/broker/email" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" - "github.com/indexdata/crosslink/iso18626" + "github.com/indexdata/crosslink/broker/patron_request/proapi" + prservice "github.com/indexdata/crosslink/broker/patron_request/service" ) -const DEFAULT_FOR_NO_VALUE = "n/a" -const DATE_LAYOUT = "2006-01-02" - type PdfService interface { GeneratePdfPullSlipForPrs(ctx common.ExtendedContext, prs []pr_db.PatronRequest) ([]byte, error) } type PdfServiceImpl struct { - prRepo pr_db.PrRepo + prRepo pr_db.PrRepo + actionMappingService prservice.ActionMappingService } func NewPdfService(prRepo pr_db.PrRepo) PdfService { return &PdfServiceImpl{ - prRepo: prRepo, + prRepo: prRepo, + actionMappingService: prservice.ActionMappingService{SMService: &prservice.StateModelService{}}, } } -type PullSlipData struct { - BorrowerName string - ReqId string - PickupLocation string - Title string - Author string - DueDate string - ReturnAddress string - BarcodeBase64 string - ServiceType string - ServiceLevel string - SystemIdentifier string - Publisher string - Volume string - Issue string - Pages string - StaffNotes string - CallNumber string - LoanConditions string -} - -//go:embed pull_slip_template.html -var pullSlipTemplate string - func (p *PdfServiceImpl) GeneratePdfPullSlipForPrs(ctx common.ExtendedContext, prs []pr_db.PatronRequest) ([]byte, error) { pdfs := []*reader.PdfReader{} for _, pr := range prs { @@ -69,7 +45,7 @@ func (p *PdfServiceImpl) GeneratePdfPullSlipForPrs(ctx common.ExtendedContext, p if err != nil { return []byte{}, err } - pdf, err := p.GeneratePdfPullSlip(pr, notes, conditions) + pdf, err := p.GeneratePdfPullSlip(ctx, pr, notes, conditions) if err != nil { return []byte{}, err } @@ -90,68 +66,20 @@ func (p *PdfServiceImpl) GeneratePdfPullSlipForPrs(ctx common.ExtendedContext, p return buf.Bytes(), nil } -func (p *PdfServiceImpl) GeneratePdfPullSlip(pr pr_db.PatronRequest, notes []pr_db.Notification, conditions []pr_db.Notification) ([]byte, error) { +func (p *PdfServiceImpl) GeneratePdfPullSlip(ctx common.ExtendedContext, pr pr_db.PatronRequest, notes []pr_db.Notification, conditions []pr_db.Notification) ([]byte, error) { barcodeData, err := getBarcodeBase64(pr.RequesterReqID.String) if err != nil { return nil, err } - data := PullSlipData{ - ReqId: pr.RequesterReqID.String, - PickupLocation: getPickupLocation(pr), - Title: DEFAULT_FOR_NO_VALUE, - Author: DEFAULT_FOR_NO_VALUE, - DueDate: DEFAULT_FOR_NO_VALUE, - ReturnAddress: DEFAULT_FOR_NO_VALUE, - BarcodeBase64: barcodeData, - ServiceType: DEFAULT_FOR_NO_VALUE, - ServiceLevel: DEFAULT_FOR_NO_VALUE, - SystemIdentifier: DEFAULT_FOR_NO_VALUE, - Publisher: DEFAULT_FOR_NO_VALUE, - Volume: DEFAULT_FOR_NO_VALUE, - Issue: DEFAULT_FOR_NO_VALUE, - Pages: DEFAULT_FOR_NO_VALUE, - StaffNotes: getStaffNotes(notes), - CallNumber: getCallNumber(pr), - LoanConditions: getLoanConditions(conditions), - } - if pr.IllRequest.BibliographicInfo.Author != "" { - data.Author = pr.IllRequest.BibliographicInfo.Author - } - if pr.IllRequest.BibliographicInfo.Title != "" { - data.Title = pr.IllRequest.BibliographicInfo.Title - } - if pr.IllRequest.BibliographicInfo.Volume != "" { - data.Volume = pr.IllRequest.BibliographicInfo.Volume - } - if pr.IllRequest.BibliographicInfo.Issue != "" { - data.Issue = pr.IllRequest.BibliographicInfo.Issue - } - if pr.IllRequest.BibliographicInfo.EstimatedNoPages != "" { - data.Pages = pr.IllRequest.BibliographicInfo.EstimatedNoPages - } - if pr.IllRequest.BibliographicInfo.SupplierUniqueRecordId != "" { - data.SystemIdentifier = pr.IllRequest.BibliographicInfo.SupplierUniqueRecordId - } - if pr.IllRequest.PublicationInfo != nil && pr.IllRequest.PublicationInfo.Publisher != "" { - data.Publisher = pr.IllRequest.PublicationInfo.Publisher - } - if pr.IllResponse.StatusInfo.DueDate != nil { - data.DueDate = pr.IllResponse.StatusInfo.DueDate.Format(DATE_LAYOUT) - } - if pr.IllResponse.ReturnInfo != nil && pr.IllResponse.ReturnInfo.PhysicalAddress != nil { - data.ReturnAddress = formatPhysicalAddress(pr.IllResponse.ReturnInfo.PhysicalAddress) - } - if pr.IllRequest.ServiceInfo != nil { - if pr.IllRequest.ServiceInfo.ServiceLevel != nil && pr.IllRequest.ServiceInfo.ServiceLevel.Text != "" { - data.ServiceLevel = pr.IllRequest.ServiceInfo.ServiceLevel.Text - } - if pr.IllRequest.ServiceInfo.ServiceType != "" { - data.ServiceType = string(pr.IllRequest.ServiceInfo.ServiceType) - } + templateBody, err := p.getTemplateForPatronRequest(ctx, pr) + if err != nil { + return []byte{}, err } + doc := document.NewDocument(document.PageSizeA4) - html, err := renderPullSlipHTML(data) + data := email.GetPullSlipData(pr, notes, conditions, barcodeData) + html, err := email.RenderPullSlipHTMLWithTemplate(data, templateBody) if err != nil { return nil, err } @@ -164,16 +92,31 @@ func (p *PdfServiceImpl) GeneratePdfPullSlip(pr pr_db.PatronRequest, notes []pr_ return doc.ToBytes() } -func renderPullSlipHTML(data PullSlipData) (string, error) { - tmpl, err := template.New("pull-slip").Parse(pullSlipTemplate) +func (p *PdfServiceImpl) getTemplateForPatronRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) (string, error) { + stateModel, err := p.actionMappingService.GetStateModelForRequest(pr.IllRequest) if err != nil { return "", err } - var buf bytes.Buffer - if err := tmpl.Execute(&buf, data); err != nil { + if stateModel.PulllslipPdfTemplateLabel == nil || *stateModel.PulllslipPdfTemplateLabel == "" { + return "", errors.New("pulllslipPdfTemplateLabel field is required") + } + owner := pr.RequesterSymbol + if pr.Side == prservice.SideLending { + owner = pr.SupplierSymbol + } + pdfTemplate, err := p.prRepo.GetTemplateByPurposeAudienceLabelAndOwner(ctx, pr_db.GetTemplateByPurposeAudienceLabelAndOwnerParams{ + Owner: owner.String, + Purpose: string(proapi.Pullslip), + Label: *stateModel.PulllslipPdfTemplateLabel, + Audience: string(proapi.ModelActionParamsSendToStaff), + }) + if err != nil { return "", err } - return buf.String(), nil + if pdfTemplate.Body == "" { + return "", errors.New("invalid pullslip pdf template, body field is required") + } + return pdfTemplate.Body, nil } // barcodeWidth calculates a suitable barcode pixel width based on the number @@ -211,80 +154,3 @@ func getBarcodeBase64(data string) (string, error) { } return base64.StdEncoding.EncodeToString(buf.Bytes()), nil } - -func formatPhysicalAddress(a *iso18626.PhysicalAddress) string { - parts := []string{} - if a.Line1 != "" { - parts = append(parts, a.Line1) - } - if a.Line2 != "" { - parts = append(parts, a.Line2) - } - if a.Locality != "" { - parts = append(parts, a.Locality) - } - if a.PostalCode != "" { - parts = append(parts, a.PostalCode) - } - if a.Region != nil && a.Region.Text != "" { - parts = append(parts, a.Region.Text) - } - if a.Country != nil && a.Country.Text != "" { - parts = append(parts, a.Country.Text) - } - return strings.Join(parts, ", ") -} - -func getStaffNotes(noteList []pr_db.Notification) string { - noteStrings := []string{} - for _, note := range noteList { - if note.Note.Valid { - noteStrings = append(noteStrings, note.Note.String) - } - } - notes := strings.Join(noteStrings, "\n") - if notes == "" { - return DEFAULT_FOR_NO_VALUE - } - return notes -} - -func getLoanConditions(conditionList []pr_db.Notification) string { - conditionStrings := []string{} - for _, note := range conditionList { - if note.Condition.Valid { - conditionStrings = append(conditionStrings, note.Condition.String) - } - } - conditions := strings.Join(conditionStrings, "\n") - if conditions == "" { - return DEFAULT_FOR_NO_VALUE - } - return conditions -} - -func getCallNumber(request pr_db.PatronRequest) string { - callNumberStrings := []string{} - for _, item := range request.Items { - if item.CallNumber != nil && *item.CallNumber != "" { - callNumberStrings = append(callNumberStrings, *item.CallNumber) - } - } - callNumber := strings.Join(callNumberStrings, ", ") - if callNumber == "" { - return DEFAULT_FOR_NO_VALUE - } - return callNumber -} - -func getPickupLocation(request pr_db.PatronRequest) string { - if len(request.IllRequest.RequestedDeliveryInfo) > 0 && request.IllRequest.RequestedDeliveryInfo[0].Address != nil { - address := *request.IllRequest.RequestedDeliveryInfo[0].Address - if address.PhysicalAddress != nil { - return formatPhysicalAddress(address.PhysicalAddress) - } else if address.ElectronicAddress != nil && address.ElectronicAddress.ElectronicAddressData != "" { - return address.ElectronicAddress.ElectronicAddressData - } - } - return DEFAULT_FOR_NO_VALUE -} diff --git a/broker/pullslip/service/pdf_test.go b/broker/pullslip/service/pdf_test.go index e4fac4505..2eb1bd19b 100644 --- a/broker/pullslip/service/pdf_test.go +++ b/broker/pullslip/service/pdf_test.go @@ -6,12 +6,13 @@ import ( "encoding/base64" "errors" "image/png" - "strings" + "slices" "testing" "time" "github.com/indexdata/crosslink/broker/common" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + prservice "github.com/indexdata/crosslink/broker/patron_request/service" "github.com/indexdata/crosslink/iso18626" "github.com/indexdata/go-utils/utils" "github.com/jackc/pgx/v5/pgtype" @@ -52,47 +53,9 @@ func TestGetBarcodeBase64(t *testing.T) { assert.Equal(t, 67, bounds.Dy()) } -func TestRenderPullSlipHTML(t *testing.T) { - html, err := renderPullSlipHTML(PullSlipData{ - ReqId: "REQ-123", - PickupLocation: "Main Library", - Title: "Big Shark", - Author: "John Doe", - DueDate: "2026-01-01", - ReturnAddress: "1 Test Street", - BarcodeBase64: "abc123", - }) - assert.NoError(t, err) - assert.True(t, strings.Contains(html, "REQ-123")) - assert.True(t, strings.Contains(html, "Main Library")) - assert.True(t, strings.Contains(html, "data:image/png;base64,abc123")) -} - -func TestRenderPullSlipHTML_InvalidTemplate(t *testing.T) { - // Temporarily swap pullSlipTemplate with an invalid one - orig := pullSlipTemplate - defer func() { pullSlipTemplate = orig }() - pullSlipTemplate = `{{.Unclosed` - - _, err := renderPullSlipHTML(PullSlipData{ReqId: "X"}) - assert.Error(t, err) -} - -func TestRenderPullSlipHTML_ExecuteError(t *testing.T) { - // A template that calls a function on a field that panics/errors at execute time - orig := pullSlipTemplate - defer func() { pullSlipTemplate = orig }() - // Use a template that references a non-existent function to trigger execute error - // The only reliable way in Go templates: call.option "missingkey=error" with unknown key on a map - pullSlipTemplate = `{{index . "nonexistent"}}` - - _, err := renderPullSlipHTML(PullSlipData{ReqId: "X"}) - // Execute on a struct with map-access fails - assert.Error(t, err) -} - func TestGeneratePdfPullSlip_Defaults(t *testing.T) { - svc := &PdfServiceImpl{} + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ RequesterReqID: pgtype.Text{ String: "REQ-DEFAULTS", @@ -100,7 +63,7 @@ func TestGeneratePdfPullSlip_Defaults(t *testing.T) { }, // No bibliographic info — all fields should fall back to DEFAULT_FOR_NO_VALUE } - pdfBytes, err := svc.GeneratePdfPullSlip(pr, []pr_db.Notification{}, []pr_db.Notification{}) + pdfBytes, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) assert.NoError(t, err) assert.NotEmpty(t, pdfBytes) // PDF magic bytes: %PDF @@ -108,7 +71,8 @@ func TestGeneratePdfPullSlip_Defaults(t *testing.T) { } func TestGeneratePdfPullSlip_WithBibliographicInfo(t *testing.T) { - svc := &PdfServiceImpl{} + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ ID: "REQ-BIB", RequesterReqID: pgtype.Text{ @@ -122,7 +86,7 @@ func TestGeneratePdfPullSlip_WithBibliographicInfo(t *testing.T) { }, }, } - pdfBytes, err := svc.GeneratePdfPullSlip(pr, []pr_db.Notification{}, []pr_db.Notification{}) + pdfBytes, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) assert.NoError(t, err) assert.NotEmpty(t, pdfBytes) assert.Equal(t, "%PDF", string(pdfBytes[:4])) @@ -132,7 +96,8 @@ func TestGeneratePdfPullSlip_FullData(t *testing.T) { callNumber := "QA76.9.A25" dueDate := utils.XSDDateTime{Time: time.Date(2026, 8, 15, 0, 0, 0, 0, time.UTC)} - svc := &PdfServiceImpl{} + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ ID: "REQ-FULL", RequesterReqID: pgtype.Text{ @@ -195,7 +160,7 @@ func TestGeneratePdfPullSlip_FullData(t *testing.T) { {Condition: pgtype.Text{String: "No photocopying", Valid: true}}, } - pdfBytes, err := svc.GeneratePdfPullSlip(pr, notes, conditions) + pdfBytes, err := svc.GeneratePdfPullSlip(appCtx, pr, notes, conditions) assert.NoError(t, err) assert.NotEmpty(t, pdfBytes) assert.Equal(t, "%PDF", string(pdfBytes[:4])) @@ -215,187 +180,53 @@ func TestGeneratePdfPullSlip_BarcodeError(t *testing.T) { Valid: true, }, } - _, err := svc.GeneratePdfPullSlip(pr, []pr_db.Notification{}, []pr_db.Notification{}) + _, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) assert.Error(t, err) } func TestGeneratePdfPullSlip_TemplateError(t *testing.T) { - orig := pullSlipTemplate - defer func() { pullSlipTemplate = orig }() - pullSlipTemplate = `{{.Unclosed` - - svc := &PdfServiceImpl{} + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ ID: "REQ-X", RequesterReqID: pgtype.Text{ String: "REQ-X", Valid: true, }, + RequesterSymbol: pgtype.Text{String: "invalid", Valid: true}, } - _, err := svc.GeneratePdfPullSlip(pr, []pr_db.Notification{}, []pr_db.Notification{}) + _, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) assert.Error(t, err) } -// ── formatPhysicalAddress ───────────────────────────────────────────────────── - -func TestFormatPhysicalAddress_Full(t *testing.T) { - a := &iso18626.PhysicalAddress{ - Line1: "1 Main St", - Line2: "Floor 2", - Locality: "Springfield", - PostalCode: "12345", - Region: &iso18626.TypeSchemeValuePair{Text: "IL"}, - Country: &iso18626.TypeSchemeValuePair{Text: "US"}, - } - assert.Equal(t, "1 Main St, Floor 2, Springfield, 12345, IL, US", formatPhysicalAddress(a)) -} - -func TestFormatPhysicalAddress_Partial(t *testing.T) { - // Only Line1 and Locality — Region/Country nil, Line2/PostalCode empty - a := &iso18626.PhysicalAddress{ - Line1: "42 Book Rd", - Locality: "Shelbyville", - } - assert.Equal(t, "42 Book Rd, Shelbyville", formatPhysicalAddress(a)) -} - -func TestFormatPhysicalAddress_EmptyRegionText(t *testing.T) { - // Region present but empty Text — should be skipped - a := &iso18626.PhysicalAddress{ - Line1: "1 St", - Region: &iso18626.TypeSchemeValuePair{Text: ""}, - Country: &iso18626.TypeSchemeValuePair{Text: ""}, - } - assert.Equal(t, "1 St", formatPhysicalAddress(a)) -} - -func TestFormatPhysicalAddress_Empty(t *testing.T) { - assert.Equal(t, "", formatPhysicalAddress(&iso18626.PhysicalAddress{})) -} - -// ── getStaffNotes ───────────────────────────────────────────────────────────── - -func TestGetStaffNotes_Empty(t *testing.T) { - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getStaffNotes([]pr_db.Notification{})) -} - -func TestGetStaffNotes_InvalidNotesSkipped(t *testing.T) { - notes := []pr_db.Notification{ - {Note: pgtype.Text{String: "valid note", Valid: true}}, - {Note: pgtype.Text{String: "ignored", Valid: false}}, - } - assert.Equal(t, "valid note", getStaffNotes(notes)) -} - -func TestGetStaffNotes_Multiple(t *testing.T) { - notes := []pr_db.Notification{ - {Note: pgtype.Text{String: "note one", Valid: true}}, - {Note: pgtype.Text{String: "note two", Valid: true}}, - } - assert.Equal(t, "note one\nnote two", getStaffNotes(notes)) -} - -// ── getLoanConditions ───────────────────────────────────────────────────────── - -func TestGetLoanConditions_Empty(t *testing.T) { - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getLoanConditions([]pr_db.Notification{})) -} - -func TestGetLoanConditions_InvalidSkipped(t *testing.T) { - conditions := []pr_db.Notification{ - {Condition: pgtype.Text{String: "library use only", Valid: true}}, - {Condition: pgtype.Text{String: "ignored", Valid: false}}, - } - assert.Equal(t, "library use only", getLoanConditions(conditions)) -} - -func TestGetLoanConditions_Multiple(t *testing.T) { - conditions := []pr_db.Notification{ - {Condition: pgtype.Text{String: "no photocopying", Valid: true}}, - {Condition: pgtype.Text{String: "in-library use", Valid: true}}, - } - assert.Equal(t, "no photocopying\nin-library use", getLoanConditions(conditions)) -} - -// ── getCallNumber ───────────────────────────────────────────────────────────── - -func TestGetCallNumber_Empty(t *testing.T) { - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr_db.PatronRequest{})) -} - -func TestGetCallNumber_NilCallNumber(t *testing.T) { - pr := pr_db.PatronRequest{Items: []pr_db.PrItem{{ID: "i1", CallNumber: nil}}} - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr)) -} - -func TestGetCallNumber_EmptyCallNumber(t *testing.T) { - empty := "" - pr := pr_db.PatronRequest{Items: []pr_db.PrItem{{ID: "i1", CallNumber: &empty}}} - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getCallNumber(pr)) -} - -func TestGetCallNumber_Multiple(t *testing.T) { - cn1, cn2 := "QA76", "PR9199" - pr := pr_db.PatronRequest{Items: []pr_db.PrItem{ - {ID: "i1", CallNumber: &cn1}, - {ID: "i2", CallNumber: &cn2}, - }} - assert.Equal(t, "QA76, PR9199", getCallNumber(pr)) -} - -// ── getPickupLocation ───────────────────────────────────────────────────────── - -func TestGetPickupLocation_NoDeliveryInfo(t *testing.T) { - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr_db.PatronRequest{})) -} - -func TestGetPickupLocation_NilAddress(t *testing.T) { - pr := pr_db.PatronRequest{ - IllRequest: iso18626.Request{ - RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{{Address: nil}}, - }, - } - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr)) -} - -func TestGetPickupLocation_PhysicalAddress(t *testing.T) { - pr := pr_db.PatronRequest{ - IllRequest: iso18626.Request{ - RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ - {Address: &iso18626.Address{ - PhysicalAddress: &iso18626.PhysicalAddress{Line1: "Pickup Desk"}, - }}, - }, - }, - } - assert.Equal(t, "Pickup Desk", getPickupLocation(pr)) -} - -func TestGetPickupLocation_ElectronicAddress(t *testing.T) { +func TestGeneratePdfPullSlip_TemplateEmpty(t *testing.T) { + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ - IllRequest: iso18626.Request{ - RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ - {Address: &iso18626.Address{ - ElectronicAddress: &iso18626.ElectronicAddress{ - ElectronicAddressData: "patron@library.org", - }, - }}, - }, + ID: "REQ-X", + RequesterReqID: pgtype.Text{ + String: "REQ-X", + Valid: true, }, + RequesterSymbol: pgtype.Text{String: "empty", Valid: true}, } - assert.Equal(t, "patron@library.org", getPickupLocation(pr)) + _, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) + assert.Error(t, err) } -func TestGetPickupLocation_AddressWithNoUsableFields(t *testing.T) { - // Address present but neither PhysicalAddress nor a non-empty ElectronicAddressData +func TestGeneratePdfPullSlip_TemplateDbError(t *testing.T) { + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ - IllRequest: iso18626.Request{ - RequestedDeliveryInfo: []iso18626.RequestedDeliveryInfo{ - {Address: &iso18626.Address{}}, - }, + ID: "REQ-X", + RequesterReqID: pgtype.Text{ + String: "REQ-X", + Valid: true, }, + RequesterSymbol: pgtype.Text{String: "error", Valid: true}, } - assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr)) + _, err := svc.GeneratePdfPullSlip(appCtx, pr, []pr_db.Notification{}, []pr_db.Notification{}) + assert.Error(t, err) } // ── GeneratePdfPullSlipForPrs ───────────────────────────────────────────────── @@ -415,6 +246,26 @@ func (m *mockPrRepo) GetNotificationsByPrId(_ common.ExtendedContext, params pr_ return m.conditions, int64(len(m.conditions)), m.condErr } +func (m *mockPrRepo) GetTemplateByPurposeAudienceLabelAndOwner(_ common.ExtendedContext, params pr_db.GetTemplateByPurposeAudienceLabelAndOwnerParams) (pr_db.Template, error) { + if params.Owner == "invalid" { + return pr_db.Template{Body: "{{.Unclosed"}, nil + } + if params.Owner == "empty" { + return pr_db.Template{}, nil + } + if params.Owner == "error" { + return pr_db.Template{}, errors.New("template db error") + } + for _, t := range prservice.GetStateModelTemplateDefaults() { + if slices.Contains(t.Labels, params.Label) { + return pr_db.Template{ + Body: t.Body, + }, nil + } + } + return pr_db.Template{}, nil +} + func newSvcWithMock(repo pr_db.PrRepo) *PdfServiceImpl { return &PdfServiceImpl{prRepo: repo} } @@ -462,7 +313,8 @@ func TestGeneratePdfPullSlipForPrs_ConditionError(t *testing.T) { // ── ServiceInfo edge cases ──────────────────────────────────────────────────── func TestGeneratePdfPullSlip_ServiceInfoEmptyServiceLevel(t *testing.T) { - svc := &PdfServiceImpl{} + repo := &mockPrRepo{} + svc := newSvcWithMock(repo) pr := pr_db.PatronRequest{ RequesterReqID: pgtype.Text{String: "REQ-SVC", Valid: true}, IllRequest: iso18626.Request{ @@ -472,7 +324,7 @@ func TestGeneratePdfPullSlip_ServiceInfoEmptyServiceLevel(t *testing.T) { }, }, } - pdfBytes, err := svc.GeneratePdfPullSlip(pr, nil, nil) + pdfBytes, err := svc.GeneratePdfPullSlip(appCtx, pr, nil, nil) assert.NoError(t, err) assert.NotEmpty(t, pdfBytes) } diff --git a/broker/pullslip/service/pull_slip_template.html b/broker/pullslip/service/pull_slip_template.html deleted file mode 100644 index b6e8ff9b8..000000000 --- a/broker/pullslip/service/pull_slip_template.html +++ /dev/null @@ -1,120 +0,0 @@ - - - - Pull slip - - - -
-
-
Borrower & More
- Barcode - {{.ReqId}} -
- -
-
- Pickup Location - {{.PickupLocation}} -
-
- -
- Service Type: {{.ServiceType}}
- Service Level: {{.ServiceLevel}}
- System Identifier: {{.SystemIdentifier}}
- Title: {{.Title}}
- Author: {{.Author}}
- Publisher: {{.Publisher}}
- Volume(s): {{.Volume}}
- Issue: {{.Issue}}
- Pages: {{.Pages}}
-
- -
- DO NOT REMOVE THIS SLIP
- This material is on loan from a Trove Partner -
-
-
- Staff Notes: - 
-                {{.StaffNotes}}
-
-

- Call Number: - {{.CallNumber}} -
-
-
-
- Due Date: - {{.DueDate}} -

- Loan Conditions: - 
-                {{.LoanConditions}}
-
-
-
- -
-
- Return Address - {{.ReturnAddress}} -
-
-
- - \ No newline at end of file diff --git a/broker/scheduler/service/email_sender.go b/broker/scheduler/service/email_sender.go index 8a7e109b8..b28a23063 100644 --- a/broker/scheduler/service/email_sender.go +++ b/broker/scheduler/service/email_sender.go @@ -3,7 +3,6 @@ package sched_service import ( "errors" "fmt" - "strings" "github.com/indexdata/crosslink/broker/common" "github.com/indexdata/crosslink/broker/email" @@ -134,15 +133,11 @@ func (s *EmailSenderService) generateAndEmailPullslip(ctx common.ExtendedContext pdfAttachment = &email.PdfAttach{Filename: "pull-slips.pdf", Data: pdfBytes} } - placeholders := map[string]string{ - "fullCount": fmt.Sprintf("%d", fullCount), - "actualCount": fmt.Sprintf("%d", len(prs)), - "batchQuery": event.EventData.BatchActionData.Selector, - } + placeholders := email.GetBatchEmailData(fullCount, len(prs), event.EventData.BatchActionData.Selector) messageData := email.EmailData{ To: emailData.To, - Subject: renderEmailTemplate(template.Subject.String, placeholders), - Body: renderEmailTemplate(template.Body, placeholders), + Subject: email.RenderBatchEmailTemplate(template.Subject.String, placeholders), + Body: email.RenderBatchEmailTemplate(template.Body, placeholders), IsHTML: template.ContentType == string(proapi.Html), IncludePdf: emailData.IncludePdf, } @@ -159,13 +154,6 @@ func (s *EmailSenderService) generateAndEmailPullslip(ctx common.ExtendedContext return events.EventStatusSuccess, nil } -func renderEmailTemplate(value string, placeholders map[string]string) string { - for key, replacement := range placeholders { - value = strings.ReplaceAll(value, "{{"+key+"}}", replacement) - } - return value -} - // extractEmailData retrieves email pullslip parameters from the event's CustomData map. func extractEmailData(eventData events.EventData) (pullslipEmailData, error) { if eventData.CustomData == nil { diff --git a/broker/test/pullslip/api/api_handler_test.go b/broker/test/pullslip/api/api_handler_test.go index 86a8d0c6e..f550a929b 100644 --- a/broker/test/pullslip/api/api_handler_test.go +++ b/broker/test/pullslip/api/api_handler_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "os" + "slices" "strconv" "strings" "testing" @@ -115,6 +116,30 @@ func TestCreateSinglePullSlip(t *testing.T) { Items: []pr_db.PrItem{}, TerminalState: false, }) + + for _, tmpl := range prservice.GetStateModelTemplateDefaults() { + if slices.Contains(tmpl.Labels, "pullslip-pdf") { + _, inErr := prRepo.SaveTemplate(appCtx, pr_db.SaveTemplateParams{ + ID: "pullslip-pdf-1", + Owner: supSymbol, + Title: tmpl.Title, + Purpose: string(tmpl.Purpose), + Body: tmpl.Body, + ContentType: string(tmpl.ContentType), + Labels: tmpl.Labels, + Audience: pgtype.Text{ + String: string(*tmpl.Audience), + Valid: true, + }, + CreatedAt: pgtype.Timestamp{ + Time: time.Now(), + Valid: true, + }, + }) + assert.NoError(t, inErr, "failed to save template") + } + } + assert.NoError(t, err) // Create pull slip diff --git a/misc/state-models.yaml b/misc/state-models.yaml index 33fa06391..e4bc29884 100644 --- a/misc/state-models.yaml +++ b/misc/state-models.yaml @@ -4,6 +4,7 @@ stateModels: name: CrossLink State Model desc: "Requester/Supplier workflow for returnable loans and non-returnable copies over ISO18626." version: 3.3.0 + pulllslipPdfTemplateLabel: pullslip-pdf selector: serviceType: [Copy, Loan, CopyOrLoan] states: @@ -914,3 +915,131 @@ templateDefaults: Matching requests: {{actualCount}} (of {{fullCount}} total)

Please process the attached pull slips at your earliest convenience.

+ + - title: Pullslip PDF template + labels: + - pullslip-pdf + contentType: html + audience: staff + purpose: pullslip + body: | + + + + Pull slip + + + +
+
+
Borrower & More
+ Barcode + {{.ReqId}} +
+ +
+
+ Pickup Location + {{.PickupLocation}} +
+
+ +
+ Service Type: {{.ServiceType}}
+ Service Level: {{.ServiceLevel}}
+ System Identifier: {{.SystemIdentifier}}
+ Title: {{.Title}}
+ Author: {{.Author}}
+ Publisher: {{.Publisher}}
+ Volume(s): {{.Volume}}
+ Issue: {{.Issue}}
+ Pages: {{.Pages}}
+
+ +
+ DO NOT REMOVE THIS SLIP
+ This material is on loan from a Trove Partner +
+
+
+ Staff Notes: + 
+                      {{.StaffNotes}}
+
+

+ Call Number: + {{.CallNumber}} +
+
+
+
+ Due Date: + {{.DueDate}} +

+ Loan Conditions: + 
+                      {{.LoanConditions}}
+
+
+
+ +
+
+ Return Address + {{.ReturnAddress}} +
+
+
+ + From 94a471e0f07217d12530175a1f9d5ca2a3cb0c3c Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Tue, 25 Aug 2026 17:24:14 +0300 Subject: [PATCH 2/9] ILLDEV-466 Remove references of hardcoded template --- broker/Dockerfile.dockerignore | 3 -- broker/Makefile | 5 +-- broker/oapi/open-api.yaml | 2 +- .../patron_request/service/statemodel_test.go | 4 +- broker/pullslip/service/pdf.go | 6 +-- misc/state-models.yaml | 42 ++++++------------- 6 files changed, 21 insertions(+), 41 deletions(-) diff --git a/broker/Dockerfile.dockerignore b/broker/Dockerfile.dockerignore index 7b43bdcd2..c54502c7d 100644 --- a/broker/Dockerfile.dockerignore +++ b/broker/Dockerfile.dockerignore @@ -24,9 +24,6 @@ broker/**/*_test.go # Include statemodels !broker/patron_request/service/statemodels -# Include pull slip templates -!broker/pullslip/service/pull_slip_template.html - # Directory sources are generated inside the build. directory/api/directory.gen.go directory/db/db.go diff --git a/broker/Makefile b/broker/Makefile index 7e4f0482f..d4d7d68ad 100644 --- a/broker/Makefile +++ b/broker/Makefile @@ -22,7 +22,6 @@ GIT_COMMIT_DEPS = $(GIT_HEAD_FILE) $(wildcard $(GIT_HEAD_REF_FILE)) $(wildcard $ COVERAGE=coverage.out STATE_MODELS_JSON=patron_request/service/statemodels/state-models.json STATE_MODELS_YAML=../misc/state-models.yaml -PULLSLIP_TEMPLATE=pullslip/service/pull_slip_template.html # SQLC SQLC ?= $(GO) tool sqlc @@ -100,10 +99,10 @@ $(SQL_GEN_OUT): $(SQL_GEN_IN) $(SQLC_CONFIG) $(COMMIT_ID): $(GIT_COMMIT_DEPS) commit_id="$$( $(GIT) rev-parse --short HEAD )" && printf '%s' "$$commit_id" > $(COMMIT_ID) -$(BINARY): $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) $(PULLSLIP_TEMPLATE) +$(BINARY): $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) $(GO) build -v -o $(BINARY) ./$(MAIN_PACKAGE) -archive: $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) $(PULLSLIP_TEMPLATE) +archive: $(COMMIT_ID) $(SQL_GEN_OUT) $(OAPI_GEN) $(PR_OAPI_GEN) $(PS_OAPI_GEN) $(SCHED_OAPI_GEN) $(BUILD_GOFILES) $(STATE_MODELS_JSON) $(GO) build -v -o archive ./cmd/archive check: generate diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index 7d885fda4..a4ddcf655 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -759,7 +759,7 @@ components: description: Version of the state model in SemVer selector: $ref: '#/components/schemas/StateModelSelector' - pulllslipPdfTemplateLabel: + pullslipPdfTemplateLabel: type: string description: Template label used to resolve the PDF pullslip template for this state model. states: diff --git a/broker/patron_request/service/statemodel_test.go b/broker/patron_request/service/statemodel_test.go index d94e5d03c..a3b6f66f6 100644 --- a/broker/patron_request/service/statemodel_test.go +++ b/broker/patron_request/service/statemodel_test.go @@ -225,8 +225,8 @@ func TestReturnablesPullslipPdfTemplateLabel(t *testing.T) { return } - if assert.NotNil(t, model.PulllslipPdfTemplateLabel) { - assert.Equal(t, "pullslip-pdf", *model.PulllslipPdfTemplateLabel) + if assert.NotNil(t, model.PullslipPdfTemplateLabel) { + assert.Equal(t, "pullslip-pdf", *model.PullslipPdfTemplateLabel) } } diff --git a/broker/pullslip/service/pdf.go b/broker/pullslip/service/pdf.go index f6dc7e570..335c2f77e 100644 --- a/broker/pullslip/service/pdf.go +++ b/broker/pullslip/service/pdf.go @@ -97,8 +97,8 @@ func (p *PdfServiceImpl) getTemplateForPatronRequest(ctx common.ExtendedContext, if err != nil { return "", err } - if stateModel.PulllslipPdfTemplateLabel == nil || *stateModel.PulllslipPdfTemplateLabel == "" { - return "", errors.New("pulllslipPdfTemplateLabel field is required") + if stateModel.PullslipPdfTemplateLabel == nil || *stateModel.PullslipPdfTemplateLabel == "" { + return "", errors.New("pullslipPdfTemplateLabel field is required") } owner := pr.RequesterSymbol if pr.Side == prservice.SideLending { @@ -107,7 +107,7 @@ func (p *PdfServiceImpl) getTemplateForPatronRequest(ctx common.ExtendedContext, pdfTemplate, err := p.prRepo.GetTemplateByPurposeAudienceLabelAndOwner(ctx, pr_db.GetTemplateByPurposeAudienceLabelAndOwnerParams{ Owner: owner.String, Purpose: string(proapi.Pullslip), - Label: *stateModel.PulllslipPdfTemplateLabel, + Label: *stateModel.PullslipPdfTemplateLabel, Audience: string(proapi.ModelActionParamsSendToStaff), }) if err != nil { diff --git a/misc/state-models.yaml b/misc/state-models.yaml index e4bc29884..91c2a9a38 100644 --- a/misc/state-models.yaml +++ b/misc/state-models.yaml @@ -3,8 +3,8 @@ stateModels: type: StateModel name: CrossLink State Model desc: "Requester/Supplier workflow for returnable loans and non-returnable copies over ISO18626." - version: 3.3.0 - pulllslipPdfTemplateLabel: pullslip-pdf + version: 3.4.0 + pullslipPdfTemplateLabel: pullslip-pdf selector: serviceType: [Copy, Loan, CopyOrLoan] states: @@ -80,16 +80,16 @@ stateModels: - name: METADATA_UPDATED display: Metadata Updated - desc: Request preparation has completed and pre-send checks can run + desc: Request metadata update has completed or been skipped side: REQUESTER - primaryAction: check-duplicate + primaryAction: send-request closingAction: close-request actions: - - name: check-duplicate - desc: Check for a recent matching patron request + - name: send-request + desc: Send ISO18626 request to the supplier or broker transitions: - success: READY_TO_SEND - review: DUPLICATE + success: SENT + duplicate: DUPLICATE trigger: auto - name: close-request desc: Close the request locally @@ -100,33 +100,16 @@ stateModels: display: Needs review desc: Request is valid but needs staff review before sending side: REQUESTER - primaryAction: check-duplicate + primaryAction: send-request closingAction: close-request needsAttention: true editable: true - actions: - - name: check-duplicate - desc: Check for a recent matching patron request before sending - transitions: - success: READY_TO_SEND - review: DUPLICATE - - name: close-request - desc: Close the request locally - transitions: - success: MANUALLY_CLOSED - - - name: READY_TO_SEND - display: Ready to send - desc: Duplicate check has completed and the request is ready to send - side: REQUESTER - primaryAction: send-request - closingAction: close-request actions: - name: send-request desc: Send ISO18626 request to the supplier or broker transitions: success: SENT - trigger: auto + duplicate: DUPLICATE - name: close-request desc: Close the request locally transitions: @@ -134,7 +117,7 @@ stateModels: - name: DUPLICATE display: Duplicate - desc: A recent matching patron request was found + desc: A duplicate request was reported by the supplier or broker side: REQUESTER primaryAction: send-request needsAttention: true @@ -142,9 +125,10 @@ stateModels: closingAction: close-request actions: - name: send-request - desc: Send the request despite the duplicate warning + desc: Retry sending the request to the supplier or broker transitions: success: SENT + duplicate: DUPLICATE - name: close-request desc: Close the duplicate request transitions: From 2fb367245aa50919934806056a67cae2dadd5134 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Wed, 26 Aug 2026 09:20:22 +0300 Subject: [PATCH 3/9] ILLDEV-466 Use default template if not specified --- broker/email/email.go | 33 +++++++++++++++++------- broker/email/email_test.go | 22 +++++++++++++--- broker/oapi/open-api.yaml | 13 ++++++++-- broker/patron_request/service/action.go | 26 ++++++++++--------- broker/pullslip/service/pdf.go | 19 +++++++++++++- broker/scheduler/service/email_sender.go | 4 +-- misc/state-models.yaml | 4 +-- 7 files changed, 88 insertions(+), 33 deletions(-) diff --git a/broker/email/email.go b/broker/email/email.go index cc8ca0c4b..d6e50d47d 100644 --- a/broker/email/email.go +++ b/broker/email/email.go @@ -11,6 +11,7 @@ import ( "mime/quotedprintable" "net/smtp" "net/textproto" + "reflect" "strings" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" @@ -187,6 +188,7 @@ func joinAddresses(addrs []string) string { return result } +// Only string fields are allowed type PullSlipData struct { BorrowerName string ReqId string @@ -211,6 +213,13 @@ type PullSlipData struct { PatronId string } +// Only string fields are allowed +type BatchEmailData struct { + FullCount string + ActualCount string + BatchQuery string +} + func GetPullSlipData(pr pr_db.PatronRequest, notes []pr_db.Notification, conditions []pr_db.Notification, barcodeData string) PullSlipData { data := PullSlipData{ ReqId: pr.RequesterReqID.String, @@ -283,7 +292,7 @@ func GetPullSlipData(pr pr_db.PatronRequest, notes []pr_db.Notification, conditi return data } -func RenderPullSlipHTMLWithTemplate(data PullSlipData, templateBody string) (string, error) { +func RenderHtmlTemplate(data any, templateBody string) (string, error) { tmpl, err := template.New("pull-slip").Parse(templateBody) if err != nil { return "", err @@ -372,17 +381,21 @@ func formatPhysicalAddress(a *iso18626.PhysicalAddress) string { return strings.Join(parts, ", ") } -func GetBatchEmailData(fullCount int64, actualCount int, batchQuery string) map[string]string { - return map[string]string{ - "fullCount": fmt.Sprintf("%d", fullCount), - "actualCount": fmt.Sprintf("%d", actualCount), - "batchQuery": batchQuery, +func GetBatchEmailData(fullCount int64, actualCount int, batchQuery string) BatchEmailData { + return BatchEmailData{ + FullCount: fmt.Sprintf("%d", fullCount), + ActualCount: fmt.Sprintf("%d", actualCount), + BatchQuery: batchQuery, } } -func RenderBatchEmailTemplate(value string, placeholders map[string]string) string { - for key, replacement := range placeholders { - value = strings.ReplaceAll(value, "{{"+key+"}}", replacement) +func RenderTextTemplate(data any, template string) string { + v := reflect.ValueOf(data) + t := v.Type() + for i := 0; i < t.NumField(); i++ { + key := t.Field(i).Name + replacement := v.Field(i).String() + template = strings.ReplaceAll(template, "{{."+key+"}}", replacement) } - return value + return template } diff --git a/broker/email/email_test.go b/broker/email/email_test.go index 14d2fde52..b5e9ca233 100644 --- a/broker/email/email_test.go +++ b/broker/email/email_test.go @@ -99,7 +99,7 @@ func TestBuildRawMessage_WithoutAttachment(t *testing.T) { func TestRenderPullSlipHTML(t *testing.T) { template := "
\n Service Type: {{.ServiceType}}
\n Service Level: {{.ServiceLevel}}
\n System Identifier: {{.SystemIdentifier}}
\n Title: {{.Title}}
\n Author: {{.Author}}
\n Publisher: {{.Publisher}}
\n Volume(s): {{.Volume}}
\n Issue: {{.Issue}}
\n Pages: {{.Pages}}
\n
" - html, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ + html, err := RenderHtmlTemplate(PullSlipData{ ServiceType: "Loan", Title: "Big Shark", Author: "John Doe", @@ -115,19 +115,19 @@ func TestRenderPullSlipHTML(t *testing.T) { } func TestRenderPullSlipHTML_UsesProvidedTemplate(t *testing.T) { - html, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") + html, err := RenderHtmlTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") assert.NoError(t, err) assert.Equal(t, "
REQ-1
", html) } func TestRenderPullSlipHTML_InvalidTemplate(t *testing.T) { - _, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") + _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") assert.Error(t, err) } func TestRenderPullSlipHTML_ExecuteError(t *testing.T) { - _, err := RenderPullSlipHTMLWithTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") + _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") // Execute on a struct with map-access fails assert.Error(t, err) } @@ -408,3 +408,17 @@ func TestGetPickupLocation_AddressWithNoUsableFields(t *testing.T) { } assert.Equal(t, DEFAULT_FOR_NO_VALUE, getPickupLocation(pr)) } + +func TestRenderTextTemplate(t *testing.T) { + template := "This is query '{{.BatchQuery}}'." + data := GetBatchEmailData(1, 1, "select 1 from dual") + assert.Equal(t, "This is query 'select 1 from dual'.", RenderTextTemplate(data, template)) + + template = "

This is request {{.ReqId}}.

" + prData := GetPullSlipData(pr_db.PatronRequest{RequesterReqID: pgtype.Text{String: "REQ-1", Valid: true}}, nil, nil, "") + assert.Equal(t, "

This is request REQ-1.

", RenderTextTemplate(prData, template)) + + result, err := RenderHtmlTemplate(prData, template) + assert.NoError(t, err) + assert.Equal(t, "

This is request REQ-1.

", result) +} diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index a4ddcf655..ece5844ef 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -1389,10 +1389,19 @@ components: $ref: '#/components/schemas/TemplatePurpose' subject: type: string - description: Subject line template, supports {{x}} placeholders. Not used for pullslip templates. + description: Subject line template, supports {{.X}} placeholders. Not used for pullslip templates. Supports same placeholders as template body. body: type: string - description: Body of the email or pull slip template. Supports {{x}} placeholders. + description: >- + Body of the email or pull slip template. Supports {{.X}} placeholders. + Supported placeholders for patron request templates include + {{.BorrowerName}}, {{.ReqId}}, {{.PickupLocation}}, {{.Title}}, + {{.Author}}, {{.DueDate}}, {{.ReturnAddress}}, {{.BarcodeBase64}}, + {{.ServiceType}}, {{.ServiceLevel}}, {{.SystemIdentifier}}, + {{.Publisher}}, {{.Volume}}, {{.Issue}}, {{.Pages}}, {{.StaffNotes}}, + {{.CallNumber}}, {{.LoanConditions}}, {{.PatronName}}, + {{.PatronSurname}}, {{.PatronId}} and batch templates include + {{.FullCount}}, {{.ActualCount}}, {{.BatchQuery}}. contentType: $ref: '#/components/schemas/TemplateContentType' labels: diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index 6382004db..f731b2e93 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -1752,26 +1752,28 @@ func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedConte if err != nil { return err } - body := template.Body + var body string + notes, _, err := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindNote)}) + if err != nil { + return err + } + conditions, _, err := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindCondition)}) + if err != nil { + return err + } + data := email.GetPullSlipData(pr, notes, conditions, email.DEFAULT_FOR_NO_VALUE) if template.ContentType == string(proapi.Html) { - notes, _, ifErr := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindNote)}) - if ifErr != nil { - return ifErr - } - conditions, _, ifErr := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindCondition)}) - if ifErr != nil { - return ifErr - } - data := email.GetPullSlipData(pr, notes, conditions, email.DEFAULT_FOR_NO_VALUE) - html, ifErr := email.RenderPullSlipHTMLWithTemplate(data, template.Body) + html, ifErr := email.RenderHtmlTemplate(data, template.Body) if ifErr != nil { return ifErr } body = html + } else { + body = email.RenderTextTemplate(data, template.Body) } emailData := email.EmailData{ To: recipients, - Subject: template.Subject.String, + Subject: email.RenderTextTemplate(data, template.Subject.String), Body: body, IsHTML: template.ContentType == string(proapi.Html), IncludePdf: false, diff --git a/broker/pullslip/service/pdf.go b/broker/pullslip/service/pdf.go index 335c2f77e..b2d6bc712 100644 --- a/broker/pullslip/service/pdf.go +++ b/broker/pullslip/service/pdf.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "errors" "image/png" + "slices" "github.com/boombuler/barcode" "github.com/boombuler/barcode/code128" @@ -16,6 +17,7 @@ import ( pr_db "github.com/indexdata/crosslink/broker/patron_request/db" "github.com/indexdata/crosslink/broker/patron_request/proapi" prservice "github.com/indexdata/crosslink/broker/patron_request/service" + "github.com/jackc/pgx/v5" ) type PdfService interface { @@ -79,7 +81,7 @@ func (p *PdfServiceImpl) GeneratePdfPullSlip(ctx common.ExtendedContext, pr pr_d doc := document.NewDocument(document.PageSizeA4) data := email.GetPullSlipData(pr, notes, conditions, barcodeData) - html, err := email.RenderPullSlipHTMLWithTemplate(data, templateBody) + html, err := email.RenderHtmlTemplate(data, templateBody) if err != nil { return nil, err } @@ -111,11 +113,17 @@ func (p *PdfServiceImpl) getTemplateForPatronRequest(ctx common.ExtendedContext, Audience: string(proapi.ModelActionParamsSendToStaff), }) if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return getDefaultTemplate(*stateModel.PullslipPdfTemplateLabel) + } return "", err } if pdfTemplate.Body == "" { return "", errors.New("invalid pullslip pdf template, body field is required") } + if pdfTemplate.ContentType != string(proapi.Html) { + return "", errors.New("invalid pullslip pdf template, it must be of type HTML") + } return pdfTemplate.Body, nil } @@ -154,3 +162,12 @@ func getBarcodeBase64(data string) (string, error) { } return base64.StdEncoding.EncodeToString(buf.Bytes()), nil } + +func getDefaultTemplate(label string) (string, error) { + for _, t := range prservice.GetStateModelTemplateDefaults() { + if slices.Contains(t.Labels, label) { + return t.Body, nil + } + } + return "", errors.New("no default template found for label: " + label) +} diff --git a/broker/scheduler/service/email_sender.go b/broker/scheduler/service/email_sender.go index b28a23063..5847b3d09 100644 --- a/broker/scheduler/service/email_sender.go +++ b/broker/scheduler/service/email_sender.go @@ -136,8 +136,8 @@ func (s *EmailSenderService) generateAndEmailPullslip(ctx common.ExtendedContext placeholders := email.GetBatchEmailData(fullCount, len(prs), event.EventData.BatchActionData.Selector) messageData := email.EmailData{ To: emailData.To, - Subject: email.RenderBatchEmailTemplate(template.Subject.String, placeholders), - Body: email.RenderBatchEmailTemplate(template.Body, placeholders), + Subject: email.RenderTextTemplate(placeholders, template.Subject.String), + Body: email.RenderTextTemplate(placeholders, template.Body), IsHTML: template.ContentType == string(proapi.Html), IncludePdf: emailData.IncludePdf, } diff --git a/misc/state-models.yaml b/misc/state-models.yaml index 91c2a9a38..760aafc79 100644 --- a/misc/state-models.yaml +++ b/misc/state-models.yaml @@ -895,8 +895,8 @@ templateDefaults: body: |

This is an automated pull slip summary.

- Query: {{batchQuery}}
- Matching requests: {{actualCount}} (of {{fullCount}} total) + Query: {{.BatchQuery}}
+ Matching requests: {{.ActualCount}} (of {{.FullCount}} total)

Please process the attached pull slips at your earliest convenience.

From ab9c167781afca9522c44c2957fa539634102bf4 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Wed, 26 Aug 2026 10:02:16 +0300 Subject: [PATCH 4/9] ILLDEV-466 Fix tests --- broker/pullslip/api/api_handler_test.go | 4 +- broker/pullslip/service/pdf_test.go | 4 +- broker/scheduler/service/email_sender_test.go | 4 +- misc/state-models.yaml | 38 +++++++++++++------ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/broker/pullslip/api/api_handler_test.go b/broker/pullslip/api/api_handler_test.go index 4c809ecef..b7c34f43c 100644 --- a/broker/pullslip/api/api_handler_test.go +++ b/broker/pullslip/api/api_handler_test.go @@ -12,6 +12,7 @@ import ( "github.com/indexdata/cql-go/pgcql" "github.com/indexdata/crosslink/broker/common" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + "github.com/indexdata/crosslink/broker/patron_request/proapi" prservice "github.com/indexdata/crosslink/broker/patron_request/service" ps_db "github.com/indexdata/crosslink/broker/pullslip/db" psoapi "github.com/indexdata/crosslink/broker/pullslip/oapi" @@ -58,7 +59,8 @@ func (m *MockPrRepo) GetTemplateByPurposeAudienceLabelAndOwner(_ common.Extended for _, t := range prservice.GetStateModelTemplateDefaults() { if slices.Contains(t.Labels, params.Label) { return pr_db.Template{ - Body: t.Body, + Body: t.Body, + ContentType: string(proapi.Html), }, nil } } diff --git a/broker/pullslip/service/pdf_test.go b/broker/pullslip/service/pdf_test.go index 2eb1bd19b..d17125a42 100644 --- a/broker/pullslip/service/pdf_test.go +++ b/broker/pullslip/service/pdf_test.go @@ -12,6 +12,7 @@ import ( "github.com/indexdata/crosslink/broker/common" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" + "github.com/indexdata/crosslink/broker/patron_request/proapi" prservice "github.com/indexdata/crosslink/broker/patron_request/service" "github.com/indexdata/crosslink/iso18626" "github.com/indexdata/go-utils/utils" @@ -259,7 +260,8 @@ func (m *mockPrRepo) GetTemplateByPurposeAudienceLabelAndOwner(_ common.Extended for _, t := range prservice.GetStateModelTemplateDefaults() { if slices.Contains(t.Labels, params.Label) { return pr_db.Template{ - Body: t.Body, + Body: t.Body, + ContentType: string(proapi.Html), }, nil } } diff --git a/broker/scheduler/service/email_sender_test.go b/broker/scheduler/service/email_sender_test.go index 6bad91527..126a63400 100644 --- a/broker/scheduler/service/email_sender_test.go +++ b/broker/scheduler/service/email_sender_test.go @@ -378,8 +378,8 @@ func TestGenerateAndEmailPullslip_PerformsPlaceholderSubstitution(t *testing.T) fullCount: 5, template: pr_db.Template{ ID: "template-id", - Subject: pgtype.Text{String: "Selected {{fullCount}}", Valid: true}, - Body: "Attached {{actualCount}} of {{fullCount}} from {{batchQuery}}", + Subject: pgtype.Text{String: "Selected {{.FullCount}}", Valid: true}, + Body: "Attached {{.ActualCount}} of {{.FullCount}} from {{.BatchQuery}}", ContentType: "text", }, } diff --git a/misc/state-models.yaml b/misc/state-models.yaml index 760aafc79..f0ee3710f 100644 --- a/misc/state-models.yaml +++ b/misc/state-models.yaml @@ -80,16 +80,16 @@ stateModels: - name: METADATA_UPDATED display: Metadata Updated - desc: Request metadata update has completed or been skipped + desc: Request preparation has completed and pre-send checks can run side: REQUESTER - primaryAction: send-request + primaryAction: check-duplicate closingAction: close-request actions: - - name: send-request - desc: Send ISO18626 request to the supplier or broker + - name: check-duplicate + desc: Check for a recent matching patron request transitions: - success: SENT - duplicate: DUPLICATE + success: READY_TO_SEND + review: DUPLICATE trigger: auto - name: close-request desc: Close the request locally @@ -100,16 +100,33 @@ stateModels: display: Needs review desc: Request is valid but needs staff review before sending side: REQUESTER - primaryAction: send-request + primaryAction: check-duplicate closingAction: close-request needsAttention: true editable: true + actions: + - name: check-duplicate + desc: Check for a recent matching patron request before sending + transitions: + success: READY_TO_SEND + review: DUPLICATE + - name: close-request + desc: Close the request locally + transitions: + success: MANUALLY_CLOSED + + - name: READY_TO_SEND + display: Ready to send + desc: Duplicate check has completed and the request is ready to send + side: REQUESTER + primaryAction: send-request + closingAction: close-request actions: - name: send-request desc: Send ISO18626 request to the supplier or broker transitions: success: SENT - duplicate: DUPLICATE + trigger: auto - name: close-request desc: Close the request locally transitions: @@ -117,7 +134,7 @@ stateModels: - name: DUPLICATE display: Duplicate - desc: A duplicate request was reported by the supplier or broker + desc: A recent matching patron request was found side: REQUESTER primaryAction: send-request needsAttention: true @@ -125,10 +142,9 @@ stateModels: closingAction: close-request actions: - name: send-request - desc: Retry sending the request to the supplier or broker + desc: Send the request despite the duplicate warning transitions: success: SENT - duplicate: DUPLICATE - name: close-request desc: Close the duplicate request transitions: From 69008ef7b99d7944e02e781081ae3692bc28dcf0 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Wed, 26 Aug 2026 10:22:20 +0300 Subject: [PATCH 5/9] ILLDEV-466 Fix open api documentation --- broker/oapi/open-api.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index ece5844ef..277b31e49 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -1454,10 +1454,10 @@ components: $ref: '#/components/schemas/TemplatePurpose' subject: type: string - description: Subject line template, supports {{x}} placeholders. Not used for pullslip templates. + description: Subject line template, supports {{.X}} placeholders. Not used for pullslip templates. For full list of supported placeholders, see the Template object. body: type: string - description: Body of the email or pull slip template. Supports {{x}} placeholders. + description: Body of the email or pull slip template. Supports {{.X}} placeholders. For full list of supported placeholders, see the Template object. contentType: $ref: '#/components/schemas/TemplateContentType' labels: @@ -1484,10 +1484,10 @@ components: description: Human-readable title for the template subject: type: string - description: Subject line template supporting {{x}} placeholders. Not used for pull slip templates. Omit to clear. + description: Subject line template supporting {{.X}} placeholders. Not used for pull slip templates. Omit to clear. For full list of supported placeholders, see the Template object. body: type: string - description: Body of the email or pull slip template. Supports {{x}} placeholders. + description: Body of the email or pull slip template. Supports {{.X}} placeholders. For full list of supported placeholders, see the Template object. contentType: $ref: '#/components/schemas/TemplateContentType' labels: From 20616d0201de876c9229f032b013c7362b2e29a9 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Thu, 27 Aug 2026 11:18:17 +0300 Subject: [PATCH 6/9] ILLDEV-466 Use same render method --- broker/email/email.go | 14 +------- broker/email/email_test.go | 16 ++++----- broker/patron_request/service/action.go | 18 +++++----- broker/patron_request/service/action_test.go | 32 +++++++++++++++++ broker/pullslip/service/pdf.go | 2 +- broker/scheduler/service/email_sender.go | 12 +++++-- broker/scheduler/service/email_sender_test.go | 34 +++++++++++++++++++ 7 files changed, 94 insertions(+), 34 deletions(-) diff --git a/broker/email/email.go b/broker/email/email.go index d6e50d47d..7e6978bc5 100644 --- a/broker/email/email.go +++ b/broker/email/email.go @@ -11,7 +11,6 @@ import ( "mime/quotedprintable" "net/smtp" "net/textproto" - "reflect" "strings" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" @@ -292,7 +291,7 @@ func GetPullSlipData(pr pr_db.PatronRequest, notes []pr_db.Notification, conditi return data } -func RenderHtmlTemplate(data any, templateBody string) (string, error) { +func RenderTemplate(data any, templateBody string) (string, error) { tmpl, err := template.New("pull-slip").Parse(templateBody) if err != nil { return "", err @@ -388,14 +387,3 @@ func GetBatchEmailData(fullCount int64, actualCount int, batchQuery string) Batc BatchQuery: batchQuery, } } - -func RenderTextTemplate(data any, template string) string { - v := reflect.ValueOf(data) - t := v.Type() - for i := 0; i < t.NumField(); i++ { - key := t.Field(i).Name - replacement := v.Field(i).String() - template = strings.ReplaceAll(template, "{{."+key+"}}", replacement) - } - return template -} diff --git a/broker/email/email_test.go b/broker/email/email_test.go index b5e9ca233..f8b1caeac 100644 --- a/broker/email/email_test.go +++ b/broker/email/email_test.go @@ -99,7 +99,7 @@ func TestBuildRawMessage_WithoutAttachment(t *testing.T) { func TestRenderPullSlipHTML(t *testing.T) { template := "
\n Service Type: {{.ServiceType}}
\n Service Level: {{.ServiceLevel}}
\n System Identifier: {{.SystemIdentifier}}
\n Title: {{.Title}}
\n Author: {{.Author}}
\n Publisher: {{.Publisher}}
\n Volume(s): {{.Volume}}
\n Issue: {{.Issue}}
\n Pages: {{.Pages}}
\n
" - html, err := RenderHtmlTemplate(PullSlipData{ + html, err := RenderTemplate(PullSlipData{ ServiceType: "Loan", Title: "Big Shark", Author: "John Doe", @@ -115,19 +115,19 @@ func TestRenderPullSlipHTML(t *testing.T) { } func TestRenderPullSlipHTML_UsesProvidedTemplate(t *testing.T) { - html, err := RenderHtmlTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") + html, err := RenderTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") assert.NoError(t, err) assert.Equal(t, "
REQ-1
", html) } func TestRenderPullSlipHTML_InvalidTemplate(t *testing.T) { - _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") + _, err := RenderTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") assert.Error(t, err) } func TestRenderPullSlipHTML_ExecuteError(t *testing.T) { - _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") + _, err := RenderTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") // Execute on a struct with map-access fails assert.Error(t, err) } @@ -412,13 +412,13 @@ func TestGetPickupLocation_AddressWithNoUsableFields(t *testing.T) { func TestRenderTextTemplate(t *testing.T) { template := "This is query '{{.BatchQuery}}'." data := GetBatchEmailData(1, 1, "select 1 from dual") - assert.Equal(t, "This is query 'select 1 from dual'.", RenderTextTemplate(data, template)) + result, err := RenderTemplate(data, template) + assert.NoError(t, err) + assert.Equal(t, "This is query 'select 1 from dual'.", result) template = "

This is request {{.ReqId}}.

" prData := GetPullSlipData(pr_db.PatronRequest{RequesterReqID: pgtype.Text{String: "REQ-1", Valid: true}}, nil, nil, "") - assert.Equal(t, "

This is request REQ-1.

", RenderTextTemplate(prData, template)) - - result, err := RenderHtmlTemplate(prData, template) + result, err = RenderTemplate(prData, template) assert.NoError(t, err) assert.Equal(t, "

This is request REQ-1.

", result) } diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index f731b2e93..c6119e884 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -1752,7 +1752,6 @@ func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedConte if err != nil { return err } - var body string notes, _, err := a.prRepo.GetNotificationsByPrId(ctx, pr_db.GetNotificationsByPrIdParams{Limit: 100, Offset: 0, PrID: pr.ID, Kind: string(pr_db.NotificationKindNote)}) if err != nil { return err @@ -1762,18 +1761,17 @@ func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedConte return err } data := email.GetPullSlipData(pr, notes, conditions, email.DEFAULT_FOR_NO_VALUE) - if template.ContentType == string(proapi.Html) { - html, ifErr := email.RenderHtmlTemplate(data, template.Body) - if ifErr != nil { - return ifErr - } - body = html - } else { - body = email.RenderTextTemplate(data, template.Body) + body, err := email.RenderTemplate(data, template.Body) + if err != nil { + return err + } + subject, err := email.RenderTemplate(data, template.Subject.String) + if err != nil { + return err } emailData := email.EmailData{ To: recipients, - Subject: email.RenderTextTemplate(data, template.Subject.String), + Subject: subject, Body: body, IsHTML: template.ContentType == string(proapi.Html), IncludePdf: false, diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 992372d4c..d69520783 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -3851,6 +3851,38 @@ func TestCreateAndSendEmail(t *testing.T) { }, wantErrSubstr: "header injection", }, + { + name: "invalid template body", + from: "from@example.com", + recipients: recipients, + setupPrRepo: func(m *MockPrRepo) { + m.On("GetTemplateByPurposeAudienceLabelAndOwner", mock.Anything).Return(pr_db.Template{ + Body: "Hello patron {{.PatronEmai ", + Subject: pgtype.Text{String: "Your request", Valid: true}, + }, nil) + }, + setupEmail: func(m *EmailSenderMock) {}, + assertEmail: func(t *testing.T, m *EmailSenderMock) { + m.AssertNotCalled(t, "SendEmail", mock.Anything) + }, + wantErrSubstr: "template: pull-slip:1: unclosed action", + }, + { + name: "invalid template body", + from: "from@example.com", + recipients: recipients, + setupPrRepo: func(m *MockPrRepo) { + m.On("GetTemplateByPurposeAudienceLabelAndOwner", mock.Anything).Return(pr_db.Template{ + Body: "Hello patron", + Subject: pgtype.Text{String: "Your request {{.PatronEmai ", Valid: true}, + }, nil) + }, + setupEmail: func(m *EmailSenderMock) {}, + assertEmail: func(t *testing.T, m *EmailSenderMock) { + m.AssertNotCalled(t, "SendEmail", mock.Anything) + }, + wantErrSubstr: "template: pull-slip:1: unclosed action", + }, } for _, tc := range tests { diff --git a/broker/pullslip/service/pdf.go b/broker/pullslip/service/pdf.go index b2d6bc712..b6c721d2a 100644 --- a/broker/pullslip/service/pdf.go +++ b/broker/pullslip/service/pdf.go @@ -81,7 +81,7 @@ func (p *PdfServiceImpl) GeneratePdfPullSlip(ctx common.ExtendedContext, pr pr_d doc := document.NewDocument(document.PageSizeA4) data := email.GetPullSlipData(pr, notes, conditions, barcodeData) - html, err := email.RenderHtmlTemplate(data, templateBody) + html, err := email.RenderTemplate(data, templateBody) if err != nil { return nil, err } diff --git a/broker/scheduler/service/email_sender.go b/broker/scheduler/service/email_sender.go index 5847b3d09..3559198cb 100644 --- a/broker/scheduler/service/email_sender.go +++ b/broker/scheduler/service/email_sender.go @@ -134,10 +134,18 @@ func (s *EmailSenderService) generateAndEmailPullslip(ctx common.ExtendedContext } placeholders := email.GetBatchEmailData(fullCount, len(prs), event.EventData.BatchActionData.Selector) + body, err := email.RenderTemplate(placeholders, template.Body) + if err != nil { + return events.NewErrorResult("failed to render email body", err.Error()) + } + subject, err := email.RenderTemplate(placeholders, template.Subject.String) + if err != nil { + return events.NewErrorResult("failed to render email subject", err.Error()) + } messageData := email.EmailData{ To: emailData.To, - Subject: email.RenderTextTemplate(placeholders, template.Subject.String), - Body: email.RenderTextTemplate(placeholders, template.Body), + Subject: subject, + Body: body, IsHTML: template.ContentType == string(proapi.Html), IncludePdf: emailData.IncludePdf, } diff --git a/broker/scheduler/service/email_sender_test.go b/broker/scheduler/service/email_sender_test.go index 126a63400..c35f50cfd 100644 --- a/broker/scheduler/service/email_sender_test.go +++ b/broker/scheduler/service/email_sender_test.go @@ -323,6 +323,40 @@ func TestGenerateAndEmailPullslip_TemplateEmptySubject(t *testing.T) { assert.NotNil(t, result) } +func TestGenerateAndEmailPullslip_TemplateInvalidBody(t *testing.T) { + prRepo := &mockEmailPrRepo{template: pr_db.Template{ + ID: "template-id", + Subject: pgtype.Text{ + Valid: true, + String: "Subject", + }, + Body: "Body {{.Invalid text", + ContentType: "text", + }} + svc := newEmailSvc(prRepo, &mockEmailService{}, nil) + status, result := svc.generateAndEmailPullslip(testCtx, validEmailEvent()) + assert.Equal(t, events.EventStatusError, status) + assert.NotNil(t, result) + assert.Equal(t, "failed to render email body", result.EventError.Message) +} + +func TestGenerateAndEmailPullslip_TemplateInvalidSubject(t *testing.T) { + prRepo := &mockEmailPrRepo{template: pr_db.Template{ + ID: "template-id", + Subject: pgtype.Text{ + Valid: true, + String: "Subject {{.Invalid text", + }, + Body: "Body", + ContentType: "text", + }} + svc := newEmailSvc(prRepo, &mockEmailService{}, nil) + status, result := svc.generateAndEmailPullslip(testCtx, validEmailEvent()) + assert.Equal(t, events.EventStatusError, status) + assert.NotNil(t, result) + assert.Equal(t, "failed to render email subject", result.EventError.Message) +} + func TestGenerateAndEmailPullslip_TemplateEmptyBody(t *testing.T) { prRepo := &mockEmailPrRepo{template: pr_db.Template{ ID: "template-id", From e793f41049a6c5d60858dd4bb4b7503a468f5853 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Thu, 27 Aug 2026 11:41:56 +0300 Subject: [PATCH 7/9] ILLDEV-466 Update existing templates with new placeholders --- .../059_normalize_templates.down.sql | 26 +++++++++++++++++++ .../migrations/059_normalize_templates.up.sql | 26 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 broker/migrations/059_normalize_templates.down.sql create mode 100644 broker/migrations/059_normalize_templates.up.sql diff --git a/broker/migrations/059_normalize_templates.down.sql b/broker/migrations/059_normalize_templates.down.sql new file mode 100644 index 000000000..5288ff78a --- /dev/null +++ b/broker/migrations/059_normalize_templates.down.sql @@ -0,0 +1,26 @@ +-- Revert placeholders in template.subject and template.body +-- Old: {{.BatchQuery}}, {{.ActualCount}}, {{.FullCount}} +-- New: {{batchQuery}}, {{actualCount}}, {{fullCount}} +UPDATE template +SET + subject = REPLACE( + REPLACE( + REPLACE(subject, '{{.BatchQuery}}', '{{batchQuery}}'), + '{{.ActualCount}}', '{{actualCount}}' + ), + '{{.FullCount}}', '{{fullCount}}' + ), + body = REPLACE( + REPLACE( + REPLACE(body, '{{.BatchQuery}}', '{{batchQuery}}'), + '{{.ActualCount}}', '{{actualCount}}' + ), + '{{.FullCount}}', '{{fullCount}}' + ) +WHERE + subject LIKE '%{{.BatchQuery}}%' + OR subject LIKE '%{{.ActualCount}}%' + OR subject LIKE '%{{.FullCount}}%' + OR body LIKE '%{{.BatchQuery}}%' + OR body LIKE '%{{.ActualCount}}%' + OR body LIKE '%{{.FullCount}}%'; \ No newline at end of file diff --git a/broker/migrations/059_normalize_templates.up.sql b/broker/migrations/059_normalize_templates.up.sql new file mode 100644 index 000000000..71fd07287 --- /dev/null +++ b/broker/migrations/059_normalize_templates.up.sql @@ -0,0 +1,26 @@ +-- Replace placeholders in template.subject and template.body +-- Old: {{batchQuery}}, {{actualCount}}, {{fullCount}} +-- New: {{.BatchQuery}}, {{.ActualCount}}, {{.FullCount}} +UPDATE template +SET + subject = REPLACE( + REPLACE( + REPLACE(subject, '{{batchQuery}}', '{{.BatchQuery}}'), + '{{actualCount}}', '{{.ActualCount}}' + ), + '{{fullCount}}', '{{.FullCount}}' + ), + body = REPLACE( + REPLACE( + REPLACE(body, '{{batchQuery}}', '{{.BatchQuery}}'), + '{{actualCount}}', '{{.ActualCount}}' + ), + '{{fullCount}}', '{{.FullCount}}' + ) +WHERE + subject LIKE '%{{batchQuery}}%' + OR subject LIKE '%{{actualCount}}%' + OR subject LIKE '%{{fullCount}}%' + OR body LIKE '%{{batchQuery}}%' + OR body LIKE '%{{actualCount}}%' + OR body LIKE '%{{fullCount}}%'; \ No newline at end of file From 8da41d27ab2fcfdeeedef61ac87e006939e6a13c Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Fri, 28 Aug 2026 13:26:05 +0300 Subject: [PATCH 8/9] ILLDEV-493 Don't send email if no patron requests are found --- broker/email/email.go | 16 +++++++++++++-- broker/email/email_test.go | 26 +++++++++++------------- broker/oapi/open-api.yaml | 2 +- broker/patron_request/service/action.go | 9 ++++++-- broker/pullslip/service/pdf.go | 2 +- broker/scheduler/service/email_sender.go | 9 ++++++-- 6 files changed, 42 insertions(+), 22 deletions(-) diff --git a/broker/email/email.go b/broker/email/email.go index 7e6978bc5..f29457d3c 100644 --- a/broker/email/email.go +++ b/broker/email/email.go @@ -12,6 +12,7 @@ import ( "net/smtp" "net/textproto" "strings" + text_template "text/template" pr_db "github.com/indexdata/crosslink/broker/patron_request/db" "github.com/indexdata/crosslink/iso18626" @@ -189,7 +190,6 @@ func joinAddresses(addrs []string) string { // Only string fields are allowed type PullSlipData struct { - BorrowerName string ReqId string PickupLocation string Title string @@ -291,7 +291,7 @@ func GetPullSlipData(pr pr_db.PatronRequest, notes []pr_db.Notification, conditi return data } -func RenderTemplate(data any, templateBody string) (string, error) { +func RenderHtmlTemplate(data any, templateBody string) (string, error) { tmpl, err := template.New("pull-slip").Parse(templateBody) if err != nil { return "", err @@ -303,6 +303,18 @@ func RenderTemplate(data any, templateBody string) (string, error) { return buf.String(), nil } +func RenderTextTemplate(data any, templateBody string) (string, error) { + tmpl, err := text_template.New("pull-slip").Parse(templateBody) + if err != nil { + return "", err + } + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return "", err + } + return buf.String(), nil +} + func getStaffNotes(noteList []pr_db.Notification) string { noteStrings := []string{} for _, note := range noteList { diff --git a/broker/email/email_test.go b/broker/email/email_test.go index f8b1caeac..c285424ff 100644 --- a/broker/email/email_test.go +++ b/broker/email/email_test.go @@ -99,7 +99,7 @@ func TestBuildRawMessage_WithoutAttachment(t *testing.T) { func TestRenderPullSlipHTML(t *testing.T) { template := "
\n Service Type: {{.ServiceType}}
\n Service Level: {{.ServiceLevel}}
\n System Identifier: {{.SystemIdentifier}}
\n Title: {{.Title}}
\n Author: {{.Author}}
\n Publisher: {{.Publisher}}
\n Volume(s): {{.Volume}}
\n Issue: {{.Issue}}
\n Pages: {{.Pages}}
\n
" - html, err := RenderTemplate(PullSlipData{ + html, err := RenderHtmlTemplate(PullSlipData{ ServiceType: "Loan", Title: "Big Shark", Author: "John Doe", @@ -115,19 +115,19 @@ func TestRenderPullSlipHTML(t *testing.T) { } func TestRenderPullSlipHTML_UsesProvidedTemplate(t *testing.T) { - html, err := RenderTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") + html, err := RenderHtmlTemplate(PullSlipData{ReqId: "REQ-1"}, "
{{.ReqId}}
") assert.NoError(t, err) assert.Equal(t, "
REQ-1
", html) } func TestRenderPullSlipHTML_InvalidTemplate(t *testing.T) { - _, err := RenderTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") + _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{.Unclosed") assert.Error(t, err) } func TestRenderPullSlipHTML_ExecuteError(t *testing.T) { - _, err := RenderTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") + _, err := RenderHtmlTemplate(PullSlipData{ReqId: "X"}, "{{index . \"nonexistent\"}}") // Execute on a struct with map-access fails assert.Error(t, err) } @@ -195,7 +195,6 @@ func TestGetPullSlipData_PopulatesAllAvailableFields(t *testing.T) { data := GetPullSlipData(pr, notes, conditions, "barcode-base64") assert.Equal(t, PullSlipData{ - BorrowerName: "", ReqId: "REQ-123", PickupLocation: "Pickup Desk, Riga, LV-1050, LV", Title: "Distributed Libraries", @@ -223,7 +222,6 @@ func TestGetPullSlipData_UsesDefaultsWhenOptionalFieldsAreMissing(t *testing.T) data := GetPullSlipData(pr_db.PatronRequest{}, nil, nil, DEFAULT_FOR_NO_VALUE) assert.Equal(t, PullSlipData{ - BorrowerName: "", ReqId: "", PickupLocation: DEFAULT_FOR_NO_VALUE, Title: DEFAULT_FOR_NO_VALUE, @@ -410,15 +408,15 @@ func TestGetPickupLocation_AddressWithNoUsableFields(t *testing.T) { } func TestRenderTextTemplate(t *testing.T) { - template := "This is query '{{.BatchQuery}}'." - data := GetBatchEmailData(1, 1, "select 1 from dual") - result, err := RenderTemplate(data, template) + template := "This is A&B query '{{.BatchQuery}}'." + data := GetBatchEmailData(1, 1, "select \"A&B\" from dual") + result, err := RenderTextTemplate(data, template) assert.NoError(t, err) - assert.Equal(t, "This is query 'select 1 from dual'.", result) + assert.Equal(t, "This is A&B query 'select \"A&B\" from dual'.", result) - template = "

This is request {{.ReqId}}.

" - prData := GetPullSlipData(pr_db.PatronRequest{RequesterReqID: pgtype.Text{String: "REQ-1", Valid: true}}, nil, nil, "") - result, err = RenderTemplate(prData, template) + template = "

This is A&B request {{.ReqId}}.

" + prData := GetPullSlipData(pr_db.PatronRequest{RequesterReqID: pgtype.Text{String: "REQ-1-A&B", Valid: true}}, nil, nil, "") + result, err = RenderHtmlTemplate(prData, template) assert.NoError(t, err) - assert.Equal(t, "

This is request REQ-1.

", result) + assert.Equal(t, "

This is A&B request REQ-1-A&B.

", result) } diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index 277b31e49..aaf5c9608 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -1395,7 +1395,7 @@ components: description: >- Body of the email or pull slip template. Supports {{.X}} placeholders. Supported placeholders for patron request templates include - {{.BorrowerName}}, {{.ReqId}}, {{.PickupLocation}}, {{.Title}}, + {{.ReqId}}, {{.PickupLocation}}, {{.Title}}, {{.Author}}, {{.DueDate}}, {{.ReturnAddress}}, {{.BarcodeBase64}}, {{.ServiceType}}, {{.ServiceLevel}}, {{.SystemIdentifier}}, {{.Publisher}}, {{.Volume}}, {{.Issue}}, {{.Pages}}, {{.StaffNotes}}, diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index c6119e884..4fb5c190d 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -1761,11 +1761,16 @@ func (a *PatronRequestActionService) createAndSendEmail(ctx common.ExtendedConte return err } data := email.GetPullSlipData(pr, notes, conditions, email.DEFAULT_FOR_NO_VALUE) - body, err := email.RenderTemplate(data, template.Body) + var body string + if template.ContentType == string(proapi.Html) { + body, err = email.RenderHtmlTemplate(data, template.Body) + } else { + body, err = email.RenderTextTemplate(data, template.Body) + } if err != nil { return err } - subject, err := email.RenderTemplate(data, template.Subject.String) + subject, err := email.RenderTextTemplate(data, template.Subject.String) if err != nil { return err } diff --git a/broker/pullslip/service/pdf.go b/broker/pullslip/service/pdf.go index b6c721d2a..b2d6bc712 100644 --- a/broker/pullslip/service/pdf.go +++ b/broker/pullslip/service/pdf.go @@ -81,7 +81,7 @@ func (p *PdfServiceImpl) GeneratePdfPullSlip(ctx common.ExtendedContext, pr pr_d doc := document.NewDocument(document.PageSizeA4) data := email.GetPullSlipData(pr, notes, conditions, barcodeData) - html, err := email.RenderTemplate(data, templateBody) + html, err := email.RenderHtmlTemplate(data, templateBody) if err != nil { return nil, err } diff --git a/broker/scheduler/service/email_sender.go b/broker/scheduler/service/email_sender.go index 3559198cb..02ed5cf5a 100644 --- a/broker/scheduler/service/email_sender.go +++ b/broker/scheduler/service/email_sender.go @@ -134,11 +134,16 @@ func (s *EmailSenderService) generateAndEmailPullslip(ctx common.ExtendedContext } placeholders := email.GetBatchEmailData(fullCount, len(prs), event.EventData.BatchActionData.Selector) - body, err := email.RenderTemplate(placeholders, template.Body) + var body string + if template.ContentType == string(proapi.Html) { + body, err = email.RenderHtmlTemplate(placeholders, template.Body) + } else { + body, err = email.RenderTextTemplate(placeholders, template.Body) + } if err != nil { return events.NewErrorResult("failed to render email body", err.Error()) } - subject, err := email.RenderTemplate(placeholders, template.Subject.String) + subject, err := email.RenderTextTemplate(placeholders, template.Subject.String) if err != nil { return events.NewErrorResult("failed to render email subject", err.Error()) } From 1cff038ae3967aeccc863caafeef32a4c8f00a80 Mon Sep 17 00:00:00 2001 From: Janis Saldabols Date: Mon, 31 Aug 2026 08:42:23 +0300 Subject: [PATCH 9/9] ILLDEV-466 Correct migration script order --- ...malize_templates.down.sql => 060_normalize_templates.down.sql} | 0 ..._normalize_templates.up.sql => 060_normalize_templates.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename broker/migrations/{059_normalize_templates.down.sql => 060_normalize_templates.down.sql} (100%) rename broker/migrations/{059_normalize_templates.up.sql => 060_normalize_templates.up.sql} (100%) diff --git a/broker/migrations/059_normalize_templates.down.sql b/broker/migrations/060_normalize_templates.down.sql similarity index 100% rename from broker/migrations/059_normalize_templates.down.sql rename to broker/migrations/060_normalize_templates.down.sql diff --git a/broker/migrations/059_normalize_templates.up.sql b/broker/migrations/060_normalize_templates.up.sql similarity index 100% rename from broker/migrations/059_normalize_templates.up.sql rename to broker/migrations/060_normalize_templates.up.sql