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

Package Actions Ignore Custom Models Defined in ModelManifest #2031

Open
RRosalia opened this issue Dec 5, 2024 · 1 comment
Open

Package Actions Ignore Custom Models Defined in ModelManifest #2031

RRosalia opened this issue Dec 5, 2024 · 1 comment
Labels
bug Something isn't working unconfirmed

Comments

@RRosalia
Copy link

RRosalia commented Dec 5, 2024

  • Lunar version: 1.0.0-beta.6
  • Laravel Version: 11.34.2
  • PHP Version: 8.3.13
  • Database Driver & Version: MySQL 9.1.0 PDO

Expected Behaviour:

When using custom models to overwrite the framework models via ModelManifest::addDirectory(dir: app_path('Domain/Models'));, all internal references to models within the framework (e.g., Lunar\Actions\*) should respect and use the custom models instead of the hardcoded framework models. This allows seamless integration of custom logic without requiring additional overwrites in action classes.

Actual Behaviour:

Inside the vendor/lunarphp/core/src/Actions/* directory, several actions are hardcoding references to the framework's models instead of leveraging the custom models. For example, in Lunar\Actions\Carts\CreateOrder, the Order model is hardcoded with new Order, bypassing the custom model provided in ModelManifest. This makes it necessary to overwrite the entire class or configure it in config/lunar/cart.php, even when the custom logic differs minimally from the original implementation.

Steps To Reproduce:

  1. Add custom models to overwrite Lunar's default models using ModelManifest::addDirectory(dir: app_path('Domain/Models'));.
  2. Attempt to use the framework actions, such as Lunar\Actions\Carts\CreateOrder, that rely on the hardcoded models (e.g., new Order).
  3. Observe that the custom model logic is not applied due to the package's hardcoded references.
  4. Attempt to customize the functionality without overwriting or explicitly configuring the action in config/lunar/cart.php.

Code Example:

Here's an example of the issue:

<?php

namespace Lunar\Actions\Carts;

use Illuminate\Pipeline\Pipeline;
use Lunar\Actions\AbstractAction;
use Lunar\Exceptions\DisallowMultipleCartOrdersException;
use Lunar\Facades\DB;
use Lunar\Jobs\Orders\MarkAsNewCustomer;
use Lunar\Models\Cart;
use Lunar\Models\Order;

final class CreateOrder extends AbstractAction
{
    public function execute(
        Cart $cart,
        bool $allowMultipleOrders = false,
        ?int $orderIdToUpdate = null
    ): self {
        $this->passThrough = DB::transaction(function () use ($cart, $allowMultipleOrders, $orderIdToUpdate) {
            $order = $cart->currentDraftOrder($orderIdToUpdate) ?: new Order; // Hardcoded Order model

            if ($cart->hasCompletedOrders() && !$allowMultipleOrders) {
                throw new DisallowMultipleCartOrdersException;
            }

            $order->fill([
                'cart_id' => $cart->id,
                'fingerprint' => $cart->fingerprint(),
            ]);

            $order = app(Pipeline::class)
                ->send($order)
                ->through(
                    config('lunar.orders.pipelines.creation', [])
                )->thenReturn(function ($order) {
                    return $order;
                });

            $cart->discounts?->each(function ($discount) use ($cart) {
                $discount->markAsUsed($cart)->discount->save();
            });

            $cart->save();

            MarkAsNewCustomer::dispatch($order->id);

            $order->refresh();

            return $order;
        });

        return $this;
    }
}

Suggested Solution:

Modify the package's action classes to dynamically resolve models via the ModelManifest or similar mechanisms instead of hardcoding models directly. For example, replace new Order with resolve(ModelManifest::class)->get(Order::class) or a similar approach. This would ensure the custom models are used wherever applicable.

$order = $cart->currentDraftOrder($orderIdToUpdate) ?: app(ModelManifest::class)->get(Order::class);

This would maintain compatibility with custom logic while preserving the default package behavior.

@RRosalia RRosalia added bug Something isn't working unconfirmed labels Dec 5, 2024
@shepard153
Copy link
Contributor

shepard153 commented Dec 24, 2024

I think there is a problem with that addDirectory method. Try something like this. It works in my case:

<?php

namespace App\Providers;

use Illuminate\Support\ServiceProvider;
use Lunar\Facades\ModelManifest;
use Lunar\Models\Contracts\Product;

class LunarServiceProvider extends ServiceProvider
{
    public function boot(): void
    {
        $this->registerLunarModels();
    }

    private function registerLunarModels(): void
    {
        ModelManifest::replace(Product::class, \App\Models\Product::class);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unconfirmed
Projects
None yet
Development

No branches or pull requests

2 participants