Maybe in the order of attributes 'receive_sms' is set before 'phone_mobile', so it's to late to change it.
Setting another attribute from within a mutator
Hi everyone,
In my Users table I have a column for phone_mobile and receive_sms. I am updating/creating the User with User::create() and User::update().
I am stripping out all but numbers when saving the phone_mobile attribute but would also like to set receive_sms to null if the phone_mobile attribute is empty. Here is what I have tried:
public function setPhoneMobileAttribute($phone)
{
if ($phone) {
// Strip all but numbers
$phone = preg_replace('/\D/', '', $phone);
$this->attributes['phone_mobile'] = $phone;
} else {
$this->attributes['phone_mobile'] = null;
$this->attributes['receive_sms'] = null;
}
}
The receive_sms attribute is untouched and I cannot seem to figure out how to change another attribute from within a mutator. Any help would be appreciated, thanks!
@bestmomo Okay, is there anything I can do about this or any way to determine if this is the case? Really want to lessen the amount of code in my controllers like Laravel Foundations teaches but finding it very difficult
I think you need to change values before update or save your model. Depending of your application logic.
@bestmomo Can you be a little more specific? Are you basically saying I can't use ::update() and ::create()?
When you use create you have an array as parameter. Couldn't work on it to manage your condition and set null where you want ?
@bestmomo I am using the following:
public function postUser(UserRequest $request)
{
$id = $request->input('id');
if ($id) {
$user = User::find($id);
$user->update($request->all());
} else {
$user = User::create($request->all());
}
Flash::success("User details saved successfully!");
return redirect("admin/users/user/{$user->id}");
}
Contents of UserRequest:
<?php namespace App\Http\Requests;
use App\Http\Requests\Request;
use Auth;
use Input;
class UserRequest extends Request {
protected $rules = [
'first_name' => 'required',
'last_name' => 'required',
'email' => ['required', 'email'],
'phone_mobile' => ['regex:/^[0-9]{3}-[0-9]{3}-[0-9]{4}$/']
];
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
if (!Auth::user()->hasRole('admin')) {
return false;
}
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
$id = Input::get('id');
$rules = $this->rules;
if ($id)
{
$rules['email'][] = "unique:users,email,$id";
$rules['phone_mobile'][] = "unique:users,phone_mobile,$id";
} else {
$rules['email'][] = "unique:users";
$rules['phone_mobile'][] = "unique:users";
}
return $rules;
}
}
One way : if you want a light controller just create a repository to manage queries. Create an update and a create function to call from controller. In this repository create a private function to change values in $request->all() to manage your null values in inputs.
@bestmomo It really seems like this should be possible to accomplish somehow with mutators... keep running into frustrations like this. :/
Maybe someone else will have some insight ;)
@bestmomo Thanks for your help.. in the meantime I'm just getting each value manually and setting receive_sms to null of the phone_mobile field is missing. I've always done it this way but it would be nice to just use $request->all() and take advantage of Form Requests and Mutators.
Indeed, if anyone else has some insight, please share. Thanks!
@opheliadesign Generally, you can set other attributes in a mutator. Just tried, works perfectly fine. However, it doesn't work if you pass receive_sms as part of $request->all(), because then the value set in your mutator is just overwritten.
The receive_sms attribute is untouched and I cannot seem to figure out how to change another attribute from within a mutator. Any help would be appreciated, thanks!
So I have to ask: You sure that receive_sms isn't even part of $request at all?
@cm Trying to understand exactly what you're saying here. In other words, if I want to conditionally set receive_sms to null if phone_mobile is empty in my mutator, I cannot pass receive_sms at all? If this is the case, I'll have to manually get each form input and...then what? At that point I might as well do the following, which is the only way I could figure this out:
$id = $request->input('id');
$first_name = $request->input('first_name');
$last_name = $request->input('last_name');
$email = $request->input('email');
$phone_mobile = $request->input('phone_mobile');
$receive_sms = $phone_mobile ? $request->input('receive_sms') : null;
$receive_email = $request->input('receive_email');
$fields = compact('first_name', 'last_name', 'email', 'phone_mobile', 'receive_sms', 'receive_email');
if ($id) {
$user = User::find($id);
$user->update($fields);
} else {
$user = User::create($fields);
}
Also, "So I have to ask: You sure that receive_sms isn't even part of $request at all?" - not sure what your question is here. I guess if the checkbox (receive_sms) isn't checked, it is not included with $request->all()?
Thanks for your help.. :)
If you have the receive_sms in your form then it is in the request ;)
@blackbird Okay so then I'm confused about @cm's question.
I think he is saying that if you don't have that value (receive_sms) in your request it will not change in your mutator, because the mutator doesn't know the field
Could be wrong but I think he means that!
@opheliadesign You need a bit more work:
$user = User::find($id); // new User for creation
$user->fill($request->except('phone_mobile'));
$user->phone_mobile = $request->get('phone_mobile');
$user->save();
Also I would adjust the mutator:
public function setPhoneMobileAttribute($value)
{
$phone = preg_replace('/\D/', '', $value);
$this->attributes['phone_mobile'] = $phone ?: null;
if ( ! $phone) $this->attributes['receive_sms'] = null;
}
When you create an object User::create($request->all());, your attributes are set in some arbitrary order, maybe like so:
- first_name
- last_name
- phone_mobile
- receive_sms
phone_mobile has a mutator, which sets receive_sms. But then, in the next step, receive_sms is set as part of the create() function. Thus, the value from the mutator is simply overwritten.
The only way to prevent this from happening, is if receive_sms isn't part of User::create($request->all());.
So, the logic to unset receive_sms must be somewhere else AFTER all attributes were set by your batch assignment. You could do this:
$user = new User($request->all());
if (!$user->phone_mobile) $user->receive_sms = null;
$user->save();
So I have this idea, don't know it works, why don't you use 2 mutators. One for the phone field and one for the receive_sms field. So in one you do the phone part and in the other one you check for the phone part and update the receive_sms field
@JarekTkaczyk That does the trick!! First time I've ever seen ->fill(), where is this documented? Quick search through the docs did not reveal it.
Could you give me a breakdown of exactly what is being accomplished with this code? Is this to make sure that phone_mobile is the last field to go through the mutator?
@blackbird @JarekTkaczyk @opheliadesign I wouldn't do anything like that. The idea is simply kind of wrong. In your mutators, you should NOT rely on a specific order how attributes are set. If you want to unset something based on another attribute, you should enforce this somehow in your business logic and not in your mutator. Mutators are used for setting one attribute in a specific way. Everything else can imho lead to side effects and unreadable code in the future.
After all, you have to type these exact lines all over whenever you want the mutator to work:
$user->fill($request->except('phone_mobile'));
$user->phone_mobile = $request->get('phone_mobile');
And if you have to type this stuff all the time (and it won't work if you don't do it like this), you can go with the code I posted. This is way more readable without side effects.
@opheliadesign fill basically sets all the fillable/not guarded attributes provided as an array, but using mutators (yours and eg. default date mutators) if applicable. It is called whenever you do this:
new Model($attributes);
Model::create($attributes);
$model->update($attributes);
$model->updateOrCreate($attributes);
@cm While I'm not a fan of such solutions (* see below), I can't agree with you saying you can't rely on order in mutator. It is not the case. You just want to make sure that when the phone is set to null, another attribute is set to null as well.
The order, you're refering to, happens only in this specific case - when you are using create or update directly on the model. And that's why the code needs adjustment for a specific situation.
But mind that you can also have other calls, like $user->phone = whatever somewhere else, and that's legit use case for such mutator.
* Better option for this would be simply checking the phone when you use receive_sms.
Okay, need an unbiased opinion on the best/safest way to accomplish this. I think this thread will be of some help to others, I would imagine this is a fairly common scenario; especially when trying to do things the "Laravel 5 way," less logic in controllers and more use of Form Requests and mutators.
Yea, this is a perfect example for Laravel starters to see what all the possibilities are and which solutions can be used ;)
You just want to make sure that when the phone is set to null, another attribute is set to null as well.
But you're actually not making sure that this is happening. The mutator in question leads to inconsistent or unexpected behavior. It works in this case: $this->phone = null, but not in one where you batch-assign things or even in this case:
$this->phone = null;
$this->receive_sms = 123;
There's too much hidden logic behind the phone mutator which leads to mistakes. You expect to know that every programmer (or yourself in 2 years) still know that unsetting the phone will unset receive_sms as well. Doable, but error-prone. There are simply better ways to enforce behavior like that.
@cm Not every User will want to receive SMS messages, hence the receive_sms boolean in the table. However, if a user does not supply a phone_mobile, we know that they also cannot receive_sms - see where I'm going with this?
Sure, when I run the query to retrieve phone numbers for a new SMS message, I could check for receive_sms being true AND phone_mobile not being null, but I'd rather just grab all entries with receive_sms knowing they'll also have a phone number.
@opheliadesign I totally understood your use case and there's nothing wrong with it. It's just that this is not the job of a mutator. A mutator is made for modifying a single attribute in a minor way, because of the side effects I explained before. If you need to cascade several effects, you should do it in another way.
@opheliadesign One more thing: The topic you're talking about is called model validation and is quite complex. This can be simple things like in your case: set B to null if A is null. Or quite complex. There aren't many threads in this forum, but this might help you: https://laracasts.com/discuss/channels/general-discussion/form-validation-or-model-validation
If you just want to ensure that receive_sms in the DB is null whenever the phone is null, you might want to intercept the model's save() method and set receive_sms just before you save. This video explains how to do it:
@cm Is this information available for free somewhere? Like in documentation?
@opheliadesign This should do the trick:
class User ... {
...
public static function boot() {
parent::boot();
static::saving(function($model) { // get's called BEFORE model is saved
$model->checkIntegrity();
return true; // true = continue saving. false would cancel saving
});
}
public function checkIntegrity() {
if (!$this->phone_mobile) $this->receive_sms = null;
}
}
Please note that this behavior has its downsides, too. E. g. if you set phone_mobile to null without saving, receive_sms will still have its value. This is just for validating stuff that goes in your DB. Not perfect, but as I said, complex topic ;)
Please or to participate in this conversation.