[9.x] Add handling of object being passed into old method

[Drewdan]

Hey Guys!

This PR allows users to pass in a Model as the default parameter to the old helper function.

If you had a form to update a username. You might set the value to something like this:

<input type="text" name="name" value="{{ old('name', $user->name) }}">

This will find the old value in the request or, default to the $user->name value which is being pulled from the User model if there is no old value.

This PR changes this to allow the following

<input type="text" name="name" value="{{ old('name', $user) }}">

The method has been updated to check if the default value being passed is an instance of a Model and gets the attribute based on the key

As many developers will follow cruddy conventions, and have their attributes named the same as their input names, so they can mass insert, I believe this would streamline form creation (if only slightly)

I have added a test to cover this behaviour, and I have also added a test to cover setting the default as an array, to ensure no existing behaviour is broken by this change.

An alternate approach I considered would be to add a new helper method, (though, not sure what the name would be, oldAttribute($class, $key) maybe?) , which would do this behaviour independently of the old helper method. I'd be happy to scope this out if this approach would be preferred.

Thanks for looking :)

[driesvints]

I personally think we shouldn't do this and this is best handled in userland but let's see what @taylorotwell says.

[taylorotwell]

@driesvints

There was no coupling before. You can make instanceof checks against classes that don't exist. So instanceof Model would not fail even if database was not pulled in.

[driesvints]

Ah damn, I always forget about that behavior of instanceof 🤦

[Drewdan]

I didn't realise that was a behavior of instanceof so, I learnt something new! :)

[taylorotwell]

@Drewdan I generally kinda like this PR overall - can you revert your recent changes and bring back the instanceof Model check so I can review it again in that form?

[Drewdan]

@taylorotwell - I shall do, I will post a new commit shortly

[Drewdan]

@taylorotwell - I have pushed a new commit up.

This one is different from my initial commit in that is just checks to see if $default is an instanceof Model, and if it is, calls getAttribute rather than also checking the array returned by getAttributes contains the key.

The reason for this is I believe getAttribute handles all of this anyway, so I think checking the array contains the key is redundant.

I have also left the array test in just to make sure this does not break any existing behavior

[taylorotwell]

Thanks!

[Drewdan]

Thank you :)

[joshhanley]

I know this is an old PR by now, but I've finally had a change to upgrade my Laravel version on one of my apps, and found this caused a breaking change for me. I've fixed up my code, so it's not an issue anymore for me, but I thought I'd make a note 🙂

In my scenario, I'm using the old() function to determine whether to output checked on a checkbox. In my case though, I'm not using the same name for the key as attributes available on the pivot model, I'm more just checking the existence of the pivot table model in the variable.

@if(old('operators.' . $loopIndex . '.isAllocated', $jobAllocation)) checked @endif

Previously, it would return the default variable if no old value was set, as such would be true for my conditional.

But now it's trying to look for the key 'operators.' . $loopIndex . '.isAllocated' on the $jobAllocated pivot model, which doesn't exist, hence ->getAttribute() is returning null. Therefore my checks are always returning false when the default is being used, even if the variable $jobAllocated is populated.

I've updated my code now to just wrap the variable in an isset() and it's all working happily again.

@if(old('operators.' . $loopIndex . '.isAllocated', isset($jobAllocation))) checked @endif

Hope this helps someone else if they are running into a similar problem!

[Drewdan]

@joshhanley that's interesting, I'd never thought of that sort of usage. Thank you for documenting it here, though. I hope I didn't cause too much of a headache for you!