diff --git a/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs b/RefactorThis.Domain.Tests/InvoicePaymentProcessorTests.cs index 3a607fd..428755f 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( ); @@ -32,7 +32,7 @@ public void ProcessPayment_Should_ThrowException_When_NoInoiceFoundForPaymentRef } [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_NoPaymentNeeded( ) + public void ProcessPayment_Should_ReturnFailureMessage_When_NoPaymentNeeded() { var repo = new InvoiceRepository( ); @@ -55,7 +55,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_NoPaymentNeeded( ) } [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_InvoiceAlreadyFullyPaid( ) + public void ProcessPayment_Should_ReturnFailureMessage_When_InvoiceAlreadyFullyPaid() { var repo = new InvoiceRepository( ); @@ -83,7 +83,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_InvoiceAlreadyFullyP } [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_PartialPaymentExistsAndAmountPaidExceedsAmountDue( ) + public void ProcessPayment_Should_ReturnFailureMessage_When_PartialPaymentExistsAndAmountPaidExceedsAmountDue() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -113,7 +113,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_PartialPaymentExists } [Test] - public void ProcessPayment_Should_ReturnFailureMessage_When_NoPartialPaymentExistsAndAmountPaidExceedsInvoiceAmount( ) + public void ProcessPayment_Should_ReturnFailureMessage_When_NoPartialPaymentExistsAndAmountPaidExceedsInvoiceAmount() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -137,7 +137,7 @@ public void ProcessPayment_Should_ReturnFailureMessage_When_NoPartialPaymentExis } [Test] - public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExistsAndAmountPaidEqualsAmountDue( ) + public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExistsAndAmountPaidEqualsAmountDue() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -167,7 +167,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_PartialPaymentExis } [Test] - public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidEqualsInvoiceAmount( ) + public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidEqualsInvoiceAmount() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -191,7 +191,7 @@ public void ProcessPayment_Should_ReturnFullyPaidMessage_When_NoPartialPaymentEx } [Test] - public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPaymentExistsAndAmountPaidIsLessThanAmountDue( ) + public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPaymentExistsAndAmountPaidIsLessThanAmountDue() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -221,7 +221,7 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_PartialPayment } [Test] - public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidIsLessThanInvoiceAmount( ) + public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPaymentExistsAndAmountPaidIsLessThanInvoiceAmount() { var repo = new InvoiceRepository( ); var invoice = new Invoice( repo ) @@ -243,5 +243,75 @@ public void ProcessPayment_Should_ReturnPartiallyPaidMessage_When_NoPartialPayme Assert.AreEqual( "invoice is now partially paid", result ); } - } + + [Test] + public void ProcessPayment_Should_TransitionToFullyPaid_When_MultiplePartialPaymentsAddUpToInvoiceAmount() + { + 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 firstPayment = new Payment { Amount = 4 }; + var firstResult = paymentProcessor.ProcessPayment(firstPayment); + Assert.AreEqual("invoice is now partially paid", firstResult); + + var secondPayment = new Payment { Amount = 6 }; + var secondResult = paymentProcessor.ProcessPayment(secondPayment); + Assert.AreEqual("final partial payment received, invoice is now fully paid", secondResult); + } + + [Test] + public void ProcessPayment_Should_AddPaymentToInvoice_When_Successful() + { + 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 = 5 }; + + var result = paymentProcessor.ProcessPayment(payment); + + Assert.AreEqual("invoice is now partially paid", result); + Assert.AreEqual(5, invoice.AmountPaid); + Assert.AreEqual(1, invoice.Payments.Count); + Assert.AreEqual(5, invoice.Payments[0].Amount); + } + + [Test] + public void ProcessPayment_Should_HandleNullPaymentsList() + { + var repo = new InvoiceRepository(); + var invoice = new Invoice(repo) + { + Amount = 10, + AmountPaid = 0, + Payments = null // deliberately null + }; + repo.Add(invoice); + + var paymentProcessor = new InvoiceService(repo); + + var payment = new Payment { Amount = 3 }; + var result = paymentProcessor.ProcessPayment(payment); + + Assert.AreEqual("invoice is now partially paid", result); + Assert.AreEqual(3, invoice.AmountPaid); + Assert.IsNotNull(invoice.Payments); + Assert.AreEqual(1, invoice.Payments.Count); + } + } } \ 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..ae7a8f4 100644 --- a/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj +++ b/RefactorThis.Domain.Tests/RefactorThis.Domain.Tests.csproj @@ -1,67 +1,165 @@  - - - 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/packages.config b/RefactorThis.Domain.Tests/packages.config index c108d44..4ef16f8 100644 --- a/RefactorThis.Domain.Tests/packages.config +++ b/RefactorThis.Domain.Tests/packages.config @@ -1,4 +1,22 @@  - + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/RefactorThis.Domain/CommercialInvoiceStrategy.cs b/RefactorThis.Domain/CommercialInvoiceStrategy.cs new file mode 100644 index 0000000..4c7e064 --- /dev/null +++ b/RefactorThis.Domain/CommercialInvoiceStrategy.cs @@ -0,0 +1,31 @@ +using RefactorThis.Persistence; +using System; + +namespace RefactorThis.Domain +{ + public class CommercialInvoiceStrategy : IInvoiceStrategy + { + private const decimal TaxRate = 0.14m; + + public void ApplyPayment(Invoice invoice, Payment payment, bool hadPreviousPayments, bool isFinalPayment) + { + if (invoice == null) throw new ArgumentNullException(nameof(invoice)); + if (payment == null) throw new ArgumentNullException(nameof(payment)); + + if (hadPreviousPayments) + { + // for Commercial, add tax for every applied payment when there are prior payments + invoice.AmountPaid += payment.Amount; + invoice.TaxAmount += payment.Amount * TaxRate; + invoice.Payments.Add(payment); + } + else + { + // for no previous payments, same as standard for initial tax assignment (original behaviour) + invoice.AmountPaid = payment.Amount; + invoice.TaxAmount = payment.Amount * TaxRate; + invoice.Payments.Add(payment); + } + } + } +} \ No newline at end of file diff --git a/RefactorThis.Domain/IInvoiceStrategy.cs b/RefactorThis.Domain/IInvoiceStrategy.cs new file mode 100644 index 0000000..059c455 --- /dev/null +++ b/RefactorThis.Domain/IInvoiceStrategy.cs @@ -0,0 +1,15 @@ +using RefactorThis.Persistence; +using System; + +namespace RefactorThis.Domain +{ + public interface IInvoiceStrategy + { + /// + /// Apply a payment to the invoice. + /// hadPreviousPayments: true if invoice already had payments. + /// isFinalPayment: true if this payment will make the invoice fully paid. + /// + void ApplyPayment(Invoice invoice, Payment payment, bool hadPreviousPayments, bool isFinalPayment); + } +} \ No newline at end of file diff --git a/RefactorThis.Domain/InvoiceService.cs b/RefactorThis.Domain/InvoiceService.cs index fbd674c..37392f6 100644 --- a/RefactorThis.Domain/InvoiceService.cs +++ b/RefactorThis.Domain/InvoiceService.cs @@ -1,149 +1,106 @@ using System; using System.Linq; +using System.Collections.Generic; 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; - } - } + public class InvoiceService + { + private readonly InvoiceRepository _invoiceRepository; + + public InvoiceService(InvoiceRepository invoiceRepository) + { + _invoiceRepository = invoiceRepository; + } + + public string ProcessPayment(Payment payment) + { + if (payment == null) + throw new ArgumentNullException(nameof(payment)); + + var inv = _invoiceRepository.GetInvoice(payment.Reference); + + if (inv == null) + { + throw new InvalidOperationException("There is no invoice matching this payment"); + } + + // Ensure Payments list exists to avoid null-ref when we add a payment + if (inv.Payments == null) + { + inv.Payments = new List(); + } + + // Case: invoice amount is zero + if (inv.Amount == 0) + { + if (inv.Payments == null || !inv.Payments.Any()) + { + return "no payment needed"; + } + + throw new InvalidOperationException("The invoice is in an invalid state, it has an amount of 0 and it has payments."); + } + + // Compute current totals + var paymentsExist = inv.Payments != null && inv.Payments.Any(); + var totalPaid = inv.Payments?.Sum(x => x.Amount) ?? 0m; + + // If there are payments and totalPaid equals invoice amount => already fully paid + if (paymentsExist && totalPaid != 0m && inv.Amount == totalPaid) + { + return "invoice was already fully paid"; + } + + // Remaining amount uses inv.AmountPaid as original code uses inv.AmountPaid in that calculation + var remaining = inv.Amount - inv.AmountPaid; + + // If payments exist and this payment is bigger than the remaining partial amount + if (paymentsExist && totalPaid != 0m && payment.Amount > remaining) + { + return "the payment is greater than the partial amount remaining"; + } + + // Choose strategy based on invoice type + var strategy = InvoiceStrategyFactory.GetStrategy(inv.Type); + + // Two main flows: invoice already had payments OR this is the very first payment + if (paymentsExist) + { + // If this payment equals the remaining amount, it's the final partial payment + var isFinal = remaining == payment.Amount; + + strategy.ApplyPayment(inv, payment, hadPreviousPayments: true, isFinalPayment: isFinal); + + // Persist only when we've modified the invoice + inv.Save(); + + return isFinal + ? "final partial payment received, invoice is now fully paid" + : "another partial payment received, still not fully paid"; + } + else + { + // No existing payments on this invoice + if (payment.Amount > inv.Amount) + { + return "the payment is greater than the invoice amount"; + } + + if (inv.Amount == payment.Amount) + { + strategy.ApplyPayment(inv, payment, hadPreviousPayments: false, isFinalPayment: true); + inv.Save(); + return "invoice is now fully paid"; + } + else + { + strategy.ApplyPayment(inv, payment, hadPreviousPayments: false, isFinalPayment: false); + inv.Save(); + return "invoice is now partially paid"; + } + } + } + } } \ No newline at end of file diff --git a/RefactorThis.Domain/InvoiceStrategyFactory.cs b/RefactorThis.Domain/InvoiceStrategyFactory.cs new file mode 100644 index 0000000..8395500 --- /dev/null +++ b/RefactorThis.Domain/InvoiceStrategyFactory.cs @@ -0,0 +1,19 @@ +using RefactorThis.Persistence; +using System; + +namespace RefactorThis.Domain +{ + public static class InvoiceStrategyFactory + { + public static IInvoiceStrategy GetStrategy(InvoiceType type) + { + switch (type) + { + case InvoiceType.Standard: return new StandardInvoiceStrategy(); + case InvoiceType.Commercial: return new CommercialInvoiceStrategy(); + default: + throw new ArgumentOutOfRangeException(nameof(type), "Unsupported invoice type"); + } + } + } +} \ No newline at end of file diff --git a/RefactorThis.Domain/RefactorThis.Domain.csproj b/RefactorThis.Domain/RefactorThis.Domain.csproj index 753e893..12bc5b0 100644 --- a/RefactorThis.Domain/RefactorThis.Domain.csproj +++ b/RefactorThis.Domain/RefactorThis.Domain.csproj @@ -1,59 +1,63 @@  - - - 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/StandardInvoiceStrategy.cs b/RefactorThis.Domain/StandardInvoiceStrategy.cs new file mode 100644 index 0000000..6c1d090 --- /dev/null +++ b/RefactorThis.Domain/StandardInvoiceStrategy.cs @@ -0,0 +1,30 @@ +using RefactorThis.Persistence; +using System; + +namespace RefactorThis.Domain +{ + public class StandardInvoiceStrategy : IInvoiceStrategy + { + private const decimal TaxRate = 0.14m; + + public void ApplyPayment(Invoice invoice, Payment payment, bool hadPreviousPayments, bool isFinalPayment) + { + if (invoice == null) throw new ArgumentNullException(nameof(invoice)); + if (payment == null) throw new ArgumentNullException(nameof(payment)); + + if (hadPreviousPayments) + { + // original behaviour: when there are existing payments, Standard does NOT modify TaxAmount + invoice.AmountPaid += payment.Amount; + invoice.Payments.Add(payment); + } + else + { + // original behaviour (no previous payments): Standard DID set TaxAmount = payment * 0.14m + invoice.AmountPaid = payment.Amount; + invoice.TaxAmount = payment.Amount * TaxRate; + invoice.Payments.Add(payment); + } + } + } +} \ No newline at end of file