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 'setRawAttribute' method #30853

Merged
merged 1 commit into from Dec 16, 2019

Conversation

@gdebrauwer
Copy link
Contributor

gdebrauwer commented Dec 16, 2019

// Set attribute to value.
$model->setRawAttribute('key', $value);

// Set attribute to value and also sync to original attribute.
$model->setRawAttribute('key', $value, true);

Currently it's not possible to skip a mutator of one specific attribute.
The setRawAttributes method exists, but that requires you to provide values for all attributes.

I did not add any tests, because I could not find one for setRawAttributes. Let me know if I should add one.

@taylorotwell taylorotwell merged commit 503e9c3 into laravel:6.x Dec 16, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@goodeas

This comment has been minimized.

Copy link

goodeas commented Dec 19, 2019

@taylorotwell I believe this creates a clash: If there is an attribute named 'raw', and the developer wanted to set its value with a mutator using $model->raw = 'value', that mutator method would be named setRawAttribute(...), with a different signature to this new one.

taylorotwell added a commit that referenced this pull request Dec 19, 2019
This reverts commit 503e9c3.
taylorotwell added a commit that referenced this pull request Dec 19, 2019
This reverts commit 503e9c3.
@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 19, 2019

I don't see why this was needed anyway. If someone wants to bypass the framework to set the actual value, they can... the attributes are a public property.

@driesvints

This comment has been minimized.

Copy link
Member

driesvints commented Dec 19, 2019

We've reverted this. See #30884

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 19, 2019

I know, that's what lead to my comment, to help anyone who wants this feature. :)

@driesvints

This comment has been minimized.

Copy link
Member

driesvints commented Dec 19, 2019

@GrahamCampbell think we cross-posted at the same time

@gdebrauwer

This comment has been minimized.

Copy link
Contributor Author

gdebrauwer commented Dec 19, 2019

@GrahamCampbell the attributes is a protected property.
But I completely agree it should be reverted in its current form, maybe a setRaw function could be better?

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 19, 2019

Heh, well, it can still be set from within the model.

@gdebrauwer

This comment has been minimized.

Copy link
Contributor Author

gdebrauwer commented Dec 19, 2019

Oh you mean ‘public’ in that way 😄 I added the ‘setRawAttribute’ because I wanted to set an attribute without created dedicated method in my model

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 19, 2019

Oh you mean ‘public’ in that way

Nah, I meant it in the real way, but was wrong. :P

@GrahamCampbell

This comment has been minimized.

Copy link
Member

GrahamCampbell commented Dec 19, 2019

But the premise is the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.