Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unused potential of OrderItem::$relatedItem #2677

Open
vitek-rostislav opened this issue Jul 25, 2023 · 0 comments
Open

Unused potential of OrderItem::$relatedItem #2677

vitek-rostislav opened this issue Jul 25, 2023 · 0 comments

Comments

@vitek-rostislav
Copy link
Contributor

Describe the bug

We have $relatedItem property in OrderItem class (https://github.com/shopsys/shopsys/blob/13.0/project-base/app/src/Model/Order/Item/OrderItem.php#L50) but it is basically not used. It is just set when creating a new order item of type coupon (which is actually an order item of type OrderItem::TYPE_PRODUCT at the moment while it deserves its own type, but we all know that 🙂). The getters are then never used if I am not mistaken.

I have two suggestions for how to improve the current implementation:

  1. $relatedItem should be rather implemented as one-to-many relation (there might be more related items eg. when selecting a collection of services for a product). Something like this is better for a wider scale of use-cases IMHO:
    /**
     * @var \App\Model\Order\Item\OrderItem|null
     * @ORM\ManyToOne(targetEntity="App\Model\Order\Item\OrderItem", inversedBy="relatedOrderItems")
     * @ORM\JoinColumn(nullable=true)
     */
    private ?OrderItem $parentOrderItem;

    /**
     * @var \Doctrine\Common\Collections\Collection<int, \App\Model\Order\Item\OrderItem>
     * @ORM\OneToMany(targetEntity="App\Model\Order\Item\OrderItem", mappedBy="parentOrderItem")
     */
    private Collection $relatedOrderItems;
  1. We should really use the relation when we approach the related items (on the storefront, in the mail, etc.) - at the moment, we just iterate through the items and rely on the fact the related items are created right after their "parent" item - ie. we rely on the following: parentItem->getId() + 1 === $relatedItem->getId(). If we messed up the ids, the output would be broken. Instead, I suggest iterating just the PRODUCT_TYPE items and under them, iterating through their relatedItems to display the related coupons, services, or whatever the related items might be on the particular implementation.
  2. BONUS - as mentioned above, there should be a separate type for the OrderItem that represents the discount/coupon. Sad enough, there are two other issues (PromoCodes and OrderRounding are OrderItems with type product #1390 and Promo code is custom instance of OrderItem #1263) and one pull request (Order item has new type discount #1563) for this ☹️
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants