diff --git a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs index 3a607fd..c070954 100644 --- a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs +++ b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs @@ -9,7 +9,7 @@ namespace RefactorThis.Domain.Tests public class InvoicePaymentProcessorTests { [Test] - public void ProcessPayment_Should_ThrowException_When_NoInoiceFoundForPaymentReference( ) + public void ProcessPayment_Should_ThrowException_When_NoInvoiceFoundForPaymentReference( ) { var repo = new InvoiceRepository( ); @@ -142,6 +142,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) { + Type = InvoiceType.Standard, Amount = 10, AmountPaid = 5, Payments = new List @@ -164,6 +165,44 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis var result = paymentProcessor.ProcessPayment( payment ); Assert.AreEqual( "final partial payment received, invoice is now fully paid", result ); + Assert.AreEqual( 10, invoice.AmountPaid ); + Assert.AreEqual( 0m, invoice.TaxAmount ); + Assert.AreEqual( 2, invoice.Payments.Count ); + } + + [Test] + public void ProcessPayment_Should_ApplyTaxToCommercialInvoice_When_PartialPaymentExistsAndAmountPaidEqualsAmountDue( ) + { + var repo = new InvoiceRepository( ); + var invoice = new Invoice( repo ) + { + Type = InvoiceType.Commercial, + Amount = 10, + AmountPaid = 5, + TaxAmount = 0.7m, + 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 ); + Assert.AreEqual( 10, invoice.AmountPaid ); + Assert.AreEqual( 1.4m, invoice.TaxAmount ); + Assert.AreEqual( 2, invoice.Payments.Count ); } [Test] @@ -174,7 +213,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx { Amount = 10, AmountPaid = 0, - Payments = new List( ) { new Payment( ) { Amount = 10 } } + Payments = new List( ) }; repo.Add( invoice ); @@ -187,7 +226,10 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx var result = paymentProcessor.ProcessPayment( payment ); - Assert.AreEqual( "invoice was already fully paid", result ); + Assert.AreEqual( "invoice is now fully paid", result ); + Assert.AreEqual( 10, invoice.AmountPaid ); + Assert.AreEqual( 1.4m, invoice.TaxAmount ); + Assert.AreEqual( 1, invoice.Payments.Count ); } [Test] @@ -196,6 +238,7 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) { + Type = InvoiceType.Commercial, Amount = 10, AmountPaid = 5, Payments = new List @@ -218,6 +261,9 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment var result = paymentProcessor.ProcessPayment( payment ); Assert.AreEqual( "another partial payment received, still not fully paid", result ); + Assert.AreEqual( 6, invoice.AmountPaid ); + Assert.AreEqual( 0.14m, invoice.TaxAmount ); + Assert.AreEqual( 2, invoice.Payments.Count ); } [Test] @@ -242,6 +288,9 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPayme var result = paymentProcessor.ProcessPayment( payment ); Assert.AreEqual( "invoice is now partially paid", result ); + Assert.AreEqual( 1, invoice.AmountPaid ); + Assert.AreEqual( 0.14m, invoice.TaxAmount ); + Assert.AreEqual( 1, invoice.Payments.Count ); } } } \ No newline at end of file diff --git a/RefactorThis.Domain/InvoiceService.cs b/RefactorThis.Domain/InvoiceService.cs index fbd674c..f79ddb5 100644 --- a/RefactorThis.Domain/InvoiceService.cs +++ b/RefactorThis.Domain/InvoiceService.cs @@ -17,133 +17,84 @@ 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.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.HasPayments( ) ) { - 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( ); - } - } - } + throw new InvalidOperationException( + "The invoice is in an invalid state, it has an amount of 0 and it has payments."); } + + return "no payment needed"; + } + + return ( inv.HasPayments( ) ) + ? ProcessPartialPayment( inv, payment ) + : ProcessFullPayment( inv, payment ); + } + + private static string ProcessPartialPayment( Invoice inv, Payment payment ) + { + if ( inv.Amount == inv.Payments.Sum( x => x.Amount ) ) + { + return "invoice was already fully paid"; + } + + if ( payment.Amount > ( inv.Amount - inv.AmountPaid ) ) + { + return "the payment is greater than the partial amount remaining"; } - - inv.Save(); - return responseMessage; + switch ( inv.Type ) + { + case InvoiceType.Standard: + // TODO: Discuss with stakeholders + // I'm preserving existing behaviour here - but it seems wrong! + // I suspect that either this is supposed to be taxed - or that Standard + // invoices should not be taxed in the ProcessFullPayment() function either + inv.AddPayment( payment, false ); + break; + case InvoiceType.Commercial: + inv.AddPayment( payment ); + break; + default: + throw new ArgumentOutOfRangeException( ); + } + + inv.Save( ); + + return ( inv.AmountPaid == inv.Amount ) + ? "final partial payment received, invoice is now fully paid" + : "another partial payment received, still not fully paid"; + } + + private static string ProcessFullPayment( Invoice inv, Payment payment ) + { + if ( payment.Amount > inv.Amount ) + { + return "the payment is greater than the invoice amount"; + } + + switch ( inv.Type ) + { + case InvoiceType.Standard: + case InvoiceType.Commercial: + inv.AddPayment( payment ); + break; + default: + throw new ArgumentOutOfRangeException( ); + } + + inv.Save( ); + + return ( inv.Amount == payment.Amount ) + ? "invoice is now fully paid" + : "invoice is now partially paid"; } } } \ No newline at end of file diff --git a/RefactorThis.Persistence/Invoice.cs b/RefactorThis.Persistence/Invoice.cs index bd4370d..0a7514d 100644 --- a/RefactorThis.Persistence/Invoice.cs +++ b/RefactorThis.Persistence/Invoice.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; namespace RefactorThis.Persistence { @@ -21,6 +22,20 @@ public void Save( ) public List Payments { get; set; } public InvoiceType Type { get; set; } + + public bool HasPayments( ) + { + return Payments != null && Payments.Any( ); + } + public void AddPayment( Payment payment, bool isTaxable = true ) + { + AmountPaid += payment.Amount; + if (isTaxable) + { + TaxAmount += payment.Amount * 0.14m; + } + Payments.Add( payment ); + } } public enum InvoiceType