Regarding dates ...

[MortenDHansen]

Hi all! (@erikn69 , @anteriovieira @parallels999 etc)

I am preparing a v14 branch, and in that process i am considering different scenarios that has been handled "here and there". One of those are how we handle datetime.

I am torn between letting the package itself lean on the principle of "simple storage of whatever is currently written", or "recording what is in the table".

The difference i subtle, but with regards to dates, the timezone on save is a piece of information we are ignoring when we save the datetime string as we are used to seeing it in MySQL.

Most database engines will keep a datetime column in some sort of format that you unpack on retrieval. As PHP and laravel developers we may be used to seeing the "Y-m-d H:i:s" format, but if we lean on this in the package, we may be limiting a bit too much, as this is not literally the information we have.

Laravels own json serializer formats according to the ISO-8601 standard (as proposed here: laravel/ideas#1940) - and thus basically doing what carbon does

My suggestion would be to change behaviour to lean on the ISO-8601 approach when serializing dates to json.

Pros or cons?

[MortenDHansen]

Basically, we currently takes "whatever eloquent casts to us". Im saying we should consider taking what is in the table instead.

[anteriovieira]

Hi @MortenDHansen , I agree with you.

[anteriovieira]

Hi, @erikn69 @parallels999 any thoughts?

And can you send me your discord username? Then I can invite you to private channels.

[parallels999]

Pros or cons?

What about backwards compatibility?
I mean with those who have millions of audits with "whatever eloquent casts to us"
Is there any way to upgrade those json?

[parallels999]

in that case it looks good to me,
it is better to simulate the operation of laravel as much as possible

[MortenDHansen]

Well, no guarantee i'm right, but the above pull request appears to be handling a case, that could have been handled correctly. I fear that its harder to maintain and reason fixes, than it is to solve the issue. I am a little uncertain about the effects of the change though.

[parallels999]

That's also a problem, it takes time to fix unexpected issues, possible incompatibilities with 3rd party packages using this

PHP7 support should be removed and PHP8 syntax adopted

Is #635 (comment), #588 related with this?

[MortenDHansen]

I'll push the current state of my proposed v14, but yes. I aim to make the behavior consistent so that these issues would not occur. ... And drop php 7.x support following laravels minimum 8.0

[MortenDHansen]

Here is the v14 dev base branch. https://github.com/owen-it/laravel-auditing/tree/v14-dev

Note it's not a v14 branch (yet)!

[MortenDHansen]

Alright. I stabbed at dates, casts and serialization, but i'm having a hard time finding and proving actual issues. It's more or less "just output logic" 🤷

Im dropping the serialization refactor all together 😄