diff --git a/.gitignore b/.gitignore index add57be..4460bfb 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ bin/ obj/ /packages/ riderModule.iml -/_ReSharper.Caches/ \ No newline at end of file +/_ReSharper.Caches/ +.vs/ \ No newline at end of file diff --git a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs index 3a607fd..dcd6d17 100644 --- a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs +++ b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs @@ -1,247 +1,104 @@ using System; using System.Collections.Generic; +using Moq; using NUnit.Framework; -using RefactorThis.Persistence; +using RefactorThis.Domain.enums; +using RefactorThis.Domain.messaging; +using RefactorThis.Persistence.messaging; +using RefactorThis.Persistence.models; +using RefactorThis.Persistence.repositories; namespace RefactorThis.Domain.Tests { - [TestFixture] - public class InvoicePaymentProcessorTests - { - [Test] - public void ProcessPayment_Should_ThrowException_When_NoInoiceFoundForPaymentReference( ) - { - var repo = new InvoiceRepository( ); - - Invoice invoice = null; - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ); - var failureMessage = ""; - - try - { - var result = paymentProcessor.ProcessPayment( payment ); - } - catch ( InvalidOperationException e ) - { - failureMessage = e.Message; - } - - Assert.AreEqual( "There is no invoice matching this payment", failureMessage ); - } - - [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_NoPaymentNeeded( ) - { - var repo = new InvoiceRepository( ); - - var invoice = new Invoice( repo ) - { - Amount = 0, - AmountPaid = 0, - Payments = null - }; - - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ); - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "no payment needed", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_InvoiceAlreadyFullyPaid( ) - { - var repo = new InvoiceRepository( ); - - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 10, - Payments = new List - { - new Payment - { - Amount = 10 - } - } - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ); - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "invoice was already fully paid", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_PartialPaymentExistsAndAmountPaidExceedsAmountDue( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 5, - Payments = new List - { - new Payment - { - Amount = 5 - } - } - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 6 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "the payment is greater than the partial amount remaining", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_NoPartialPaymentExistsAndAmountPaidExceedsInvoiceAmount( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 5, - AmountPaid = 0, - Payments = new List( ) - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 6 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "the payment is greater than the invoice amount", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExistsAndAmountPaidEqualsAmountDue( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 5, - Payments = new List - { - new Payment - { - Amount = 5 - } - } - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 5 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "final partial payment received, invoice is now fully paid", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidEqualsInvoiceAmount( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 0, - Payments = new List( ) { new Payment( ) { Amount = 10 } } - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 10 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "invoice was already fully paid", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPaymentExistsAndAmountPaidIsLessThanAmountDue( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 5, - Payments = new List - { - new Payment - { - Amount = 5 - } - } - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 1 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "another partial payment received, still not fully paid", result ); - } - - [Test] - public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidIsLessThanInvoiceAmount( ) - { - var repo = new InvoiceRepository( ); - var invoice = new Invoice( repo ) - { - Amount = 10, - AmountPaid = 0, - Payments = new List( ) - }; - repo.Add( invoice ); - - var paymentProcessor = new InvoiceService( repo ); - - var payment = new Payment( ) - { - Amount = 1 - }; - - var result = paymentProcessor.ProcessPayment( payment ); - - Assert.AreEqual( "invoice is now partially paid", result ); - } - } + [TestFixture] + public class InvoicePaymentProcessorTests + { + private static Guid _invoiceId = Guid.NewGuid(); + private Mock _mockRepo; + + [SetUp] + public void Setup() + { + _mockRepo = new Mock(); + } + + [Test] + public void ProcessPayment_Should_ThrowException_When_NoInvoiceFound() + { + _mockRepo.Setup(repo => repo.GetInvoice(_invoiceId)).Throws(new InvalidOperationException(InvoiceRepositoryMessages.NoInvoiceFound)); + + var paymentProcessor = new InvoiceService(_mockRepo.Object); + var payment = new Payment(); + + var ex = Assert.Throws(() => paymentProcessor.ProcessPayment(payment, _invoiceId)); + + Assert.That(ex.Message, Is.EqualTo(InvoiceRepositoryMessages.NoInvoiceFound)); + } + + [Test] + public void ProcessPayment_Should_ThrowException_When_InvoiceWithPaymentAlreadyExists() + { + var payment = new Payment + { + Reference = "Reference" + }; + var invoice = new Invoice + { + Id = _invoiceId, + Amount = 10, + Payments = null + }; + _mockRepo.Setup(repo => repo.GetInvoice(_invoiceId)).Returns(invoice); + _mockRepo.Setup(repo => repo.FindInvoiceWithPayment(payment.Reference)).Returns(new Invoice()); + + var paymentProcessor = new InvoiceService(_mockRepo.Object); + + var ex = Assert.Throws(() => paymentProcessor.ProcessPayment(payment, _invoiceId)); + + Assert.That(ex.Message, Is.EqualTo(ProcessPaymentErrorMessages.InvoiceWithPaymentAlreadyExists)); + } + + public static IEnumerable InvalidPaymentScenarios => + new[] + { + new TestCaseData(new Invoice { Id = _invoiceId, Amount = 0, Payments = null}, new Payment(), ProcessPaymentErrorMessages.NoPaymentNecessary), + new TestCaseData(new Invoice { Id = _invoiceId, Amount = 0, Payments = new List()}, new Payment(), ProcessPaymentErrorMessages.NoPaymentNecessary), + new TestCaseData(new Invoice { Id = _invoiceId, Amount = 10, Payments = new List{ new Payment {Amount = 10} } }, new Payment(), ProcessPaymentErrorMessages.InvoiceAlreadyPaid), + new TestCaseData(new Invoice { Id = _invoiceId, Amount = 10, Payments = new List{ new Payment {Amount = 5} } }, new Payment { Amount = 6 }, ProcessPaymentErrorMessages.PaymentTooGreat), + new TestCaseData(new Invoice { Id = _invoiceId, Amount = 5, Payments = new List() }, new Payment { Amount = 6 }, ProcessPaymentErrorMessages.PaymentTooGreat) + }; + + [Test, TestCaseSource(nameof(InvalidPaymentScenarios))] + public void ProcessPayment_Should_ThrowException_When_InvalidPayments(Invoice invoice, Payment payment, string errorMessage) + { + _mockRepo.Setup(repo => repo.GetInvoice(_invoiceId)).Returns(invoice); + + var paymentProcessor = new InvoiceService(_mockRepo.Object); + + var ex = Assert.Throws(() => paymentProcessor.ProcessPayment(payment, _invoiceId)); + + Assert.That(ex.Message, Is.EqualTo(errorMessage)); + } + + public static IEnumerable ValidPaymentScenarios => + new[] + { + new TestCaseData(new Invoice{Id = _invoiceId, Amount = 10, Payments = new List{ new Payment { Amount = 5} }}, new Payment { Amount = 5}, InvoicePaymentStatus.Paid), + new TestCaseData(new Invoice{Id = _invoiceId, Amount = 10, Payments = new List() }, new Payment { Amount = 10}, InvoicePaymentStatus.Paid), + new TestCaseData(new Invoice{Id = _invoiceId, Amount = 10, Payments = new List{ new Payment { Amount = 5} }}, new Payment { Amount = 1}, InvoicePaymentStatus.PartiallyPaid), + new TestCaseData(new Invoice{Id = _invoiceId, Amount = 10, Payments = new List() }, new Payment { Amount = 1}, InvoicePaymentStatus.PartiallyPaid) + }; + + [Test, TestCaseSource(nameof(ValidPaymentScenarios))] + public void ProcessPayment_Should_ReturnCorrectStatus_When_ValidPaymentMade(Invoice invoice, Payment payment, InvoicePaymentStatus status) + { + _mockRepo.Setup(repo => repo.GetInvoice(_invoiceId)).Returns(invoice); + + var paymentProcessor = new InvoiceService(_mockRepo.Object); + + var result = paymentProcessor.ProcessPayment(payment, _invoiceId); + + Assert.That(result, Is.EqualTo(status)); + } + } } \ No newline at end of file diff --git a/RefactorThis.Domain.Tests/Properties/AssemblyInfo.cs b/RefactorThis.Domain.Tests/Properties/AssemblyInfo.cs index 15a32a8..8bdf283 100644 --- a/RefactorThis.Domain.Tests/Properties/AssemblyInfo.cs +++ b/RefactorThis.Domain.Tests/Properties/AssemblyInfo.cs @@ -4,22 +4,22 @@ // General Information about an assembly is controlled through the following // set of attributes. Change these attribute values to modify the information // associated with an assembly. -[assembly: AssemblyTitle( "RefactorThis.Domain.Tests" )] -[assembly: AssemblyDescription( "" )] -[assembly: AssemblyConfiguration( "" )] -[assembly: AssemblyCompany( "" )] -[assembly: AssemblyProduct( "RefactorThis.Domain.Tests" )] -[assembly: AssemblyCopyright( "Copyright © 2021" )] -[assembly: AssemblyTrademark( "" )] -[assembly: AssemblyCulture( "" )] +[assembly: AssemblyTitle("RefactorThis.Domain.Tests")] +[assembly: AssemblyDescription("")] +[assembly: AssemblyConfiguration("")] +[assembly: AssemblyCompany("")] +[assembly: AssemblyProduct("RefactorThis.Domain.Tests")] +[assembly: AssemblyCopyright("Copyright © 2021")] +[assembly: AssemblyTrademark("")] +[assembly: AssemblyCulture("")] // Setting ComVisible to false makes the types in this assembly not visible // to COM components. If you need to access a type in this assembly from // COM, set the ComVisible attribute to true on that type. -[assembly: ComVisible( false )] +[assembly: ComVisible(false)] // The following GUID is for the ID of the typelib if this project is exposed to COM -[assembly: Guid( "7971BDEC-EAD1-4FB8-A4F5-B1F67E4F6355" )] +[assembly: Guid("7971BDEC-EAD1-4FB8-A4F5-B1F67E4F6355")] // Version information for an assembly consists of the following four values: // @@ -31,5 +31,5 @@ // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion( "1.0.0.0" )] -[assembly: AssemblyFileVersion( "1.0.0.0" )] \ No newline at end of file +[assembly: AssemblyVersion("1.0.0.0")] +[assembly: AssemblyFileVersion("1.0.0.0")] \ No newline at end of file diff --git a/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj b/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj index f118007..c80a3f4 100644 --- a/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj +++ b/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj @@ -1,67 +1,160 @@  - - - Debug - AnyCPU - {7971BDEC-EAD1-4FB8-A4F5-B1F67E4F6355} - {FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} - Library - Properties - RefactorThis.Domain.Tests - RefactorThis.Domain.Tests - v4.7.2 - 512 - - - AnyCPU - true - full - false - bin\Debug\ - DEBUG;TRACE - prompt - 4 - - - AnyCPU - pdbonly - true - bin\Release\ - TRACE - prompt - 4 - - - - - - - - ..\packages\NUnit.3.5.0\lib\net45\nunit.framework.dll - - - - - - - - - {5310b2fe-e26d-414e-b656-1f74c5a70368} - RefactorThis.Domain - - - {33cdc796-ff75-449c-9637-59c2efc46361} - RefactorThis.Persistence - - - - - - + \ No newline at end of file diff --git a/RefactorThis.Domain.Tests/app.config b/RefactorThis.Domain.Tests/app.config new file mode 100644 index 0000000..ef77b31 --- /dev/null +++ b/RefactorThis.Domain.Tests/app.config @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/RefactorThis.Domain.Tests/packages.config b/RefactorThis.Domain.Tests/packages.config index c108d44..1ba84d7 100644 --- a/RefactorThis.Domain.Tests/packages.config +++ b/RefactorThis.Domain.Tests/packages.config @@ -1,4 +1,23 @@  - + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/RefactorThis.Domain/InvoiceService.cs b/RefactorThis.Domain/InvoiceService.cs deleted file mode 100644 index fbd674c..0000000 --- a/RefactorThis.Domain/InvoiceService.cs +++ /dev/null @@ -1,149 +0,0 @@ -using System; -using System.Linq; -using RefactorThis.Persistence; - -namespace RefactorThis.Domain -{ - public class InvoiceService - { - private readonly InvoiceRepository _invoiceRepository; - - public InvoiceService( InvoiceRepository invoiceRepository ) - { - _invoiceRepository = invoiceRepository; - } - - public string ProcessPayment( Payment payment ) - { - var inv = _invoiceRepository.GetInvoice( payment.Reference ); - - var responseMessage = string.Empty; - - if ( inv == null ) - { - throw new InvalidOperationException( "There is no invoice matching this payment" ); - } - else - { - if ( inv.Amount == 0 ) - { - if ( inv.Payments == null || !inv.Payments.Any( ) ) - { - responseMessage = "no payment needed"; - } - else - { - throw new InvalidOperationException( "The invoice is in an invalid state, it has an amount of 0 and it has payments." ); - } - } - else - { - if ( inv.Payments != null && inv.Payments.Any( ) ) - { - if ( inv.Payments.Sum( x => x.Amount ) != 0 && inv.Amount == inv.Payments.Sum( x => x.Amount ) ) - { - responseMessage = "invoice was already fully paid"; - } - else if ( inv.Payments.Sum( x => x.Amount ) != 0 && payment.Amount > ( inv.Amount - inv.AmountPaid ) ) - { - responseMessage = "the payment is greater than the partial amount remaining"; - } - else - { - if ( ( inv.Amount - inv.AmountPaid ) == payment.Amount ) - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid += payment.Amount; - inv.Payments.Add( payment ); - responseMessage = "final partial payment received, invoice is now fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid += payment.Amount; - inv.TaxAmount += payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "final partial payment received, invoice is now fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - - } - else - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid += payment.Amount; - inv.Payments.Add( payment ); - responseMessage = "another partial payment received, still not fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid += payment.Amount; - inv.TaxAmount += payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "another partial payment received, still not fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } - } - } - else - { - if ( payment.Amount > inv.Amount ) - { - responseMessage = "the payment is greater than the invoice amount"; - } - else if ( inv.Amount == payment.Amount ) - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now fully paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now fully paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } - else - { - switch ( inv.Type ) - { - case InvoiceType.Standard: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now partially paid"; - break; - case InvoiceType.Commercial: - inv.AmountPaid = payment.Amount; - inv.TaxAmount = payment.Amount * 0.14m; - inv.Payments.Add( payment ); - responseMessage = "invoice is now partially paid"; - break; - default: - throw new ArgumentOutOfRangeException( ); - } - } - } - } - } - - inv.Save(); - - return responseMessage; - } - } -} \ No newline at end of file diff --git a/RefactorThis.Domain/Properties/AssemblyInfo.cs b/RefactorThis.Domain/Properties/AssemblyInfo.cs index 5901127..3466c58 100644 --- a/RefactorThis.Domain/Properties/AssemblyInfo.cs +++ b/RefactorThis.Domain/Properties/AssemblyInfo.cs @@ -4,22 +4,22 @@ // General Information about an assembly is controlled through the following // set of attributes. Change these attribute values to modify the information // associated with an assembly. -[assembly: AssemblyTitle( "RefactorThis.Domain" )] -[assembly: AssemblyDescription( "" )] -[assembly: AssemblyConfiguration( "" )] -[assembly: AssemblyCompany( "" )] -[assembly: AssemblyProduct( "RefactorThis.Domain" )] -[assembly: AssemblyCopyright( "Copyright © 2021" )] -[assembly: AssemblyTrademark( "" )] -[assembly: AssemblyCulture( "" )] +[assembly: AssemblyTitle("RefactorThis.Domain")] +[assembly: AssemblyDescription("")] +[assembly: AssemblyConfiguration("")] +[assembly: AssemblyCompany("")] +[assembly: AssemblyProduct("RefactorThis.Domain")] +[assembly: AssemblyCopyright("Copyright © 2021")] +[assembly: AssemblyTrademark("")] +[assembly: AssemblyCulture("")] // Setting ComVisible to false makes the types in this assembly not visible // to COM components. If you need to access a type in this assembly from // COM, set the ComVisible attribute to true on that type. -[assembly: ComVisible( false )] +[assembly: ComVisible(false)] // The following GUID is for the ID of the typelib if this project is exposed to COM -[assembly: Guid( "5310B2FE-E26D-414E-B656-1F74C5A70368" )] +[assembly: Guid("5310B2FE-E26D-414E-B656-1F74C5A70368")] // Version information for an assembly consists of the following four values: // @@ -31,5 +31,5 @@ // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion( "1.0.0.0" )] -[assembly: AssemblyFileVersion( "1.0.0.0" )] \ No newline at end of file +[assembly: AssemblyVersion("1.0.0.0")] +[assembly: AssemblyFileVersion("1.0.0.0")] \ No newline at end of file diff --git a/RefactorThis.Domain/RefactorThis.Domain.csproj b/RefactorThis.Domain/RefactorThis.Domain.csproj index 753e893..64ce149 100644 --- a/RefactorThis.Domain/RefactorThis.Domain.csproj +++ b/RefactorThis.Domain/RefactorThis.Domain.csproj @@ -1,59 +1,61 @@  - - - Debug - AnyCPU - {5310B2FE-E26D-414E-B656-1F74C5A70368} - Library - Properties - RefactorThis.Domain - RefactorThis.Domain - v4.7.2 - 512 - - - AnyCPU - true - full - false - bin\Debug\ - DEBUG;TRACE - prompt - 4 - - - AnyCPU - pdbonly - true - bin\Release\ - TRACE - prompt - 4 - - - - - - - - - - - - - - {33cdc796-ff75-449c-9637-59c2efc46361} - RefactorThis.Persistence - - - - - - + \ No newline at end of file diff --git a/RefactorThis.Domain/enums/InvoicePaymentStatus.cs b/RefactorThis.Domain/enums/InvoicePaymentStatus.cs new file mode 100644 index 0000000..f834829 --- /dev/null +++ b/RefactorThis.Domain/enums/InvoicePaymentStatus.cs @@ -0,0 +1,8 @@ +namespace RefactorThis.Domain.enums +{ + public enum InvoicePaymentStatus + { + PartiallyPaid, + Paid + } +} diff --git a/RefactorThis.Domain/messaging/ProcessPaymentErrorMessages.cs b/RefactorThis.Domain/messaging/ProcessPaymentErrorMessages.cs new file mode 100644 index 0000000..f287aa3 --- /dev/null +++ b/RefactorThis.Domain/messaging/ProcessPaymentErrorMessages.cs @@ -0,0 +1,12 @@ + +namespace RefactorThis.Domain.messaging +{ + public class ProcessPaymentErrorMessages + { + public const string NoPaymentNecessary = "This invoice has no amount, so no payment is needed."; + public const string InvalidInvoice_NoAmountButHasPayments = "The invoice is in an invalid state, it has an amount of 0 and it has payments."; + public const string InvoiceAlreadyPaid = "The invoice was already fully paid"; + public const string PaymentTooGreat = "The payment is greater than the partial amount remaining"; + public const string InvoiceWithPaymentAlreadyExists = "An invoice with this payment already exists"; + } +} diff --git a/RefactorThis.Domain/services/InvoiceService.cs b/RefactorThis.Domain/services/InvoiceService.cs new file mode 100644 index 0000000..ade5e50 --- /dev/null +++ b/RefactorThis.Domain/services/InvoiceService.cs @@ -0,0 +1,71 @@ +using System; +using RefactorThis.Domain.enums; +using RefactorThis.Domain.messaging; +using RefactorThis.Persistence.models; +using RefactorThis.Persistence.repositories; + +namespace RefactorThis.Domain +{ + public class InvoiceService + { + private readonly IInvoiceRepository _invoiceRepository; + + public InvoiceService(IInvoiceRepository invoiceRepository) + { + _invoiceRepository = invoiceRepository; + } + + // Just having the Payment here doesn't make sense to me. There seems to be an assumption that the payment would already + // be associated with the Invoice, in which case, adding it again makes no sense. It makes more sense to provide the invoiceId + // which this payment must be associated with. This is something I would normally check with product, but here, I am assuming it's + // a bad code design decision, and fixing it accordingly. + // The general assumption of what this method must do is it must add a payment to an invoice, ensuring that the payment is valid + // It then returns the invoice status - is it partially paid, or fully paid. + public InvoicePaymentStatus ProcessPayment(Payment payment, Guid invoiceId) + { + var invoice = _invoiceRepository.GetInvoice(invoiceId); + + ValidateInvoicePayment(payment, invoice); + + AddPaymentToInvoice(payment, invoice); + + return (invoice.Amount == invoice.AmountPaid) ? InvoicePaymentStatus.Paid : InvoicePaymentStatus.PartiallyPaid; + } + + // In the validation, rather than returning status messages for invalid Payment processing options, throw an exception instead. + // In general, don't rely on statuses to check whether an operation succeeded or not. + private void ValidateInvoicePayment(Payment payment, Invoice invoice) + { + if (invoice.Amount == 0) + { + if (invoice.HasPayments) + { + throw new InvalidOperationException(ProcessPaymentErrorMessages.InvalidInvoice_NoAmountButHasPayments); + } + + throw new InvalidOperationException(ProcessPaymentErrorMessages.NoPaymentNecessary); + } + + if (invoice.AmountPaid == invoice.Amount) + { + throw new InvalidOperationException(ProcessPaymentErrorMessages.InvoiceAlreadyPaid); + } + + if (invoice.AmountPaid + payment.Amount > invoice.Amount) + { + throw new InvalidOperationException(ProcessPaymentErrorMessages.PaymentTooGreat); + } + + if (_invoiceRepository.FindInvoiceWithPayment(payment.Reference) != null) + { + throw new InvalidOperationException(ProcessPaymentErrorMessages.InvoiceWithPaymentAlreadyExists); + } + } + + private void AddPaymentToInvoice(Payment payment, Invoice invoice) + { + invoice.Payments.Add(payment); + _invoiceRepository.SaveInvoice(invoice); + } + } +} \ No newline at end of file diff --git a/RefactorThis.Persistence/Invoice.cs b/RefactorThis.Persistence/Invoice.cs deleted file mode 100644 index bd4370d..0000000 --- a/RefactorThis.Persistence/Invoice.cs +++ /dev/null @@ -1,31 +0,0 @@ -using System.Collections.Generic; - -namespace RefactorThis.Persistence -{ - public class Invoice - { - private readonly InvoiceRepository _repository; - public Invoice( InvoiceRepository repository ) - { - _repository = repository; - } - - public void Save( ) - { - _repository.SaveInvoice( this ); - } - - public decimal Amount { get; set; } - public decimal AmountPaid { get; set; } - public decimal TaxAmount { get; set; } - public List Payments { get; set; } - - public InvoiceType Type { get; set; } - } - - public enum InvoiceType - { - Standard, - Commercial - } -} \ No newline at end of file diff --git a/RefactorThis.Persistence/InvoiceRepository.cs b/RefactorThis.Persistence/InvoiceRepository.cs deleted file mode 100644 index 38548c7..0000000 --- a/RefactorThis.Persistence/InvoiceRepository.cs +++ /dev/null @@ -1,21 +0,0 @@ -namespace RefactorThis.Persistence { - public class InvoiceRepository - { - private Invoice _invoice; - - public Invoice GetInvoice( string reference ) - { - return _invoice; - } - - public void SaveInvoice( Invoice invoice ) - { - //saves the invoice to the database - } - - public void Add( Invoice invoice ) - { - _invoice = invoice; - } - } -} \ No newline at end of file diff --git a/RefactorThis.Persistence/Payment.cs b/RefactorThis.Persistence/Payment.cs deleted file mode 100644 index db29372..0000000 --- a/RefactorThis.Persistence/Payment.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace RefactorThis.Persistence -{ - public class Payment - { - public decimal Amount { get; set; } - public string Reference { get; set; } - } -} \ No newline at end of file diff --git a/RefactorThis.Persistence/messaging/InvoiceRepositoryMessages.cs b/RefactorThis.Persistence/messaging/InvoiceRepositoryMessages.cs new file mode 100644 index 0000000..1a537b8 --- /dev/null +++ b/RefactorThis.Persistence/messaging/InvoiceRepositoryMessages.cs @@ -0,0 +1,7 @@ +namespace RefactorThis.Persistence.messaging +{ + public static class InvoiceRepositoryMessages + { + public const string NoInvoiceFound = "There is no invoice matching this id."; + } +} diff --git a/RefactorThis.Persistence/models/Invoice.cs b/RefactorThis.Persistence/models/Invoice.cs new file mode 100644 index 0000000..37dfbd1 --- /dev/null +++ b/RefactorThis.Persistence/models/Invoice.cs @@ -0,0 +1,24 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace RefactorThis.Persistence.models +{ + public class Invoice + { + public Guid Id { get; set; } + public decimal Amount { get; set; } + public decimal AmountPaid => Payments != null ? Payments.Sum(p => p.Amount) : 0; + public decimal TaxAmount => Type == InvoiceType.Commercial && Payments != null ? Payments.Sum(p => p.Amount * 0.14m) : 0; + public List Payments { get; set; } + public bool HasPayments => Payments != null && Payments.Any(); + + public InvoiceType Type { get; set; } + } + + public enum InvoiceType + { + Standard, + Commercial + } +} \ No newline at end of file diff --git a/RefactorThis.Persistence/models/Payment.cs b/RefactorThis.Persistence/models/Payment.cs new file mode 100644 index 0000000..5d36506 --- /dev/null +++ b/RefactorThis.Persistence/models/Payment.cs @@ -0,0 +1,8 @@ +namespace RefactorThis.Persistence.models +{ + public class Payment + { + public decimal Amount { get; set; } + public string Reference { get; set; } + } +} \ No newline at end of file diff --git a/RefactorThis.Persistence/repositories/IInvoiceRepository.cs b/RefactorThis.Persistence/repositories/IInvoiceRepository.cs new file mode 100644 index 0000000..71a9d08 --- /dev/null +++ b/RefactorThis.Persistence/repositories/IInvoiceRepository.cs @@ -0,0 +1,15 @@ +using System; +using RefactorThis.Persistence.models; + +namespace RefactorThis.Persistence.repositories +{ + public interface IInvoiceRepository + { + // I have made this use an id rather than check the payment reference because it seems to make more sense. + Invoice GetInvoice(Guid id); + // This function assumes that paymentReference is a unique value. This is not guaranteed though, and represents a risk. + // If we want to be able to check this, the reference must either be unique, or we need to update this to use a unique id instead. + Invoice FindInvoiceWithPayment(string paymentReference); + void SaveInvoice(Invoice invoice); + } +} diff --git a/RefactorThis.Persistence/repositories/InvoiceRepository.cs b/RefactorThis.Persistence/repositories/InvoiceRepository.cs new file mode 100644 index 0000000..545a5ed --- /dev/null +++ b/RefactorThis.Persistence/repositories/InvoiceRepository.cs @@ -0,0 +1,28 @@ +using System; +using RefactorThis.Persistence.models; + +namespace RefactorThis.Persistence.repositories +{ + // This is very much a placeholder class. It needs to be implemented correctly for the repository we end up using. It is currently ignored in all tests. + public class InvoiceRepository : IInvoiceRepository + { + private Invoice _invoice; + + public Invoice GetInvoice(Guid id) + { + return _invoice; + } + + public Invoice FindInvoiceWithPayment(string paymentReference) + { + return _invoice; + } + + public void SaveInvoice(Invoice invoice) + { + //saves the invoice to the database + } + + // I have removed Add because it isn't necessary. If we want to Add an invoice, we can just Save a new invoice. + } +} \ No newline at end of file