Fix model.setAttribute event value

[mjauvin]

Fixes wintercms/winter#1047
Replaces wintercms/winter#1071

[mjauvin]

@bennothommo any idea about the larastan error ?

[bennothommo]

@mjauvin I have no idea - it seems to be happening randomly at the moment. Retrying the test usually resolves it.

[LukeTowers]

It could just be because it's late and I should probably go to bed, but why exactly is this necessary? This feels very wrong, to be modifying the value returned from the event; why wouldn't it be the job of the listener to retrieve the accessor value if it didn't like the value as provided by this event?

[bennothommo]

It goes to the expectation of the event - given that there's a beforeSetAttribute event as well, and the value itself is potentially modified from when it first went it to the setAttribute() method (it could be trimmed or JSON-encoded or modified by the beforeSetAttribute event), the implication is that the setAttribute event (being an after the fact event) should contain what the attribute is now it has been set, so arguably, this should be after it has been mutated as well.

[LukeTowers]

But the setAttribute is dealing with the raw value of the attribute as stored in the attributes array, it's not dealing with the value as potentially mutated through an accessor method; so I'm concerned that this will break things.

Why does it need to be done this way, can't the consumer of this event change what it's doing if it's having problems with that value?

[bennothommo]

My main concern is that there's potentially a disconnect between the value that's provided to the event and the value that's actually stored in the attributes array, and I was simply following the behaviour that's being exhibited by these lines:

• https://github.com/wintercms/storm/blob/develop/src/Database/Model.php#L1362
• https://github.com/wintercms/storm/blob/develop/src/Database/Model.php#L1369
• https://github.com/wintercms/storm/blob/develop/src/Database/Model.php#L1376

In that they're modifying the original value set in the parameters (as $value) before being sent to the setAttribute event.

If the intention of the event was to send the value pre-modification to the setAttribute event, then it's not doing that currently and should probably be fixed. If on the other hand, it was to actually be what's stored in the attributes array at that point, then it needs to be modified to include any modification by a mutator, hence changing the event as @mjauvin has proposed.

I'm certainly amenable to adding this as an additional parameter to the event if we must, but I'd rather fix the oversight if possible.