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

Use fromRawAttributes() for custom pivot models #184

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Conversation

mjauvin
Copy link
Member

@mjauvin mjauvin commented Jul 29, 2024

@mjauvin
Copy link
Member Author

mjauvin commented Jul 29, 2024

@hirisov if you could test this fix, that would be appreciated.

Thanks.

@mjauvin
Copy link
Member Author

mjauvin commented Jul 29, 2024

@bennothommo this should resolve the issue with the User's Roles awards in Winter.Test plugin as well.

@LukeTowers
Copy link
Member

@mjauvin does line 838 also need to be changed?

@mjauvin
Copy link
Member Author

mjauvin commented Jul 29, 2024

@mjauvin does line 838 also need to be changed?

I don't think so, but I'm not 100% sure.

@LukeTowers LukeTowers merged commit 339af87 into develop Jul 29, 2024
8 checks passed
@LukeTowers LukeTowers deleted the fix-pivot branch July 29, 2024 20:27
@mjauvin
Copy link
Member Author

mjauvin commented Jul 30, 2024

@mjauvin does line 838 also need to be changed?

According to the newPivot() method in Laravel's Eloquent Model, I think we're good:

    public function newPivot(self $parent, array $attributes, $table, $exists, $using = null)
    {   
        return $using ? $using::fromRawAttributes($parent, $attributes, $table, $exists)
            : Pivot::fromAttributes($parent, $attributes, $table, $exists);
    }       

But I realize we could completely remove our overriden method now, since it's a carbon copy of Laravel's ...

@mjauvin
Copy link
Member Author

mjauvin commented Jul 30, 2024

See #185

@LukeTowers
Copy link
Member

See #185 (comment), our override is necessary to return our version of the Pivot model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repeater field not initialzed properly pivot modal
2 participants