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

Fix belongsToMany relation #543

Merged
merged 12 commits into from
Dec 1, 2020
Merged

Fix belongsToMany relation #543

merged 12 commits into from
Dec 1, 2020

Conversation

mjauvin
Copy link
Contributor

@mjauvin mjauvin commented Nov 28, 2020

Follows on from #536.

I also added the test that was failing from october/october to make sure this actually fixes the problem.

@bennothommo bennothommo merged commit ef6b7f0 into octobercms:develop Dec 1, 2020
@bennothommo bennothommo deleted the fix-belongs-to-many-relation branch December 1, 2020 07:36
@Denoder
Copy link
Contributor

Denoder commented Dec 20, 2020

I hadn't updated my library in a bit (since my pull request). The when i updated to this release i ended up getting a lot of ambiguous errors:

i had to remove

    /**
     * Create a new query builder for the pivot table.
     *
     * This is an extension of Laravel's `newPivotQuery` method that allows `belongsToMany` and `morphToMany` relations
     * to have conditions.
     *
     * @return \Illuminate\Database\Query\Builder
     */
    public function newPivotQuery()
    {
        $query = parent::newPivotQuery();

        // add relation's conditions and scopes to the query
        $this->addDefinedConstraintsToQuery($query);

        return $query->join($this->related->getTable(), $this->relatedPivotKey, '=', $this->relatedKey);
    }

    ```

@bennothommo
Copy link
Contributor

@teranode are you using either of those fields for conditions or scope filters when defining the relation?

@Denoder
Copy link
Contributor

Denoder commented Dec 21, 2020

@bennothommo None of them uses conditions or scopes.

@mjauvin
Copy link
Contributor Author

mjauvin commented Dec 21, 2020

@teranode Can you show the relations definitions for the models exhibiting this error?

Also, the migration table for the pivot table would be helpful.

@mjauvin
Copy link
Contributor Author

mjauvin commented Dec 21, 2020

@teranode maybe opening a new issue for this would be better... you can reference this PR for reference.

LukeTowers pushed a commit that referenced this pull request Feb 3, 2021
* return a collection, not an object

* use fully qualified key names to avoid conflicts

* add pivot data test; add timestamps

* override sync() method in order to flush query cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants