From c06a27aea7a518d399776a6c408c5be2f20f43a2 Mon Sep 17 00:00:00 2001 From: Paul Graham Date: Tue, 25 Feb 2020 09:36:33 +0000 Subject: [PATCH 1/4] Added retries in NSB command handlers --- ...proveTransferRequestCommandHandlerTests.cs | 26 +++++++++++++++++++ ...roviderApproveCohortCommandHandlerTests.cs | 21 ++++++++++++++- .../ProviderSendCohortCommandHandlerTests.cs | 19 ++++++++++++++ .../ApproveTransferRequestCommandHandler.cs | 7 +++++ .../ProviderApproveCohortCommandHandler.cs | 6 +++++ .../ProviderSendCohortCommandHandler.cs | 6 +++++ .../SFA.DAS.CommitmentsV2/Models/Cohort.cs | 4 +-- 7 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs index 3b72ce20ee..6a25fea98e 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs @@ -44,6 +44,18 @@ public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ThenItShouldPu fixture.VerifyTransferRequestApprovedEventIsPublished(); } + [Test] + public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ForASecondTime_ThenItShouldLogWarningAndReturn() + { + var fixture = new ApproveTransferRequestCommandHandlerTestsFixture(); + fixture.SetupTransfer().SetupTransferSenderApproveCohortCommand().SetTransferStatusToApproved(); + + fixture.Handle(); + + fixture.VerifyTransferRequestApprovedEventIsNotPublished(); + } + + [Test] public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ThenItShouldPublishChangeTrackingEvents() { @@ -136,6 +148,12 @@ public ApproveTransferRequestCommandHandlerTestsFixture SetupTransfer() return this; } + public ApproveTransferRequestCommandHandlerTestsFixture SetTransferStatusToApproved() + { + TransferRequest.Status = TransferApprovalStatus.Approved; + return this; + } + public Task Handle() { return Sut.Handle(TransferSenderApproveCohortCommand, Mock.Of()); @@ -165,6 +183,14 @@ public void VerifyTransferRequestApprovedEventIsPublished() Assert.AreEqual(Now, list[0].ApprovedOn); } + public void VerifyTransferRequestApprovedEventIsNotPublished() + { + var list = UnitOfWorkContext.GetEvents().OfType().ToList(); + + Assert.AreEqual(0, list.Count); + } + + public void VerifyEntityIsBeingTracked() { var list = UnitOfWorkContext.GetEvents().OfType().Where(x=>x.StateChangeType == UserAction.ApproveTransferRequest).ToList(); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs index 810121f472..dc6c195670 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs @@ -26,12 +26,19 @@ public void Arrange() } [Test] - public async Task When_HandlingCommand_Approve_Cohort() + public async Task When_HandlingCommand_ApproveCohort() { await _fixture.Handle(); _fixture.VerifyCohortApproval(); } + [Test] + public async Task When_HandlingCommand_ApproveCohort_Again_ShouldNotCallCohortApproval() + { + await _fixture.AddAlreadyApprovedByProvider().Handle(); + _fixture.VerifyCohortApprovalWasNotCalled(); + } + public class ProviderApproveCohortCommandHandlerTestsFixture { private ProviderApproveCohortCommandHandler _handler; @@ -55,7 +62,9 @@ public ProviderApproveCohortCommandHandlerTestsFixture() _cohort = new Mock(); _cohort.Setup(x => x.Id).Returns(_command.CohortId); + _cohort.Setup(x => x.Approvals).Returns(Party.None); _cohort.Setup(x => x.IsApprovedByAllParties).Returns(false); + _cohort.Setup(x => x.Approve(Party.Provider, It.IsAny(), It.IsAny(), It.IsAny())); @@ -63,6 +72,12 @@ public ProviderApproveCohortCommandHandlerTestsFixture() _dbContext.Object.SaveChanges(); } + public ProviderApproveCohortCommandHandlerTestsFixture AddAlreadyApprovedByProvider() + { + _cohort.Setup(x => x.Approvals).Returns(Party.Provider); + return this; + } + public async Task Handle() { await _handler.Handle(_command, _messageHandlerContext.Object); @@ -72,6 +87,10 @@ public void VerifyCohortApproval() { _cohort.Verify(x=> x.Approve(Party.Provider, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + public void VerifyCohortApprovalWasNotCalled() + { + _cohort.Verify(x => x.Approve(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs index b27912982d..61c2999506 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs @@ -32,6 +32,13 @@ public async Task When_HandlingCommand_CohortIsSentToOtherParty() _fixture.VerifyCohortIsSentToOtherParty(); } + [Test] + public async Task When_HandlingCommand_CohortIsAlreadySentToOtherParty_Then_ShouldNotCallSendToOtherParty() + { + await _fixture.SetWithPartyToEmployer().Handle(); + _fixture.VerifySendToOtherPartyIsNotCalled(); + } + public class ProviderSendCohortCommandHandlerTestsFixture { private ProviderSendCohortCommandHandler _handler; @@ -55,6 +62,7 @@ public ProviderSendCohortCommandHandlerTestsFixture() _cohort = new Mock(); _cohort.Setup(x => x.Id).Returns(_command.CohortId); + _cohort.Setup(x => x.WithParty).Returns(Party.Provider); _cohort.Setup(x => x.IsApprovedByAllParties).Returns(false); _cohort.Setup(x => x.SendToOtherParty(Party.Provider, It.IsAny(), It.IsAny(), It.IsAny())); @@ -63,6 +71,12 @@ public ProviderSendCohortCommandHandlerTestsFixture() _dbContext.Object.SaveChanges(); } + public ProviderSendCohortCommandHandlerTestsFixture SetWithPartyToEmployer() + { + _cohort.Setup(x => x.WithParty).Returns(Party.Employer); + return this; + } + public async Task Handle() { await _handler.Handle(_command, _messageHandlerContext.Object); @@ -72,6 +86,11 @@ public void VerifyCohortIsSentToOtherParty() { _cohort.Verify(x => x.SendToOtherParty(Party.Provider, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + + public void VerifySendToOtherPartyIsNotCalled() + { + _cohort.Verify(x => x.SendToOtherParty(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs index 27ed0fd07b..647e9624b9 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs @@ -7,6 +7,7 @@ using SFA.DAS.CommitmentsV2.Data; using SFA.DAS.CommitmentsV2.Domain.Interfaces; using SFA.DAS.CommitmentsV2.Messages.Commands; +using SFA.DAS.CommitmentsV2.Types; namespace SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers { @@ -26,6 +27,12 @@ public async Task Handle(ApproveTransferRequestCommand message, IMessageHandlerC try { var transferRequest = await _dbContext.Value.TransferRequests.Include(c => c.Cohort).Where(x=>x.Id == message.TransferRequestId).SingleAsync(); + if (transferRequest.Status == TransferApprovalStatus.Approved) + { + _logger.LogWarning($"Transfer Request {transferRequest.Id} has already been approved"); + return; + } + transferRequest.Approve(message.UserInfo, message.ApprovedOn); } catch (Exception e) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs index d6d80482f2..883fde9540 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs @@ -28,6 +28,12 @@ public async Task Handle(ProviderApproveCohortCommand message, IMessageHandlerCo var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); + if (cohort.Approvals.HasFlag(Party.Provider)) + { + _logger.LogWarning($"Cohort {message.CohortId} has already been approved by the Provider"); + return; + } + cohort.Approve(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); await _dbContext.Value.SaveChangesAsync(); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs index 430a973125..4ee797264b 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs @@ -28,6 +28,12 @@ public async Task Handle(ProviderSendCohortCommand message, IMessageHandlerConte var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); + if (cohort.WithParty != Party.Provider) + { + _logger.LogWarning($"Cohort {message.CohortId} has already been SentToOtherParty by the Provider"); + return; + } + cohort.SendToOtherParty(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); await _dbContext.Value.SaveChangesAsync(); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2/Models/Cohort.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2/Models/Cohort.cs index 9ee367d7fb..8ee44fc21f 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2/Models/Cohort.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2/Models/Cohort.cs @@ -158,7 +158,7 @@ internal Cohort(Provider provider, public string LastMessage => Messages.OrderByDescending(x => x.Id).FirstOrDefault()?.Text; - public Party WithParty + public virtual Party WithParty { get { @@ -642,7 +642,7 @@ private void UpdatedBy(Party party, UserInfo userInfo) } } - public Party Approvals + public virtual Party Approvals { get { From 15894fffff063d524e7b92a7398715a0b11d28c8 Mon Sep 17 00:00:00 2001 From: Paul Graham Date: Wed, 26 Feb 2020 10:54:38 +0000 Subject: [PATCH 2/4] added logging of exceptions to command handlers, and ability of the handlers to consume commands which are being replayed (and log warning) --- ...proveTransferRequestCommandHandlerTests.cs | 7 ++++ ...roviderApproveCohortCommandHandlerTests.cs | 33 +++++++++++++++--- .../ProviderSendCohortCommandHandlerTests.cs | 30 ++++++++++++++-- ...ejectTransferRequestCommandHandlerTests.cs | 34 +++++++++++++++---- .../SendEmailToEmployerCommandHandlerTests.cs | 33 +++++++++++++++--- .../SendEmailToProviderCommandHandlerTests.cs | 28 +++++++++++++-- ...erEventHandlerForLegacyTaskCounterTests.cs | 1 + ...ransferRequestApprovedEventHandlerTests.cs | 1 + ...ransferRequestRejectedEventHandlerTests.cs | 1 + ...itmentsV2.MessageHandlers.UnitTests.csproj | 3 +- .../ApproveTransferRequestCommandHandler.cs | 3 +- .../ProviderApproveCohortCommandHandler.cs | 26 +++++++++----- .../ProviderSendCohortCommandHandler.cs | 26 +++++++++----- .../RejectTransferRequestCommandHandler.cs | 7 ++++ .../SendEmailToEmployerCommandHandler.cs | 6 +++- .../SendEmailToProviderCommandHandler.cs | 32 +++++++++++------ .../FakeLogger.cs | 3 +- .../SFA.DAS.CommitmentsV2.TestHelpers.csproj | 3 +- .../SFA.DAS.CommitmentsV2.UnitTests.csproj | 2 +- 19 files changed, 223 insertions(+), 56 deletions(-) rename src/CommitmentsV2/{SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests => SFA.DAS.CommitmentsV2.TestHelpers}/FakeLogger.cs (87%) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs index 6a25fea98e..1504f30c8a 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs @@ -13,6 +13,7 @@ using SFA.DAS.CommitmentsV2.Messages.Commands; using SFA.DAS.CommitmentsV2.Messages.Events; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; using SFA.DAS.UnitOfWork.Context; @@ -53,6 +54,7 @@ public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ForASecondTime fixture.Handle(); fixture.VerifyTransferRequestApprovedEventIsNotPublished(); + fixture.VerifyHasWarning(); } @@ -172,6 +174,11 @@ public void VerifyHasError() Assert.IsTrue(Logger.HasErrors); } + public void VerifyHasWarning() + { + Assert.IsTrue(Logger.HasWarnings); + } + public void VerifyTransferRequestApprovedEventIsPublished() { var list = UnitOfWorkContext.GetEvents().OfType().ToList(); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs index dc6c195670..2c71b9a053 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs @@ -2,7 +2,6 @@ using System.Threading.Tasks; using AutoFixture; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Logging; using NUnit.Framework; using Moq; using NServiceBus; @@ -10,6 +9,7 @@ using SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers; using SFA.DAS.CommitmentsV2.Messages.Commands; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; namespace SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.CommandHandlers @@ -33,10 +33,18 @@ public async Task When_HandlingCommand_ApproveCohort() } [Test] - public async Task When_HandlingCommand_ApproveCohort_Again_ShouldNotCallCohortApproval() + public async Task When_HandlingCommand_ApproveCohort_Again_ShouldNotCallCohortApprovalAndShouldLogWarning() { await _fixture.AddAlreadyApprovedByProvider().Handle(); _fixture.VerifyCohortApprovalWasNotCalled(); + _fixture.VerifyHasWarning(); + } + + [Test] + public void Handle_WhenHandlingCommandAndItFails_ThenItShouldThrowAnExceptionAndLogIt() + { + Assert.ThrowsAsync( () =>_fixture.SetupNullMessage().Handle()); + _fixture.VerifyHasError(); } public class ProviderApproveCohortCommandHandlerTestsFixture @@ -45,6 +53,7 @@ public class ProviderApproveCohortCommandHandlerTestsFixture private ProviderApproveCohortCommand _command; public Mock _dbContext { get; set; } private Mock _messageHandlerContext; + private FakeLogger _logger; private Mock _cohort; public ProviderApproveCohortCommandHandlerTestsFixture() @@ -52,8 +61,9 @@ public ProviderApproveCohortCommandHandlerTestsFixture() var autoFixture = new Fixture(); _dbContext = new Mock(new DbContextOptionsBuilder().UseInMemoryDatabase(Guid.NewGuid().ToString()).Options) { CallBase = true }; - - _handler = new ProviderApproveCohortCommandHandler(Mock.Of>(), + _logger = new FakeLogger(); + + _handler = new ProviderApproveCohortCommandHandler(_logger, new Lazy(() => _dbContext.Object)); _messageHandlerContext = new Mock(); @@ -77,6 +87,11 @@ public ProviderApproveCohortCommandHandlerTestsFixture AddAlreadyApprovedByProvi _cohort.Setup(x => x.Approvals).Returns(Party.Provider); return this; } + public ProviderApproveCohortCommandHandlerTestsFixture SetupNullMessage() + { + _command = null; + return this; + } public async Task Handle() { @@ -87,10 +102,20 @@ public void VerifyCohortApproval() { _cohort.Verify(x=> x.Approve(Party.Provider, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + public void VerifyCohortApprovalWasNotCalled() { _cohort.Verify(x => x.Approve(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + + public void VerifyHasError() + { + Assert.IsTrue(_logger.HasErrors); + } + public void VerifyHasWarning() + { + Assert.IsTrue(_logger.HasWarnings); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs index 61c2999506..f1a8b693b8 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs @@ -2,7 +2,6 @@ using System.Threading.Tasks; using AutoFixture; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Logging; using NUnit.Framework; using Moq; using NServiceBus; @@ -10,6 +9,7 @@ using SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers; using SFA.DAS.CommitmentsV2.Messages.Commands; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; namespace SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.CommandHandlers @@ -33,10 +33,18 @@ public async Task When_HandlingCommand_CohortIsSentToOtherParty() } [Test] - public async Task When_HandlingCommand_CohortIsAlreadySentToOtherParty_Then_ShouldNotCallSendToOtherParty() + public async Task When_HandlingCommand_CohortIsAlreadySentToOtherParty_Then_ShouldNotCallSendToOtherPartyAndLogWarning() { await _fixture.SetWithPartyToEmployer().Handle(); _fixture.VerifySendToOtherPartyIsNotCalled(); + _fixture.VerifyHasWarning(); + } + + [Test] + public void Handle_WhenHandlingCommandAndItFails_ThenItShouldThrowAnExceptionAndLogIt() + { + Assert.ThrowsAsync(() => _fixture.SetupNullMessage().Handle()); + _fixture.VerifyHasError(); } public class ProviderSendCohortCommandHandlerTestsFixture @@ -45,6 +53,7 @@ public class ProviderSendCohortCommandHandlerTestsFixture private ProviderSendCohortCommand _command; public Mock _dbContext { get; set; } private Mock _messageHandlerContext; + private FakeLogger _logger; private Mock _cohort; public ProviderSendCohortCommandHandlerTestsFixture() @@ -52,8 +61,9 @@ public ProviderSendCohortCommandHandlerTestsFixture() var autoFixture = new Fixture(); _dbContext = new Mock(new DbContextOptionsBuilder().UseInMemoryDatabase(Guid.NewGuid().ToString()).Options) { CallBase = true }; + _logger = new FakeLogger(); - _handler = new ProviderSendCohortCommandHandler(Mock.Of>(), + _handler = new ProviderSendCohortCommandHandler(_logger, new Lazy(() => _dbContext.Object)); _messageHandlerContext = new Mock(); @@ -76,6 +86,12 @@ public ProviderSendCohortCommandHandlerTestsFixture SetWithPartyToEmployer() _cohort.Setup(x => x.WithParty).Returns(Party.Employer); return this; } + public ProviderSendCohortCommandHandlerTestsFixture SetupNullMessage() + { + _command = null; + return this; + } + public async Task Handle() { @@ -91,6 +107,14 @@ public void VerifySendToOtherPartyIsNotCalled() { _cohort.Verify(x => x.SendToOtherParty(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + public void VerifyHasError() + { + Assert.IsTrue(_logger.HasErrors); + } + public void VerifyHasWarning() + { + Assert.IsTrue(_logger.HasWarnings); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs index d7e05d5ef7..ec2cb761fe 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs @@ -1,6 +1,5 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; -using Microsoft.Extensions.Logging; using Moq; using NServiceBus; using NUnit.Framework; @@ -14,6 +13,7 @@ using System; using System.Linq; using System.Threading.Tasks; +using SFA.DAS.CommitmentsV2.TestHelpers; namespace SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.CommandHandlers { @@ -29,9 +29,10 @@ public void Setup() } [Test] - public Task WhenHandlingRejectTransferRequestCommand_AndDbDoesNotHaveRecord_ThenExceptionThrown() + public void WhenHandlingRejectTransferRequestCommand_AndDbDoesNotHaveRecord_ThenExceptionThrownAndLogged() { - return Task.FromResult(Assert.ThrowsAsync(() => _fixture.Act())); + Assert.ThrowsAsync(() => _fixture.Act()); + _fixture.VerifyHasError(); } [Test] @@ -44,7 +45,6 @@ public async Task WhenHandlingRejectTransferRequestCommand_AndDbHasRecord_ThenNo [TestCase(TransferApprovalStatus.Pending, false)] [TestCase(TransferApprovalStatus.Approved, true)] - [TestCase(TransferApprovalStatus.Rejected, true)] public async Task WhenCallingRejectOnTransferRequest_ThenTransferApprovalStatusHandledCorrectly(TransferApprovalStatus status, bool shouldThrowException) { _fixture @@ -61,6 +61,17 @@ public async Task WhenCallingRejectOnTransferRequest_ThenTransferApprovalStatusH } } + [Test] + public async Task WhenCallingRejectOnTransferRequestAndAlreadyBeenRejected_ThenShouldSwallowMessageAndLogWarning() + { + _fixture + .WithTransferRequestAndCohortInDatabase() + .WithTransferApprovalStatus(TransferApprovalStatus.Rejected); + + await _fixture.Act(); + _fixture.VerifyHasWarning(); + } + [Test] public async Task WhenCallingRejectOnTransferRequest_ThenEntityIsTracked() { @@ -106,7 +117,7 @@ public class RejectTransferRequestCommandHandlerTestsFixture private TransferRequest _transferRequest; private Cohort _cohort; private ProviderCommitmentsDbContext _db; - private Mock> _mockLogger; + private FakeLogger _logger; private Mock _mockMessageHandlerContext; private RejectTransferRequestCommandHandler _sut; private UnitOfWorkContext _unitOfWorkContext; @@ -137,12 +148,12 @@ public RejectTransferRequestCommandHandlerTestsFixture() .ConfigureWarnings(w => w.Throw(RelationalEventId.QueryClientEvaluationWarning)) .Options); - _mockLogger = new Mock>(); + _logger = new FakeLogger(); _mockMessageHandlerContext = new Mock(); _rejectedOnDate = new DateTime(2020, 01, 30, 14, 22, 00); _command = new RejectTransferRequestCommand(_transferRequestId, _rejectedOnDate, _userInfo); _unitOfWorkContext = new UnitOfWorkContext(); - _sut = new RejectTransferRequestCommandHandler(new Lazy(() => _db), _mockLogger.Object); + _sut = new RejectTransferRequestCommandHandler(new Lazy(() => _db), _logger); } public async Task Act() => await _sut.Handle(_command, _mockMessageHandlerContext.Object); @@ -201,5 +212,14 @@ public void VerifyEntityIsBeingTracked() Assert.AreEqual(_userInfo.UserDisplayName, rejectedEvent.UpdatingUserName); Assert.AreEqual(Party.TransferSender, rejectedEvent.UpdatingParty); } + + public void VerifyHasError() + { + Assert.IsTrue(_logger.HasErrors); + } + public void VerifyHasWarning() + { + Assert.IsTrue(_logger.HasWarnings); + } } } \ No newline at end of file diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs index eddc4d88ef..d681df4205 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using AutoFixture; @@ -8,6 +9,7 @@ using NUnit.Framework; using SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers; using SFA.DAS.CommitmentsV2.Messages.Commands; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.EAS.Account.Api.Client; using SFA.DAS.EAS.Account.Api.Types; using SFA.DAS.Notifications.Messages.Commands; @@ -82,6 +84,14 @@ public async Task When_HandlingCommand_AndTheOnlyUserNotificationsOnHasAEmptyEma _fixture.SetupCommandWithoutEmailAddress().AddEmployeeOwnerWithEmail(email); await _fixture.Handle(); _fixture.VerifyNoMessagesSent(); + _fixture.VerifyHasWarning(); + } + + [Test] + public void When_HandlingCommandAndAnErrorOccurs_AnExceptionIsThrownAndLogged() + { + Assert.ThrowsAsync(() => _fixture.SetupNullMessage().Handle()); + _fixture.VerifyHasError(); } private class SendEmailToEmployerCommandHandlerTestsFixture @@ -89,7 +99,7 @@ private class SendEmailToEmployerCommandHandlerTestsFixture public SendEmailToEmployerCommandHandler Handler; public SendEmailToEmployerCommand Command; public Mock AccountApiClient; - public Mock> Logger; + public FakeLogger Logger; public Mock MessageHandlerContext; public Mock PipelineContext; @@ -110,13 +120,13 @@ public SendEmailToEmployerCommandHandlerTestsFixture() Tokens = _autoFixture.Create>(); Employees = _autoFixture.CreateMany().ToList(); - Logger = new Mock>(); + Logger = new FakeLogger(); MessageHandlerContext = new Mock(); PipelineContext = MessageHandlerContext.As(); AccountApiClient = new Mock(); AccountApiClient.Setup(x => x.GetAccountUsers(It.IsAny())).ReturnsAsync(Employees); - Handler = new SendEmailToEmployerCommandHandler(AccountApiClient.Object, Logger.Object); + Handler = new SendEmailToEmployerCommandHandler(AccountApiClient.Object, Logger); } public SendEmailToEmployerCommandHandlerTestsFixture SetupCommandWithoutEmailAddress() @@ -131,6 +141,12 @@ public SendEmailToEmployerCommandHandlerTestsFixture SetupCommandWithSpecificEma return this; } + public SendEmailToEmployerCommandHandlerTestsFixture SetupNullMessage() + { + Command = null; + return this; + } + public SendEmailToEmployerCommandHandlerTestsFixture AddToEmployeeListOneOwnerOneTransactorAndOneViewer() { Employees.Add(new TeamMemberViewModel { CanReceiveNotifications = true, Email = "owner@test.com", Role = "Owner" }); @@ -183,6 +199,15 @@ public void VerifyMessageAreSentForOwnersAndTransactors() PipelineContext.Verify(x => x.Send(It.Is(c => c.RecipientsAddress == "owner@test.com"), It.IsAny()), Times.Once); PipelineContext.Verify(x => x.Send(It.Is(c => c.RecipientsAddress == "transactor@test.com"), It.IsAny()), Times.Once); } + + public void VerifyHasError() + { + Assert.IsTrue(Logger.HasErrors); + } + public void VerifyHasWarning() + { + Assert.IsTrue(Logger.HasWarnings); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToProviderCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToProviderCommandHandlerTests.cs index 30c99e5dee..7441337314 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToProviderCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToProviderCommandHandlerTests.cs @@ -1,13 +1,16 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; using AutoFixture; +using Microsoft.Extensions.Logging; using Moq; using NServiceBus; using NUnit.Framework; using SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers; using SFA.DAS.CommitmentsV2.Messages.Commands; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.PAS.Account.Api.ClientV2; using SFA.DAS.PAS.Account.Api.Types; @@ -40,18 +43,25 @@ public async Task When_HandlingCommand_EmailIsSentToExplicitAddressIfSpecified() _fixture.VerifyEmailIsSentToExplicitAddress(); } + [Test] + public void When_HandlingCommandAndAnErrorOccurs_AnExceptionIsThrownAndLogged() + { + Assert.ThrowsAsync(() => _fixture.SetupNullMessage().Handle()); + _fixture.VerifyHasError(); + } + private class SendEmailToProviderCommandHandlerTestsFixture { public SendEmailToProviderCommandHandler Handler; public SendEmailToProviderCommand Command; public Mock PasAccountApiClient; public Mock MessageHandlerContext; + public FakeLogger Logger; public long ProviderId; public string TemplateId; public string ExplicitEmailAddress; public Dictionary Tokens; - private Fixture _autoFixture; @@ -66,12 +76,13 @@ public SendEmailToProviderCommandHandlerTestsFixture() Command = new SendEmailToProviderCommand(ProviderId, TemplateId, Tokens, null); MessageHandlerContext = new Mock(); + Logger = new FakeLogger(); PasAccountApiClient = new Mock(); PasAccountApiClient.Setup(x => x.SendEmailToAllProviderRecipients(It.IsAny(), It.IsAny(), It.IsAny())); - Handler = new SendEmailToProviderCommandHandler(PasAccountApiClient.Object); + Handler = new SendEmailToProviderCommandHandler(PasAccountApiClient.Object, Logger); } public SendEmailToProviderCommandHandlerTestsFixture SetExplicitEmailAddress() @@ -80,6 +91,12 @@ public SendEmailToProviderCommandHandlerTestsFixture SetExplicitEmailAddress() return this; } + public SendEmailToProviderCommandHandlerTestsFixture SetupNullMessage() + { + Command = null; + return this; + } + public async Task Handle() { await Handler.Handle(Command, MessageHandlerContext.Object); @@ -101,6 +118,11 @@ public void VerifyEmailIsSentToExplicitAddress() && r.TemplateId == TemplateId && r.Tokens == Tokens), It.IsAny())); } + + public void VerifyHasError() + { + Assert.IsTrue(Logger.HasErrors); + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/CohortAssignedToEmployerEventHandlerForLegacyTaskCounterTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/CohortAssignedToEmployerEventHandlerForLegacyTaskCounterTests.cs index 41b28b2415..e1b47f1990 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/CohortAssignedToEmployerEventHandlerForLegacyTaskCounterTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/CohortAssignedToEmployerEventHandlerForLegacyTaskCounterTests.cs @@ -14,6 +14,7 @@ using SFA.DAS.CommitmentsV2.MessageHandlers.EventHandlers; using SFA.DAS.CommitmentsV2.Messages.Events; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; using SFA.DAS.UnitOfWork.Context; diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestApprovedEventHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestApprovedEventHandlerTests.cs index 56e59f1a59..8701378e5e 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestApprovedEventHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestApprovedEventHandlerTests.cs @@ -13,6 +13,7 @@ using SFA.DAS.CommitmentsV2.MessageHandlers.EventHandlers; using SFA.DAS.CommitmentsV2.Messages.Events; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; using SFA.DAS.UnitOfWork.Context; diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestRejectedEventHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestRejectedEventHandlerTests.cs index c2d2c9ba89..983f49c1d2 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestRejectedEventHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/EventHandlers/TransferRequestRejectedEventHandlerTests.cs @@ -15,6 +15,7 @@ using SFA.DAS.CommitmentsV2.MessageHandlers.EventHandlers; using SFA.DAS.CommitmentsV2.Messages.Events; using SFA.DAS.CommitmentsV2.Models; +using SFA.DAS.CommitmentsV2.TestHelpers; using SFA.DAS.CommitmentsV2.Types; using SFA.DAS.UnitOfWork.Context; diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.csproj b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.csproj index dd94e453ac..ade3ab7805 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.csproj +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests.csproj @@ -9,7 +9,7 @@ - + @@ -23,6 +23,7 @@ + diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs index 647e9624b9..847e12217d 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ApproveTransferRequestCommandHandler.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Logging; using NServiceBus; using SFA.DAS.CommitmentsV2.Data; -using SFA.DAS.CommitmentsV2.Domain.Interfaces; using SFA.DAS.CommitmentsV2.Messages.Commands; using SFA.DAS.CommitmentsV2.Types; @@ -37,7 +36,7 @@ public async Task Handle(ApproveTransferRequestCommand message, IMessageHandlerC } catch (Exception e) { - _logger.LogError("Error processing TransferSenderApproveCohortCommand", e); + _logger.LogError($"Error processing {nameof(ApproveTransferRequestCommand)}", e); throw; } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs index 883fde9540..0459318c7a 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs @@ -24,19 +24,27 @@ public ProviderApproveCohortCommandHandler( public async Task Handle(ProviderApproveCohortCommand message, IMessageHandlerContext context) { - _logger.LogInformation($"Handled {nameof(ProviderApproveCohortCommand)} with MessageId '{context.MessageId}'"); + try + { + _logger.LogInformation($"Handled {nameof(ProviderApproveCohortCommand)} with MessageId '{context.MessageId}'"); - var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); + var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); - if (cohort.Approvals.HasFlag(Party.Provider)) - { - _logger.LogWarning($"Cohort {message.CohortId} has already been approved by the Provider"); - return; - } + if (cohort.Approvals.HasFlag(Party.Provider)) + { + _logger.LogWarning($"Cohort {message.CohortId} has already been approved by the Provider"); + return; + } - cohort.Approve(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); + cohort.Approve(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); - await _dbContext.Value.SaveChangesAsync(); + await _dbContext.Value.SaveChangesAsync(); + } + catch (Exception e) + { + _logger.LogError($"Error processing {nameof(ProviderApproveCohortCommand)}", e); + throw; + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs index 4ee797264b..7ced4b582d 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs @@ -24,19 +24,27 @@ public ProviderSendCohortCommandHandler( public async Task Handle(ProviderSendCohortCommand message, IMessageHandlerContext context) { - _logger.LogInformation($"Handled {nameof(ProviderSendCohortCommand)} with MessageId '{context.MessageId}'"); + try + { + _logger.LogInformation($"Handled {nameof(ProviderSendCohortCommand)} with MessageId '{context.MessageId}'"); - var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); + var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); - if (cohort.WithParty != Party.Provider) - { - _logger.LogWarning($"Cohort {message.CohortId} has already been SentToOtherParty by the Provider"); - return; - } + if (cohort.WithParty != Party.Provider) + { + _logger.LogWarning($"Cohort {message.CohortId} has already been SentToOtherParty by the Provider"); + return; + } - cohort.SendToOtherParty(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); + cohort.SendToOtherParty(Party.Provider, message.Message, message.UserInfo, DateTime.UtcNow); - await _dbContext.Value.SaveChangesAsync(); + await _dbContext.Value.SaveChangesAsync(); + } + catch (Exception e) + { + _logger.LogError($"Error processing {nameof(ProviderSendCohortCommand)}", e); + throw; + } } } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/RejectTransferRequestCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/RejectTransferRequestCommandHandler.cs index f6e4c64def..76bee44aa0 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/RejectTransferRequestCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/RejectTransferRequestCommandHandler.cs @@ -6,6 +6,7 @@ using NServiceBus; using SFA.DAS.CommitmentsV2.Data; using SFA.DAS.CommitmentsV2.Messages.Commands; +using SFA.DAS.CommitmentsV2.Types; namespace SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers { @@ -29,6 +30,12 @@ public async Task Handle(RejectTransferRequestCommand message, IMessageHandlerCo .Where(x => x.Id == message.TransferRequestId) .SingleAsync(); + if (transferRequest.Status == TransferApprovalStatus.Rejected) + { + _logger.LogWarning($"Cohort {message.TransferRequestId} has already Rejected"); + return; + } + transferRequest.Reject(message.UserInfo, message.RejectedOn); } catch (Exception e) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToEmployerCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToEmployerCommandHandler.cs index 949bd809ea..4da773cb89 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToEmployerCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToEmployerCommandHandler.cs @@ -56,10 +56,14 @@ bool IsOwnerOrTransactor(string role) await Task.WhenAll(emails.Select(email => context.Send(new SendEmailCommand(message.Template, email, message.Tokens)))); } + else + { + _logger.LogWarning($"No Email Addresses found to send Template {message.Template} for AccountId {message.AccountId}"); + } } catch (Exception e) { - _logger.LogError("Error processing SendEmailToEmployerCommand", e); + _logger.LogError($"Error processing {nameof(SendEmailToEmployerCommand)}", e); throw; } } diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToProviderCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToProviderCommandHandler.cs index 909c160b0e..eceb05e01b 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToProviderCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/SendEmailToProviderCommandHandler.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using NServiceBus; using SFA.DAS.CommitmentsV2.Messages.Commands; using SFA.DAS.PAS.Account.Api.ClientV2; @@ -10,24 +12,34 @@ namespace SFA.DAS.CommitmentsV2.MessageHandlers.CommandHandlers public class SendEmailToProviderCommandHandler : IHandleMessages { private readonly IPasAccountApiClient _pasAccountApi; + private readonly ILogger _logger; - public SendEmailToProviderCommandHandler(IPasAccountApiClient pasAccountApi) + public SendEmailToProviderCommandHandler(IPasAccountApiClient pasAccountApi, ILogger logger) { _pasAccountApi = pasAccountApi; + _logger = logger; } public async Task Handle(SendEmailToProviderCommand message, IMessageHandlerContext context) { - var providerEmailRequest = new ProviderEmailRequest + try { - TemplateId = message.Template, - Tokens = message.Tokens, - ExplicitEmailAddresses = string.IsNullOrWhiteSpace(message.EmailAddress) - ? new List() - : new List { message.EmailAddress } - }; + var providerEmailRequest = new ProviderEmailRequest + { + TemplateId = message.Template, + Tokens = message.Tokens, + ExplicitEmailAddresses = string.IsNullOrWhiteSpace(message.EmailAddress) + ? new List() + : new List {message.EmailAddress} + }; - await _pasAccountApi.SendEmailToAllProviderRecipients(message.ProviderId, providerEmailRequest); + await _pasAccountApi.SendEmailToAllProviderRecipients(message.ProviderId, providerEmailRequest); + } + catch (Exception e) + { + _logger.LogError($"Error processing {nameof(SendEmailToProviderCommand)}", e); + throw; + } } } } \ No newline at end of file diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/FakeLogger.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/FakeLogger.cs similarity index 87% rename from src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/FakeLogger.cs rename to src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/FakeLogger.cs index 1dc9e1c08a..95bfd5b1e9 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/FakeLogger.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/FakeLogger.cs @@ -3,7 +3,7 @@ using System.Linq; using Microsoft.Extensions.Logging; -namespace SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests +namespace SFA.DAS.CommitmentsV2.TestHelpers { public class FakeLogger : ILogger { @@ -26,5 +26,6 @@ public IDisposable BeginScope(TState state) public bool HasErrors => LogMessages.Any(l => l.logLevel == LogLevel.Error); public bool HasInfo => LogMessages.Any(l => l.logLevel == LogLevel.Information); + public bool HasWarnings => LogMessages.Any(l => l.logLevel == LogLevel.Warning); } } \ No newline at end of file diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/SFA.DAS.CommitmentsV2.TestHelpers.csproj b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/SFA.DAS.CommitmentsV2.TestHelpers.csproj index 9c85dba2e7..96c8fc7824 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/SFA.DAS.CommitmentsV2.TestHelpers.csproj +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.TestHelpers/SFA.DAS.CommitmentsV2.TestHelpers.csproj @@ -6,7 +6,8 @@ - + + diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.UnitTests/SFA.DAS.CommitmentsV2.UnitTests.csproj b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.UnitTests/SFA.DAS.CommitmentsV2.UnitTests.csproj index 7eada77848..e971a5e762 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.UnitTests/SFA.DAS.CommitmentsV2.UnitTests.csproj +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.UnitTests/SFA.DAS.CommitmentsV2.UnitTests.csproj @@ -9,7 +9,7 @@ - + From e9d50a4bc111726fc4d96e275d1b2915d5c7d1ea Mon Sep 17 00:00:00 2001 From: Paul Graham Date: Wed, 26 Feb 2020 11:30:22 +0000 Subject: [PATCH 3/4] removed extra spaces/lines --- .../ApproveTransferRequestCommandHandlerTests.cs | 2 -- .../ProviderApproveCohortCommandHandlerTests.cs | 2 ++ .../CommandHandlers/ProviderSendCohortCommandHandlerTests.cs | 3 ++- .../RejectTransferRequestCommandHandlerTests.cs | 1 + .../CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs | 1 + 5 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs index 1504f30c8a..0310c6bd56 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ApproveTransferRequestCommandHandlerTests.cs @@ -57,7 +57,6 @@ public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ForASecondTime fixture.VerifyHasWarning(); } - [Test] public void Handle_WhenHandlingTransferSenderApproveCohortCommand_ThenItShouldPublishChangeTrackingEvents() { @@ -197,7 +196,6 @@ public void VerifyTransferRequestApprovedEventIsNotPublished() Assert.AreEqual(0, list.Count); } - public void VerifyEntityIsBeingTracked() { var list = UnitOfWorkContext.GetEvents().OfType().Where(x=>x.StateChangeType == UserAction.ApproveTransferRequest).ToList(); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs index 2c71b9a053..0f177f90ef 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderApproveCohortCommandHandlerTests.cs @@ -87,6 +87,7 @@ public ProviderApproveCohortCommandHandlerTestsFixture AddAlreadyApprovedByProvi _cohort.Setup(x => x.Approvals).Returns(Party.Provider); return this; } + public ProviderApproveCohortCommandHandlerTestsFixture SetupNullMessage() { _command = null; @@ -112,6 +113,7 @@ public void VerifyHasError() { Assert.IsTrue(_logger.HasErrors); } + public void VerifyHasWarning() { Assert.IsTrue(_logger.HasWarnings); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs index f1a8b693b8..2a3af36289 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/ProviderSendCohortCommandHandlerTests.cs @@ -92,7 +92,6 @@ public ProviderSendCohortCommandHandlerTestsFixture SetupNullMessage() return this; } - public async Task Handle() { await _handler.Handle(_command, _messageHandlerContext.Object); @@ -107,10 +106,12 @@ public void VerifySendToOtherPartyIsNotCalled() { _cohort.Verify(x => x.SendToOtherParty(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + public void VerifyHasError() { Assert.IsTrue(_logger.HasErrors); } + public void VerifyHasWarning() { Assert.IsTrue(_logger.HasWarnings); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs index ec2cb761fe..9c5fe93ec9 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/RejectTransferRequestCommandHandlerTests.cs @@ -217,6 +217,7 @@ public void VerifyHasError() { Assert.IsTrue(_logger.HasErrors); } + public void VerifyHasWarning() { Assert.IsTrue(_logger.HasWarnings); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs index d681df4205..cb71b0be96 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers.UnitTests/CommandHandlers/SendEmailToEmployerCommandHandlerTests.cs @@ -204,6 +204,7 @@ public void VerifyHasError() { Assert.IsTrue(Logger.HasErrors); } + public void VerifyHasWarning() { Assert.IsTrue(Logger.HasWarnings); From 7a884a8407729f37f6e65c32caae481f33261f1e Mon Sep 17 00:00:00 2001 From: Paul Graham Date: Thu, 27 Feb 2020 14:01:24 +0000 Subject: [PATCH 4/4] now says handling not handled --- .../CommandHandlers/ProviderApproveCohortCommandHandler.cs | 2 +- .../CommandHandlers/ProviderSendCohortCommandHandler.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs index 0459318c7a..883be1d6de 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderApproveCohortCommandHandler.cs @@ -26,7 +26,7 @@ public async Task Handle(ProviderApproveCohortCommand message, IMessageHandlerCo { try { - _logger.LogInformation($"Handled {nameof(ProviderApproveCohortCommand)} with MessageId '{context.MessageId}'"); + _logger.LogInformation($"Handling {nameof(ProviderApproveCohortCommand)} with MessageId '{context.MessageId}'"); var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default); diff --git a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs index 7ced4b582d..2308908139 100644 --- a/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs +++ b/src/CommitmentsV2/SFA.DAS.CommitmentsV2.MessageHandlers/CommandHandlers/ProviderSendCohortCommandHandler.cs @@ -26,7 +26,7 @@ public async Task Handle(ProviderSendCohortCommand message, IMessageHandlerConte { try { - _logger.LogInformation($"Handled {nameof(ProviderSendCohortCommand)} with MessageId '{context.MessageId}'"); + _logger.LogInformation($"Handling {nameof(ProviderSendCohortCommand)} with MessageId '{context.MessageId}'"); var cohort = await _dbContext.Value.GetCohortAggregate(message.CohortId, default);