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

Using Select within BelongsToMany fields() #6512

Closed
Norgul opened this issue Aug 28, 2024 · 7 comments
Closed

Using Select within BelongsToMany fields() #6512

Norgul opened this issue Aug 28, 2024 · 7 comments
Assignees

Comments

@Norgul
Copy link

Norgul commented Aug 28, 2024

  • Laravel Version: v11.15.0
  • Nova Version: v4.34.3
  • PHP Version: 8.2.20
  • Database Driver & Version:
  • Operating System and Version: MacOS Sonoma 14.5 (23F79)
  • Browser type and version: Chrome Version 127.0.6533.90 (Official Build) (arm64)

Description:

Not sure if this is a bug or I implemented something wrong. I have a pivot table with additional FK (unit_id). If I use Select in fields() method pointing to that additional FK, saving the relation will fail even though unit_id is present in the payload in the failing request.

// migration
Schema::create('recipe_composition', function (Blueprint $table) {
    $table->id();
    $table->foreignId('parent_id')->constrained('products');
    $table->foreignId('child_id')->constrained('products');

    $table->foreignId('unit_id')->constrained();
    ...
});

// Product.php
public function children(): BelongsToMany
{
    return $this->belongsToMany(self::class, 'recipe_composition', 'parent_id', 'child_id')
        ->withPivot(RecipeComposition::WITH_PIVOT)
        ->using(RecipeComposition::class)
        ->with('children')
        ->withTimestamps();
}

// RecipeComposition.php pivot model
class RecipeComposition extends Pivot
{
    public const WITH_PIVOT = [
        'unit_id',
        'net_amount',
        'gross_amount',
        'note',
        'removable_on_sale',
    ];

    public function parent(): BelongsTo
    {
        return $this->belongsTo(Product::class, 'parent_id');
    }

    public function child(): BelongsTo
    {
        return $this->belongsTo(Product::class, 'child_id');
    }

    public function unit(): BelongsTo
    {
        return $this->belongsTo(Unit::class);
    }
}

// Nova relation
BelongsToMany::make('Recipe elements', 'children', Product::class)
    ->fields(fn() => [
        Select::make('Unit', 'unit_id')
            ->dependsOn('children', function (Select $field, NovaRequest $request, FormData $formData) {
                $productId = $formData->get('children');

                /** @var \App\Models\Tenant\Product $product */
                $product = \App\Models\Tenant\Product::query()->with('unit')->find($productId);

                if (!$product) {
                    $field->readonly();
                    return;
                }

                $field->options(fn() => collect()
                    ->merge(Collection::wrap($product->unit->ancestors))
                    ->merge(Collection::wrap($product->unit))
                    ->merge(Collection::wrap($product->unit->grandchildren))
                    ->pluck('label', 'id')
                );

                $field->value = $product->unit->id;
            }),
        ...
    ])
    ->singularLabel('Recipe or Goods'),

image

If however I replace Select with a hardcoded Text or Number field, it will have the same payload, but will go through.

@Norgul
Copy link
Author

Norgul commented Aug 28, 2024

I found what is the issue (though this still might be a bug).

In dependsOn() this part is causing the issue:

if (!$product) {
    $field->readonly();
    return;
}

Even if I later set it as $field->readonly(false) it doesn't work. To solve it, I needed to remove that part so that I have this:

if (!$product) {
    return;
}

Is this normal behavior?

@Norgul Norgul changed the title Using select within BelongsToMany field Using Select within BelongsToMany fields() Aug 28, 2024
@jeremynikolic
Copy link

Hi @Norgul 👋
Thanks for reaching out to us.
What you describe here is expected behavior, inputs for fields marked as readonly are discarded in the backend for safety reasons.
I am unsure what you are trying to achieve here, if you need some more directions in settings things up please describe your goal and I'll do my best 👌

@jeremynikolic jeremynikolic self-assigned this Sep 2, 2024
@jeremynikolic jeremynikolic added the needs more info More information is required label Sep 2, 2024
@Norgul
Copy link
Author

Norgul commented Sep 2, 2024

@jeremynikolic thanks for the answer.

I know readonly fields are discarded. What I think is a bug there though is the fact that even after I canceled readonly status for a field with $field->readonly(false), it would still treat it as such.

@jeremynikolic
Copy link

Ah got you ! My bad, I missed that extra disabling readonly step 👍
I believe you are right, updating the readonly after initial setup may not be picked up.

I'll look into it 🕵️

@jeremynikolic jeremynikolic added pending Issues that are pending triage and removed needs more info More information is required labels Sep 2, 2024
@jeremynikolic jeremynikolic added this to the 4.x milestone Sep 6, 2024
@jeremynikolic
Copy link

@Norgul I've been investigating this and the readonly change is actually well received and accounted for on the frontend 🤔

Something to check on your side, the screenshot you provided indicates your unit_id is not nullable therefore the form failing when not having value for it therefore your schema most likely needs adjusting

Looking at the code samples I am wondering if the models/resources setup may be the cause here. To go further, please provide a sample reproducing repository 🙏

@Norgul
Copy link
Author

Norgul commented Sep 11, 2024

Not sure really. As you can see from payload submitted to the API, unit_id does get forwarded. So I assume once the request is submitted and gets to the controller, Nova discards the field. I will see if I catch anything else. Thanks

@jeremynikolic jeremynikolic removed this from the 4.x milestone Sep 11, 2024
@jeremynikolic jeremynikolic added needs more info More information is required and removed pending Issues that are pending triage labels Sep 16, 2024
@crynobone
Copy link
Member

I believe this is now solved with Immutable Field: https://nova.laravel.com/docs/v5/resources/fields#immutable-fields

@crynobone crynobone removed needs more info More information is required stale labels Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants