wdelfuego / nova-datetime

Makes working with DateTime fields in Laravel's Nova easier
10 stars 1 forks source link

Support timezone! #1

Closed 4n70w4 closed 1 year ago

4n70w4 commented 2 years ago

Hi! I configured timezone in config/app.php, for example:

    'timezone' => 'Europe/Moscow',

Then I added this DateTime field and Nova default DateTime field:

            \Wdelfuego\Nova\DateTime\Fields\DateTime::make(__('Date 1'), 'date'),
            \Laravel\Nova\Fields\DateTime::make(__('Date 2'), 'date'),

and I saw:

image

Nova default DateTime field show time with applying timezone. This DateTime field show time without applying timezone.

wdelfuego commented 2 years ago

Hi, thanks for reporting this issue!

What values would you expect to see under Date 1 exactly?

wdelfuego commented 2 years ago

Also, what is the database column type and could you show a screen shot of the raw values as they are stored in your database for those 4 models?

4n70w4 commented 2 years ago

Just need to add something like this: image

wdelfuego commented 2 years ago

I am aware of that, but I want to be sure that I understand your specific case because my test environment is showing different results.

What specific values would you expect to see in the column under Date 1 instead of the ones in your screenshot? It'd be really helpful.

Thanks!

wdelfuego commented 2 years ago

I've been working trying to understand exactly what's going on here, but so far I can't reproduce the behavior you describe and I'm not sure why. I suspect your proposed change is what's required but since quite some people already rely on this package I don't want to release an update without completely understanding what's going on and being 100% certain about why I'm changing what I'm changing. I hope you can help me understand better.

In my test environment I made a Nova resource with an Eloquent model that has two datetime cast attributes:

On the Nova resource, as you did, I show both properties twice to be able to see any difference between the default Nova field and this package's field:

use Wdelfuego\Nova\DateTime\Fields\DateTime;
use Laravel\Nova\Fields\DateTime as NovaDateTime;
    public function fields(NovaRequest $request)
    {
        return [
            Fields\ID::make()->sortable(),
            DateTime::make('tz wdelfuego', 'moment'),
            NovaDateTime::make('tz nova', 'moment'),
            DateTime::make('wdelfuego', 'updated_at'),
            NovaDateTime::make('nova', 'updated_at'),
        ];
    }

I start with the timezone value in config/app.php set to the default UTC value and get the following output:

Screenshot 2022-06-19 at 20 35 05

I then change the timezone value in config/app.php to Europe/Moscow as you did and get the following output:

Screenshot 2022-06-19 at 20 36 25

The timezone indicators changed, but the date and time values shown are the same and both fields behave identically.

I then changed the withDateFormatFunction by adding the setTimeZone method as you proposed and nothing changes.

I suspect that what's going on here is that your results are different from mine because you are setting timezone info on your DateTime values somewhere, which I'm not doing, and you then want to show those timezoned DateTime values relative to your app's configured timezone instead of to their own, which the Nova field does but my custom field doesn't do, but I'm not 100% certain because I can't reproduce it yet.

My money is on this being an issue and your proposed solution being the correct solution, I just want to be absolutely sure of what problem it is that I'm solving before releasing an update to possibly hundreds of users.

I hope you can help me reproduce and understand the issue better so I can be certain before releasing the fix.

Thanks in advance!!

wdelfuego commented 2 years ago

Hi @4n70w4! If you could help me exactly replicate and understand your issue that’d be great!

If not, don’t worry, maybe someone else having timezone issues with this package could chime in. Thanks! 🙏🏻

4n70w4 commented 2 years ago

Database: postgres:14.3 column type: timestamp with time zone

image

Nova Created At shows correct time with my app config timezone. Wdelfuego Created At shown incorrect time with ignored my app config timezone. image

image

wdelfuego commented 1 year ago

Hi, thanks for getting back and sorry for the late reply. I have fixed this in version 1.1.2 which was just released.

Tanariel commented 8 months ago

Unfortunately the implemented solution doesn't follow the nova behavior with timezones.

the solution you implemented

public static function withDateFormatFunction() : callable { return function (string $format) { return $this->displayUsing(fn ($d) => ($d instanceof Carbon) ? (config('app.timezone') ? $d->setTimezone(new DateTimeZone(config('app.timezone')))->translatedFormat($format) : $d->translatedFormat($format)) : (($d instanceof DateTimeInterface) ? (config('app.timezone') ? $d->setTimezone(new DateTimeZone(config('app.timezone')))->format($format) : $d->format($format)) : '') ); }; }

is using config('app.timezone') which is the default timezone of the application. But laravel nova is providing a way to personalise the timezone per user : see => https://nova.laravel.com/docs/4.0/resources/date-fields.html#timezones

use Laravel\Nova\Nova;
use Illuminate\Http\Request;

/**
 * Bootstrap any application services.
 *
 * @return void
 */
public function boot()
{
    parent::boot();

    Nova::userTimezone(function (Request $request) {
        return $request->user()?->timezone;
    });
}

to be compatible with nova and the default customisation of timezone foreach user, the code should somehow look like this instead :

$d->setTimezone(new DateTimeZone($request->user()->timezone ?? config('app.timezone')))->translatedFormat($format)

if you see what i mean ? Actually i'm facing the problem where the app default TZ is UTC, but users are coming from multiple tz and will only see the fields in the app default tz if i use this bundle.