Skip to content

Make send-notification best effort #701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jakub-id wants to merge 1 commit into
base: main
Choose a base branch
from
+25 −21
Open

Make send-notification best effort #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions broker/patron_request/service/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ func (a *PatronRequestActionService) acceptRetryBorrowingRequest(ctx common.Exte

func (a *PatronRequestActionService) sendNotificationBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams) actionExecutionResult {
if !a.emailService.IsReadyToSend() {
return actionExecutionResult{status: events.EventStatusSuccess, result: &events.EventResult{CommonEventData: events.CommonEventData{Note: "email service is not ready to send"}}, pr: pr}
return logNotificationErrorAndReturnSuccess(ctx, pr, "email service is not ready to send", nil)
}
return a.sendEmailNotification(ctx, pr, params, pr.RequesterSymbol.String)
Comment on lines 706 to 710
}
Expand Down Expand Up @@ -1083,25 +1083,29 @@ func (a *PatronRequestActionService) askRetryLenderRequest(ctx common.ExtendedCo

func (a *PatronRequestActionService) sendNotificationLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams) actionExecutionResult {
if !a.emailService.IsReadyToSend() {
return actionExecutionResult{status: events.EventStatusSuccess, result: &events.EventResult{CommonEventData: events.CommonEventData{Note: "email service is not ready to send"}}, pr: pr}
return logNotificationErrorAndReturnSuccess(ctx, pr, "email service is not ready to send", nil)
}
return a.sendEmailNotification(ctx, pr, params, pr.SupplierSymbol.String)
}

func logErrorAndReturnActionExecutionResult(ctx common.ExtendedContext, pr pr_db.PatronRequest, msg string, err error) actionExecutionResult {
status, result := logActionErrorAndReturnResult(ctx, msg, err)
return actionExecutionResult{status: status, result: result, pr: pr}
func logNotificationErrorAndReturnSuccess(ctx common.ExtendedContext, pr pr_db.PatronRequest, msg string, err error) actionExecutionResult {
ctx.Logger().Error(msg, "error", err)
return actionExecutionResult{
status: events.EventStatusSuccess,
result: &events.EventResult{CommonEventData: events.CommonEventData{Note: msg}},
pr: pr,
}
}
Comment on lines +1091 to 1098

func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams, symbol string) actionExecutionResult {
result := events.EventResult{}
if params.AutoActionParams != nil && params.AutoActionParams.SendTo != nil && len(*params.AutoActionParams.SendTo) > 0 {
if params.AutoActionParams.TemplateLabel == nil {
return logErrorAndReturnActionExecutionResult(ctx, pr, "template label is not set", nil)
return logNotificationErrorAndReturnSuccess(ctx, pr, "template label is not set", nil)
}
from, to, err := a.getDirectoryEmailData(ctx, symbol, slices.Contains(*params.AutoActionParams.SendTo, proapi.ModelActionParamsSendToStaff))
if err != nil {
return logErrorAndReturnActionExecutionResult(ctx, pr, "error getting directory email data", err)
return logNotificationErrorAndReturnSuccess(ctx, pr, "error getting directory email data", err)
}
if slices.Contains(*params.AutoActionParams.SendTo, proapi.ModelActionParamsSendToPatron) {
recipients := patronEmail(pr)
Expand All @@ -1110,7 +1114,7 @@ func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedCo
} else {
sendErr := a.createAndSendEmail(ctx, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToPatron)
if sendErr != nil {
return logErrorAndReturnActionExecutionResult(ctx, pr, "error sending email to patron", sendErr)
return logNotificationErrorAndReturnSuccess(ctx, pr, "error sending email to patron", sendErr)
}
result.Note = "patron email sent successfully"
}
Expand All @@ -1130,7 +1134,7 @@ func (a *PatronRequestActionService) sendEmailNotification(ctx common.ExtendedCo
} else {
sendErr := a.createAndSendEmail(ctx, symbol, from, recipients, *params.AutoActionParams.TemplateLabel, proapi.ModelActionParamsSendToStaff)
if sendErr != nil {
return logErrorAndReturnActionExecutionResult(ctx, pr, "error sending email to staff", sendErr)
return logNotificationErrorAndReturnSuccess(ctx, pr, "error sending email to staff", sendErr)
}
result.Note = "staff email sent successfully"
}
Expand Down
24 changes: 12 additions & 12 deletions broker/patron_request/service/action_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2330,26 +2330,26 @@ func TestSendEmailNotification(t *testing.T) {
wantStatus: events.EventStatusSuccess,
},
{
name: "nil TemplateLabel – error",
name: "nil TemplateLabel – logged success",
pr: pr_db.PatronRequest{},
symbol: testSymbol,
params: actionParams{AutoActionParams: &proapi.ModelAction_Params{
SendTo: sendToTargets(proapi.ModelActionParamsSendToPatron),
}},
setupMocks: func(_ *MockPrRepo, _ *IllRepoMock, _ *EmailSenderMock) {},
wantStatus: events.EventStatusError,
wantErr: "template label is not set",
wantStatus: events.EventStatusSuccess,
wantNote: "template label is not set",
},
{
name: "GetPeerBySymbol error – error result",
name: "GetPeerBySymbol error – logged success",
pr: pr_db.PatronRequest{},
symbol: testSymbol,
params: autoParams(testTemplate, proapi.ModelActionParamsSendToPatron),
setupMocks: func(_ *MockPrRepo, illRepo *IllRepoMock, _ *EmailSenderMock) {
illRepo.On("GetPeerBySymbol", testSymbol).Return(ill_db.Peer{}, errors.New("db error"))
},
wantStatus: events.EventStatusError,
wantErr: "error getting directory email data",
wantStatus: events.EventStatusSuccess,
wantNote: "error getting directory email data",
},
{
name: "SendTo patron – no patron email addresses – note set",
Expand All @@ -2376,7 +2376,7 @@ func TestSendEmailNotification(t *testing.T) {
wantNote: "patron email sent successfully",
},
{
name: "SendTo patron – SendEmail fails – error result",
name: "SendTo patron – SendEmail fails – logged success",
pr: prWithPatronEmail(testPatronTo),
symbol: testSymbol,
params: autoParams(testTemplate, proapi.ModelActionParamsSendToPatron),
Expand All @@ -2385,8 +2385,8 @@ func TestSendEmailNotification(t *testing.T) {
prRepo.On("GetTemplateByPurposeAudienceLabelAndOwner", mock.Anything).Return(foundTemplate, nil)
emailSvc.On("SendEmail", testFrom).Return(errors.New("smtp error"))
},
wantStatus: events.EventStatusError,
wantErr: "error sending email to patron",
wantStatus: events.EventStatusSuccess,
wantNote: "error sending email to patron",
},
{
name: "SendTo staff – email sent successfully",
Expand Down Expand Up @@ -2441,7 +2441,7 @@ func TestSendEmailNotification(t *testing.T) {
wantNote: "staff email sent successfully",
},
{
name: "SendTo staff – SendEmail fails – error result",
name: "SendTo staff – SendEmail fails – logged success",
pr: pr_db.PatronRequest{},
symbol: testSymbol,
params: autoParams(testTemplate, proapi.ModelActionParamsSendToStaff),
Expand All @@ -2450,8 +2450,8 @@ func TestSendEmailNotification(t *testing.T) {
prRepo.On("GetTemplateByPurposeAudienceLabelAndOwner", mock.Anything).Return(foundTemplate, nil)
emailSvc.On("SendEmail", testFrom).Return(errors.New("smtp error"))
},
wantStatus: events.EventStatusError,
wantErr: "error sending email to staff",
wantStatus: events.EventStatusSuccess,
wantNote: "error sending email to staff",
},
{
name: "SendTo patron and staff – both emails sent – staff note wins",
Expand Down