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

[6.x] Add AsMorphPivot trait #34488

Closed
wants to merge 2 commits into from

Conversation

idsulik
Copy link

@idsulik idsulik commented Sep 23, 2020

The benefit of the trait is that you can add it to your custom model(which extends base Model, but not Pivot or MorphPivot model).
There is already exists AsPivot trait which is useful, so I add AsMorphPivot for morph pivot models

@idsulik idsulik changed the base branch from 8.x to 6.x September 23, 2020 12:41
@taylorotwell
Copy link
Member

Why would you have a morph pivot model that does not extend MorphPivot?

@idsulik
Copy link
Author

idsulik commented Sep 23, 2020

@taylorotwell , because of the error - Invalid parameter number: parameter was not defined (SQL: update "model_properties" set "status" = rejected, "updated_at" = 2020-09-23 12:48:00 where "" = section and "" = 469)

$sectionProperty = factory(ModelProperty::class)->create([
  'relation_type' => ModelProperty::RELATION_TYPE_SECTION,
  'relation_id' => $section->id,
  'status' => ModelProperty::STATUS_DRAFT,
]);
$sectionProperty->status = $newStatus;
$sectionProperty->save();

It works only if I extends base Model and use AsMorphPivot trait

@GrahamCampbell GrahamCampbell changed the title Add AsMorphPivot trait [6.x] Add AsMorphPivot trait Sep 23, 2020
@idsulik
Copy link
Author

idsulik commented Sep 23, 2020

@taylorotwell One more reason #25851 to use it on model which extends my own abstract model

@idsulik
Copy link
Author

idsulik commented Sep 23, 2020

If extends MorphPivot and try to update the model I get the sql:
update "model_properties" set "status" = ?, "updated_at" = ? where "" = ? and "" = ? - where are morph column names?
image

ModelProperty is simple MorphPivot model:
image

It works only if I add the AsMorphPivot trait

@taylorotwell
Copy link
Member

We aren't adding new features to 6.x and this also needs a lot more explanation. It feels like we should fix the root cause of your problem - not just slap everything in a trait and say you need to extend Model.

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

Successfully merging this pull request may close these issues.

2 participants