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

Having a null updated_at column throws an error when updating model #2533

Closed
goodevilgenius opened this issue Apr 14, 2023 · 4 comments
Closed

Comments

@goodevilgenius
Copy link
Contributor

goodevilgenius commented Apr 14, 2023

  • Laravel-mongodb Version: 3.9.2 (should still be present in 3.9.5)
  • PHP Version: 8.1.17
  • Database Driver & Version: 4.4

Description:

When UPDATED_AT (or CREATED_AT) is set to null in the model, a warning is thrown when updating the model.

class SomeModel extends Model
{
    public const UPDATED_AT = null;
}

Setting UPDATED_AT to null is a common way to allow for a created_at column, but not updated_at.

For example:
https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Concerns/HasTimestamps.php#L62-L70

Here, Laravel checks that the updated_at column isn't null before setting it.

However:
https://github.com/jenssegers/laravel-mongodb/blob/v3.9.5/src/Eloquent/Model.php#L226-L230

Here, we get all the dates columns, which could include nulls. Then, calling Str::contains($key, '.') results in a deprecation warning:

PHP Deprecated: str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in /var/www/app/vendor/laravel/framework/src/Illuminate/Support/Str.php on line 242

Steps to reproduce

  1. Set UPDATED_AT to null in a model class.
  2. Call $model->update([/* ... */]);
  3. See deprecation warning

Expected behaviour

null should be skipped

Actual behaviour

Warning

Possible fix

        foreach ($this->getDates() as $key) {
            if ($key && Str::contains($key, '.') && Arr::has($attributes, $key)) {
                Arr::set($attributes, $key, (string) $this->asDateTime(Arr::get($attributes, $key)));
            }
        }
@goodevilgenius
Copy link
Contributor Author

I'll try to get a PR up for this on Monday morning.

@puuble
Copy link

puuble commented Apr 15, 2023

Hi can you try to add this dates in Model with protected $dates = ['startDate', 'endDate'];

WorkPeriod::create([ 'samba_id' => $id, 'startDate' => $startDay, 'endDate' => null, 'user' => $user_id ]); I used like this

@goodevilgenius
Copy link
Contributor Author

goodevilgenius commented Apr 17, 2023

@puuble that's not what I'm talking about. I don't mean having a nullable date column. That doesn't cause problems.

I'm talking about not having an updated_at column, while still having a created_at column. You can do that by enabling timestamps, but setting your updated column (not the value in the column) to null.

I updated my description to make it clearer.

@goodevilgenius
Copy link
Contributor Author

Actually, this looks like it was fixed in 2ea1a7c which doesn't seem to have been included in recent releases.

I guess it'll be fine with versions that support Laravel 10.

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

No branches or pull requests

2 participants