Skip to content

ILLDEV-493 Don't send email if no patron requests are found - #740

Open
JanisSaldabols wants to merge 2 commits into
mainfrom
ILLDEV-493
Open

ILLDEV-493 Don't send email if no patron requests are found#740
JanisSaldabols wants to merge 2 commits into
mainfrom
ILLDEV-493

Conversation

@JanisSaldabols

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI balanced review requested due to automatic review settings August 28, 2026 10:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Prevents pull-slip emails when no patron requests match.

Changes:

  • Returns a successful no-op result for empty batches.
  • Updates tests to cover empty and non-empty results.

Reviewed changes

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

File Description
broker/scheduler/service/email_sender.go Skips email generation for empty batches.
broker/scheduler/service/email_sender_test.go Adjusts and adds email-sending tests.

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

Comment thread broker/scheduler/service/email_sender.go
Comment thread broker/scheduler/service/email_sender.go Outdated
"matched", fullCount, "limit", MAX_RECORDS_PER_EMAIL)
}
if len(prs) == 0 {
ctx.Logger().Warn("email batch empty: selector matched no records", "selector", event.EventData.BatchActionData.Selector)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's not log anything that has to do with regular operations to stdout, only devops look at these logs. It's a problem elsewhere but fix this here for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants