laravel / nova-issues

553 stars 34 forks source link

BelongsToMany fields doesn't allow same relation despite different pivot attributes value #1585

Closed f-liva closed 3 years ago

f-liva commented 5 years ago

Description:

The BelongsToMany field does not allow the creation of existing relationships, although there may be additional fields in the pivot table that discriminate against the same relationship.

Steps To Reproduce:

  1. Install the reproduction repository, migrate and seed the db
  2. Create your nova user with php artisan nova:user
  3. Login to /nova and go to /nova/resources/books/1
  4. I want to create another attachment for the same user – book couple, with a different pivot attribute value
  5. I got the error This user is already attached.

image

davidhemphill commented 5 years ago

There are cases where you could want a duplicate relation to be created, so definitely seems like a bug to me. Thanks for reporting.

f-liva commented 5 years ago

Great to hear this

refucktor commented 5 years ago

Hey guys, is there any workaround to avoid/solve this problem?

jbrooksuk commented 5 years ago

I've been looking into this issue and whilst I've been able to add functionality to remove the validation, it opens up another issue in that there are now duplicate row ids within relationship tables. We could probably resolve this, but I believe it would open up more issues. Leave it with us for now :)

f-liva commented 5 years ago

Okey thanks, do you have any ETA?

f-liva commented 4 years ago

Any news here?

zareismail commented 4 years ago

hi. you can resolve this situation by some hacks on the BelongsToMany field. first: create a new BelongsToMany field and extends the base BelongsToMany field.

second: create new Rule class and extends Laravel\Nova\Rules\NotAttached

third:

change getCreationRules method on the new BelongsToMany field like follow:

    /**
     * Get the creation rules for this field.
     *
     * @param  \Laravel\Nova\Http\Requests\NovaRequest  $request
     * @return array
     */
    public function getCreationRules(NovaRequest $request)
    { 
        $rules = parent::getCreationRules($request);

        $rules[$this->attribute] = collect($rules[$this->attribute])->reject(function($rule) {
            return $rule instanceof \Laravel\Nova\Rules\NotAttached;
        })->all();

        return array_merge_recursive($rules, [
            $this->attribute => [
                new [your-new-rule-class]($request, $request->findModelOrFail()),
            ],
        ]);
    }

at the end; in the new Rule change the passess method like below:


    /**
     * Determine if the validation rule passes.
     *
     * @param  string  $attribute
     * @param  mixed  $value
     * @return bool
     */
    public function passes($attribute, $value)
    {  
        return ! in_array(
            $this->request->input($this->request->relatedResource),
            $this
                ->model
                ->{$this->request->viaRelationship}()
                ->withoutGlobalScopes()
                ->wherePivot('your-column', $this->request->your-column)
                ->wherePivot('your-column', $this->request->your-column)
                ->wherePivot('meal', $this->request->meal)
                ->get()->modelKeys()
        );
    } 
zareismail commented 4 years ago

Hey guys, is there any workaround to avoid/solve this problem?

f-liva commented 4 years ago

@zareismail your solution works perfectly for the creation, while for the deletion or execution of actions remains the problem that Nova acts on all records regardless of the additional column defined to establish the uniqueness of the record.

zareismail commented 4 years ago

@zareismail your solution works perfectly for the creation, while for the deletion or execution of actions remains the problem that Nova acts on all records regardless of the additional column defined to establish the uniqueness of the record.

can you give an example? or share some of that project?

tott commented 4 years ago

Ran into the same issue. Wondering if someone already has a working solution for this.

zareismail commented 4 years ago

I must say there exists another solution arising from a bug. you can change name of your relation. refer here

aniket-magadum commented 4 years ago

@jbrooksuk can we get any updates on when this will be resolved

WARHORSE-PRO commented 4 years ago

@jbrooksuk 8 days from now I have to delivery the project to client! will be great if this is solved in the coming release

zareismail commented 4 years ago

This package will solve your problems

f-liva commented 4 years ago

Nope

This package will not change the default Nova behaviour. Your suggested package is a field, not a core change. And the Nova UI will be still unable to handle same relationships with different fields because, for Nova, will always be the SAME relation instead of different two

AforDesign commented 4 years ago

Is there any progress on fixing this bug?

It's been allmost a year since this issue was placed.

f-liva commented 4 years ago

Apparently no user of Nova has that need. It's hard to understand why such well designed software doesn't support this functionality, which is basic to me, but for that reason I don't think developers will ever add this support.

aniket-magadum commented 4 years ago

I am still waiting for this

SiebeVE commented 4 years ago

This issue is probably related to #2625. Notice that this could have the effect that data is lost because of: https://github.com/laravel/nova-issues/issues/2625#issuecomment-638425042. (When updating a pivot field with extra pivot attributes, all the the other records to with the same foreign keys)

martindanielsson commented 4 years ago

I also ran into this when upgrading to 3.x and started looking around in the source and solved the creation the way @zareismail posted, with a slight modification to make it a bit more versatile.

<?php

namespace App\Nova\Rules;

use Laravel\Nova\Nova;

class NotAttached extends \Laravel\Nova\Rules\NotAttached
{
    /**
     * Determine if the validation rule passes, taking pivot fields into account.
     *
     * @param  string  $attribute
     * @param  mixed  $value
     * @return bool
     */
    public function passes($attribute, $value)
    {
        $query = $this
            ->model
            ->{$this->request->viaRelationship}()
            ->withoutGlobalScopes();

        $resource = Nova::resourceForModel($this->model);

        $resource = new $resource($this->model);

        $pivotFields = $resource->resolvePivotFields($this->request, $this->request->relatedResource); // Check if resource has any pivotFields

        $pivotFields->each(function ($field) use ($query) {
            $query->wherePivot($field->attribute, $this->request->input($field->attribute)); // Include them in the exists query
        });

        return ! in_array(
            $this->request->input($this->request->relatedResource),
            $query->get()->modelKeys()
        );
    }
}

The issue with editing and deleting created entries remain because the edit and delete methods in detail and index views both send only the resource.id.value to the delete-method in backend.

So in my case the image below has multiple relations to id: 1, title: cash, but deleting one of them deletes all of the cash-options regardless of the pivotFields.

belongsToMany

@crynobone this is what I found so far.

trippo commented 3 years ago

I have the same problem of @martindanielsson, when you have a relationship with multiple rows for the same resource, the edit /delete link has the same URL.

navidnadali commented 3 years ago

Thanks @martindanielsson , Just to confirm we have just come across this issue as well (where deleting 1 row deletes all regardless of pivotFields).

Hi @crynobone I saw that you were currently working on Pivot fields in (#2625), is there any scope within that to resolve this? Are there any plans to resolve this? Causing us a lot of issues as we've got to a point of development where release is going to get delayed due to this as we did not envision this being a problem when scoping.

Thanks

crynobone commented 3 years ago

@navidnadali this issue also part of the PR.

crynobone commented 3 years ago

Fixed and released with v3.23.1.

Also review https://nova.laravel.com/docs/3.0/resources/relationships.html#allowing-duplicate-relations

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.