techouse / intl-date-time

International DateTime for Laravel Nova
MIT License
57 stars 21 forks source link

Incorrect date when using without time #25

Closed alexrififi closed 4 years ago

alexrififi commented 4 years ago

Hi! Thank for your package!

background info
My local timezone is **UTC+05** **config/app.php** ```php return [ ... 'timezone' => 'UTC', 'locale' => 'ru', ... ]; ``` In model I have field `birthdate` (type date without time). Nova resource ```php use Techouse\IntlDateTime\IntlDateTime; IntlDateTime::make( 'Дата рождения', 'birthdate' ) ->hideUserTimeZone() ->firstDayOfWeek( 1 ), ```

The problem

Date (field birthdate) is not saved correctly. It is equal to the previous day.

This is what the Vue Devtool shows when choosing a date, for example 16.01.1994

Screenshot

This is related to https://github.com/techouse/intl-date-time/issues/15

techouse commented 4 years ago

Hey!

What setting do you have in your app/Providers/NovaServiceProvider.php?

Try adding:

    public function boot()
    {
        parent::boot();

        Nova::userTimezone(function () {
            return 'UTC';
        });
    }
techouse commented 4 years ago

Bump.

alexrififi commented 4 years ago

@techouse I think I know why it's happend. My project in UTC. My system in UTC+05. I selected 16.01.1994. Your tool think that I really selected 16.01.1994 00:00 in UTC+05 and converts this value to UTC. The result is 15.01.1994 19:00. This value store in db in column type date ==> in db stored 15.01.1994


    public function boot()
    {
        parent::boot();

        Nova::userTimezone(function () {
            return 'UTC';
        });
    }

I can't use forced UTC for all users, cause Russia has many timezones. And user must see local datetime. But in db all datetime fields stored as UTC

That's what i will do:

  1. Add timezone settings in user profile.
  2. Before saving convert value to user timezone. 15.01.1994 19:00 => 16.01.1994 00:00

Looks like a hack)

techouse commented 4 years ago

Thanks for the feedback @medvinator 😃

The whole date parsing is quite basic as you can see here https://github.com/techouse/intl-date-time/blob/master/src/IntlDateTime.php#L294

I guess what I could do in this instance is upgrade the plugin to always use UTC internally. Let me see if that's possible.

techouse commented 4 years ago

@medvinator Try and update to v1.5.6

I've changed the way moment.js sends the data from Vue to PHP. Now it uses .toISOString() which adds the TimeZone you've specified in Nova. Then Carbon parses that TimeZone and feeds that directly to Nova.

https://github.com/techouse/intl-date-time/commit/4bb68c71df2ac2fb7714a9aca692a34b49b3b833#diff-d99220cdf632895fba8dd69a349a9096R185

Jon78 commented 4 years ago

@medvinator Try and update to v1.5.6

I've changed the way moment.js sends the data from Vue to PHP. Now it uses .toISOString() which adds the TimeZone you've specified in Nova. Then Carbon parses that TimeZone and feeds that directly to Nova.

4bb68c7#diff-d99220cdf632895fba8dd69a349a9096R185

This now messes up our timezones. If we set the time to 15:52 it is stored as 13:52. Our timezone is Europe/Amsterdam and we use mysql datetime columns.

techouse commented 4 years ago

@medvinator Try and update to v1.5.6 I've changed the way moment.js sends the data from Vue to PHP. Now it uses .toISOString() which adds the TimeZone you've specified in Nova. Then Carbon parses that TimeZone and feeds that directly to Nova. 4bb68c7#diff-d99220cdf632895fba8dd69a349a9096R185

This now messes up our timezones. If we set the time to 15:52 it is stored as 13:52. Our timezone is Europe/Amsterdam and we use mysql datetime columns.

Indeed you are correct! I messed up the Moment.js TimeZone API. It should be fixed now in v1.5.7 with proper application of Laravel config timezones. Please give it a spin and report back :)

Jon78 commented 4 years ago

Works! Thanks.

EDIT: I can't confirm whether the OP's issue has been resolved though, I just came to complain about timezones :smiley:

techouse commented 4 years ago

I think the issue was caused by the same problem that I wasn't paying attention to timezones at all. 😅

Jon78 commented 4 years ago

Sorry but I spoke too soon, we're still having timezone related issues.

techouse commented 4 years ago

More detail?

Jon78 commented 4 years ago

When I

When I don't select a date this is stored as 2020-04-21 21:31:00 and displayed in Nova as 2020-04-21 19:31:00.

When I revert back to the regular DateTime field things work normally again.

techouse commented 4 years ago

What are your timezone settings? The one in config and in Nova?

Jon78 commented 4 years ago

Our config is 'timezone' => 'Europe/Amsterdam', and the one in Nova is unset, but it doesn't make much of a difference setting it.

rocke-red commented 4 years ago

A have the same issue. If I don't select a datetime field it doesn't change during update a resource. But if I select and then deselect field (even doesn't change!) it decreases a hour by 3 (offset of my timezone) during update a resource.

My app config: 'timezone' => 'Europe/Moscow',

NovaServiceProvider.php: Nova::userTimezone(function () { return config('app.timezone'); });

techouse commented 4 years ago

I'll look into this during the weekend. 🛠

remkobrenters commented 4 years ago

We want to chip in. We tried debugging it without success so far. Basically our setup:

Our conclusion so far is that it looks the CEST is converted to UTC before saving while it thinks on saving the time received is a CEST time (thus the incorrect saved conversion). But maybe I am overthinking this.

techouse commented 4 years ago

What version of Laravel and Carbon do you have?

Jon78 commented 4 years ago

We use Laravel 6.18.10 with Carbon 2.32.2

remkobrenters commented 4 years ago

Laravel 7. Please note that the 'mutation' occurs before submitting.

techouse commented 4 years ago

Fixed.

Chased down the bug to the usage of MomentJS's .toISOString() method which, as well as the one provided by the native Date object, converts the time to UTC regardless of the set timezone. Now I'm using .format("YYYY-MM-DD HH:mm:ss ZZ") which respects the user set timezone and passes that down to Carbon.

I erroneously thought Laravel would automatically apply the timezone when storing it in the database since the date passed to Carbon was UTC (with a timezone) and inside config/app.php one sets a timezone Laravel should respect. Apparently it is not so and that's just for reading purposes it seems.

techouse commented 4 years ago

Please test from your end as well @Jon78 @remkobrenters @rocke-red @medvinator

remkobrenters commented 4 years ago

This looks promising but I still get the same results. Some debugging info:

Form interaction Peek 2020-05-04 06-40

Submitted data customer: 60 customer_trashed: false license_plate: x-16-s label: x86_64 valid_from: 2020-03-19 00:00:00 +0100 valid_until: 2020-04-23 00:00:00 +0200 _method: PUT _retrieved_at: 1588567524

I tested two configurations for the form fields.

First field

DateTime::make(__('vehicles.valid_from'), 'valid_from')
    ->placeholder(false)
    ->rules('required', 'date'),

Second field

DateTime::make(__('vehicles.valid_until'), 'valid_until')
    ->format('d-m-Y')
    ->dateFormat('DD-MM-YYYY')
    ->placeholder(false)
    ->nullable(true)
    ->rules('nullable', 'date', 'after:valid_from'),
rocke-red commented 4 years ago

I use it withTime and now it works correctly

remkobrenters commented 4 years ago

@rocke-red did a short test: If I submit any time it works, if I leave the time at 00:00:00 it will fail in a similar way as stated above by me.

techouse commented 4 years ago

🤔 this is interesting. Will take a look at it.

techouse commented 4 years ago

I can't seem to reproduce this. Here's my setup:

Laravel 5.8.37 Carbon: 2.30.0 Nova: 2.11.1

config/app.php

'timezone' => 'Europe/Amsterdam',

app/Providers/NovaServiceProvider.php

public function boot()
{
    parent::boot();

    Nova::userTimezone(function () {
        return config('app.timezone');
    });
}

Usages that work:

IntlDateTime::make(__('Updated at'), 'updated_at')
                ->placeholder(false)
                ->rules('required', 'date'),
IntlDateTime::make(__('Updated at'), 'updated_at')
                        ->dateFormat('DD-MM-YYYY')
                        ->locale('nl')
                        ->withShortcutButtons(),

date

Jon78 commented 4 years ago

I can confirm this works locally, will test in production later.

remkobrenters commented 4 years ago

Driving me crazy. I only see one big difference in your situation and that is the versions.

Laravel 7.x Nova: 3.x

techouse commented 4 years ago

Driving me crazy. I only see one big difference in your situation and that is the versions.

Laravel 7.x Nova: 3.x

I will test on newer versions of Laravel and Nova on the weekend, however, have you yourself tired your setup with the same versions I have tried above?

remkobrenters commented 4 years ago

I will try to test this before the weekend (or early saturday).

remkobrenters commented 4 years ago

Ok tested it with the provided Laravel and Nova version without success. If I hook into the PerformsValidation trait and I add a dd() for the validateForUpdate $request contents I am pretty sure the data comes from the form.

If I submit 28-05-2020 it is sent as: "valid_until" => "2020-05-28 00:00:00 +0200"

If I go further into the handle() of the ResourceUpdateController I notice that after fillForUpdate this date is changed into "valid_until" => "2020-05-27 22:00:00"

I am pretty sure this +0200 is the issue. At it sends a datetime string with a time of 00:00:00 and a timezone of +0200 which is corrected by Laravel to 00:00:00 minus those +2 hours resulting in 22:00:00 the day before.

The only thing I cannot get is why your instance is not having this behaviour. I tested this with a complete empty project with the provided versions. Tested it (just to be sure) in multiple browsers.

remkobrenters commented 4 years ago

For what it's worth, I managed to resolve this issue by changing the casting of dates to not include the time which resolves this issue for us:

/** @var array<string, string> */
protected $casts = [
    'valid_until' => 'datetime:Y-m-d',
];