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

Float attribute: Change null to 0 doesn't save to DB - HasAttributes::originalIsEquivalent() bug #33431

Closed
sshead opened this issue Jul 4, 2020 · 4 comments

Comments

@sshead
Copy link
Contributor

sshead commented Jul 4, 2020

  • Laravel Version: 7.18.0
  • PHP Version: 7.4.6
  • Database Driver & Version: MySQL 5.7

Description:

It seems recent changes to fix originalIsEquivalent for floats in \Illuminate\Database\Eloquent\Concerns\HasAttributes have broken the ability to change null to 0 with a float attribute.

That is, if you do $model->attr = 0 then $model->save(), the change is not saved to the database, because originalIsEquivalent('attr') returns true (so $model->getDirty() returns []).

I think it started with #33259, but the most recent version seems to be #33322.

PS: The first two tests in originalIsEquivalent() are:

if ($attribute === $original) {
    return true;
} elseif (is_null($attribute)) {
    return false;
}

I'm wondering if you could simply change the second of these to elseif (is_null($attribute) || is_null($original))? Is there any case in which the original can be null and the new value non-null, but they are considered equivalent?

PPS: I'm a very part-time volunteer dev - I've almost never done this sort of thing, so hope I'm doing it right...

Steps To Reproduce:

Here's the result of my tinkering (some irrelevant output lines omitted):

>>> $result = App\Models\Result::first();
=> App\Models\Result {#4410
     id: 1,
     mark_0: 15.0,
     mark_1: 75.0,
     mark_2: null,
   }
>>> $result->mark_2 = 0;
=> 0
>>> $result->getDirty();
=> []
>>> $result->save();
=> true
>>> $result->fresh();
=> App\Models\Result {#4394
     id: 1,
     mark_0: 15.0,
     mark_1: 75.0,
     mark_2: null,
   }

And here's the same test run on an earlier version (7.12) of the framework:

>>> $result = App\Models\Result::first();
=> App\Models\Result {#4387
     id: 1,
     mark_0: 15.0,
     mark_1: 75.0,
     mark_2: null,
   }
>>> $result->mark_2 = 0;
=> 0
>>> $result->getDirty();
=> [
     "mark_2" => 0,
   ]
>>> $result->save();
=> true
>>> $result->fresh();
=> App\Models\Result {#4400
     id: 1,
     mark_0: 15.0,
     mark_1: 75.0,
     mark_2: 0.0,
   }
@driesvints
Copy link
Member

Did #33421 fix this?

@driesvints
Copy link
Member

Try requiring 7.x-dev as it's unreleased atm.

@sshead
Copy link
Contributor Author

sshead commented Jul 4, 2020

Sorry, missed that - though is that merged to 7.x? I tried 7.x-dev and it's still the old version.

@sshead
Copy link
Contributor Author

sshead commented Jul 4, 2020

Oh - seems there are other differences between 6.x and 7.x, so implementation would be slightly different ($attribute not $current).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants