Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Remove AddOrderItem method from Order class #1416

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Microsoft.eShopOnContainers.Services.Ordering.API.Application.Commands
using System.Linq;

namespace Microsoft.eShopOnContainers.Services.Ordering.API.Application.Commands
{
using Domain.AggregatesModel.OrderAggregate;
using global::Ordering.API.Application.IntegrationEvents;
Expand Down Expand Up @@ -46,12 +48,19 @@ public async Task<bool> Handle(CreateOrderCommand message, CancellationToken can
// methods and constructor so validations, invariants and business logic
// make sure that consistency is preserved across the whole aggregate
var address = new Address(message.Street, message.City, message.State, message.Country, message.ZipCode);
var order = new Order(message.UserId, message.UserName, address, message.CardTypeId, message.CardNumber, message.CardSecurityNumber, message.CardHolderName, message.CardExpiration);
var orderItems = message.OrderItems.Select(item => new OrderItem(item.ProductId, item.ProductName,
item.UnitPrice, item.Discount, item.PictureUrl, item.Units));

foreach (var item in message.OrderItems)
{
order.AddOrderItem(item.ProductId, item.ProductName, item.UnitPrice, item.Discount, item.PictureUrl, item.Units);
}
var order = new Order(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wish to handle bulk complexity I would rather opt into implementing AddOrderItems instead of CTOR where you lose some flexibility like returning results, async, etc..

Reasons being AddOrderItem can/could change from void to some functional object, validation, error result with some success flag for instance since it should encapsulate some logic which currently is not there but this is still a showcase of the pattern.

message.UserId,
message.UserName,
address,
message.CardTypeId,
message.CardNumber,
message.CardSecurityNumber,
message.CardHolderName,
message.CardExpiration,
orderItems.ToList());

_logger.LogInformation("----- Creating Order - Order: {@Order}", order);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,10 @@ public CreateOrderDraftCommandHandler(IMediator mediator, IIdentityService iden

public Task<OrderDraftDTO> Handle(CreateOrderDraftCommand message, CancellationToken cancellationToken)
{
var orderItems = message.Items.Select(i => i.ToOrderItemDTO())
.Select(item => new OrderItem(item.ProductId, item.ProductName, item.UnitPrice, item.Discount, item.PictureUrl, item.Units));

var order = Order.NewDraft();
var orderItems = message.Items.Select(i => i.ToOrderItemDTO());
foreach (var item in orderItems)
{
order.AddOrderItem(item.ProductId, item.ProductName, item.UnitPrice, item.Discount, item.PictureUrl, item.Units);
}
var order = Order.NewDraft(orderItems.ToList());

return Task.FromResult(OrderDraftDTO.FromOrder(order));
}
Expand All @@ -53,7 +50,7 @@ public static OrderDraftDTO FromOrder(Order order)
{
OrderItems = order.OrderItems.Select(oi => new OrderItemDTO
{
Discount = oi.GetCurrentDiscount(),
Discount = oi.GetDiscount(),
ProductId = oi.ProductId,
UnitPrice = oi.GetUnitPrice(),
PictureUrl = oi.GetPictureUri(),
Expand All @@ -63,10 +60,5 @@ public static OrderDraftDTO FromOrder(Order order)
Total = order.GetTotal()
};
}

}




}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class Order
private string _description;



// Draft orders have this set to true. Currently we don't check anywhere the draft status of an Order, but we could do it if needed
private bool _isDraft;

Expand All @@ -40,21 +39,27 @@ public class Order

private int? _paymentMethodId;

public static Order NewDraft()
public static Order NewDraft(List<OrderItem> orderItems)
{
var order = new Order();
var order = new Order(orderItems);
order._isDraft = true;
return order;
}

protected Order()
protected Order(List<OrderItem> orderItems)
{
_orderItems = new List<OrderItem>();

foreach (var item in orderItems)
{
AddOrderItem(item);
}

_isDraft = false;
}

public Order(string userId, string userName, Address address, int cardTypeId, string cardNumber, string cardSecurityNumber,
string cardHolderName, DateTime cardExpiration, int? buyerId = null, int? paymentMethodId = null) : this()
string cardHolderName, DateTime cardExpiration, List<OrderItem> orderItems, int? buyerId = null, int? paymentMethodId = null) : this(orderItems)
{
_buyerId = buyerId;
_paymentMethodId = paymentMethodId;
Expand All @@ -72,27 +77,27 @@ protected Order()
// This Order AggregateRoot's method "AddOrderitem()" should be the only way to add Items to the Order,
// so any behavior (discounts, etc.) and validations are controlled by the AggregateRoot
// in order to maintain consistency between the whole Aggregate.
public void AddOrderItem(int productId, string productName, decimal unitPrice, decimal discount, string pictureUrl, int units = 1)
private void AddOrderItem(OrderItem orderItem)
{
var existingOrderForProduct = _orderItems.Where(o => o.ProductId == productId)
.SingleOrDefault();
var existingOrderForProduct = _orderItems
.SingleOrDefault(o => o.ProductId == orderItem.ProductId);

if (existingOrderForProduct != null)
{
//if previous line exist modify it with higher discount and units..

if (discount > existingOrderForProduct.GetCurrentDiscount())
var discount = orderItem.GetDiscount();
if (discount > existingOrderForProduct.GetDiscount())
{
existingOrderForProduct.SetNewDiscount(discount);
}

existingOrderForProduct.AddUnits(units);
existingOrderForProduct.AddUnits(orderItem.GetUnits());
}
else
{
//add validated new order item

var orderItem = new OrderItem(productId, productName, unitPrice, discount, pictureUrl, units);
_orderItems.Add(orderItem);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public OrderItem(int productId, string productName, decimal unitPrice, decimal d

public string GetPictureUri() => _pictureUrl;

public decimal GetCurrentDiscount()
public decimal GetDiscount()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename is out of proposed scope of this PR

{
return _discount;
}
Expand Down