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

ProductAccessoryFacade (and repository) are probably unnecessary #2518

Open
vitek-rostislav opened this issue Nov 28, 2022 · 0 comments
Open
Labels
DX & Refactoring Requests for DX improvements and refactorings In Backlog

Comments

@vitek-rostislav
Copy link
Contributor

vitek-rostislav commented Nov 28, 2022

Describe the expected outcome of your requested feature

To access product accessories, we currently use ProductAccessoryFacade (resp. ProductAccessoryRepository). It is not possible to access the accessories directly from a product. In ProductDataFactory, there is the following code:

protected function getAccessoriesData(Product $product)
    {
        $productAccessoriesByPosition = [];
        foreach ($this->productAccessoryRepository->getAllByProduct($product) as $productAccessory) {
            $productAccessoriesByPosition[$productAccessory->getPosition()] = $productAccessory->getAccessory();
        }

        return $productAccessoriesByPosition;
    }

It seems like all of that is unnecessary, as we can do the following changes:

  1. Add relation to Product:
    /**
     * @var \Doctrine\Common\Collections\Collection<int, \Shopsys\FrameworkBundle\Model\Product\Accessory\ProductAccessory>
     * @ORM\OneToMany(targetEntity="Shopsys\FrameworkBundle\Model\Product\Accessory\ProductAccessory", mappedBy="product")
     */
    protected Collection $productAccessories;

   /**
     * @return  \Shopsys\FrameworkBundle\Model\Product\Product[]
     */
    public function getAccessoryProductsIndexedByPosition(): array
    {
        $accessoryProducts = [];
        foreach ($this->productAccessories as $productAccessory) {
            $accessoryProducts[$productAccessory->getPosition()] = $productAccessory->getAccessory();
        }

        return $accessoryProducts;
    }

  1. add inverseBy setting to ProductAccessory::$product:
/**
     * @var \App\Model\Product\Product
-    * @ORM\ManyToOne(targetEntity="Shopsys\FrameworkBundle\Model\Product\Product")
+    * @ORM\ManyToOne(targetEntity="Shopsys\FrameworkBundle\Model\Product\Product", inversedBy="productAccessories")
     * @ORM\JoinColumn(nullable=false, name="product_id", referencedColumnName="id", onDelete="CASCADE")
     * @ORM\Id
     */
    protected $product;
  1. initialize and set the relation in Product::setData() method:
$this->productAccessories = new ArrayCollection($productData->accessories);

Btw in ProductFacade, there is quite a complicated method refreshProductAccessories that is called in create and edit methods. I am not sure, maybe this one can be removed as well

@vitek-rostislav vitek-rostislav added the DX & Refactoring Requests for DX improvements and refactorings label Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX & Refactoring Requests for DX improvements and refactorings In Backlog
Projects
None yet
Development

No branches or pull requests

2 participants