DateTime hardcoded format `Y-m-d H:i:s` drives to "Error converting data type varchar to datetime"

[mikeabbott10]

PHP Version

8.1

CodeIgniter4 Version

4.3

Shield Version

1.0.0-beta3

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

SQL Server 2019

Did you customize Shield?

No

What happened?

My db language is Italian and expects datetime format like 'd-m-Y H:i:s'. When you attempt to login, it tries to insert a row using the format 'Y-m-d H:i:s' by default and that's hardcoded at Shield\Models\LoginModel row 71. https://github.com/codeigniter4/shield/blob/697f570277a4d2205852cacaae4837604b1c4b88/src/Models/LoginModel.php#L71

Steps to Reproduce

Connect to a database with a datetime expected format not equal to 'Y-m-d H:i:s' and try to login

Expected Output

The row gets insterted

Anything else?

I think the format has to be customizable without the need to extend the LoginModel and override the whole recordLoginAttempt just for this.

[kenjis]

Thank you for reporting.

Can SQL Server accept YYYY-MM-DD H:i:s for the column? I believe this is standard notation.

[mikeabbott10]

Yes, it can but my company wants it to not be modified. It's standard notation for some locales, not for all

[mikeabbott10]

The title of this post should be edited. It's not about LoginModel only. It's about every datetime value insertion in db. The solution is to change the hardcoded 'Y-m-d H:i:s' string with a simple datetimeFormat constant saved somewhere. Please tell me where these kind of constants should go so i can solve this and make a PR

[kenjis]

We thought Y-m-d H:i:s is safe to databases regardless of locale. So once we added Time::toDatabase() but removed it before the release. https://github.com/codeigniter4/CodeIgniter4/pull/6509 https://github.com/codeigniter4/CodeIgniter4/pull/6461

Please tell me where these kind of constants should go so i can solve this and make a PR

I'm not sure, but

1. add Time::toDatabase() and make it configurable with Config file for it.
2. add in CodeIgniter\Shield\Config\Auth.

[mikeabbott10]

I've just noticed that codeigniter 4 hardcodes the format too. They call "datetime" the "SQL datetime format" as you can see here . I think it's a mistake because there are many datetime formats a sql server might expect. My server, for example, expects DMY and not YMD as they hardcoded there. So maybe it's more complicated than it seemed.

Relative to codeigniter Shield, the solution would be to overwrite all the fields that BaseModel inserts automatically with that date format (i'm thinking about 'created_at' or 'updated_at' but maybe there are more). But I think it's not worth it at this point since we can move this discussion on CI4 and, after we fixed it on that side, we can make a simple constant adjustment here as we said before

[kenjis]

Yes, the datetime (and date only) format for database is related to data from and to database, and all the CI4 applications. Not only for Shield.

[jozefrebjak]

@mikeabbott10 Why do you need to store datetime in a different format? I'm from Europe as well and I'm using DDMMYY for users in WEB UI and I don't have any problems at all. I'm using a simple app_date helper function from Bonfire2 to translate datetime columns to a format I want and it's configurable.

I don't understand why somebody is changing the language of the database or Linux to the native one. I'm from Slovakia and I'm fine with the default settings of DB and CodeIgniter at all.

As @kenjis noted, YYYYY-MM-DD H:i:s is a standard. For me, it's a bad decision in your company to use something else.

@mikeabbott10 Don't take it the wrong way, just an opinion.

[mikeabbott10]

YYYY-MM-DD H:i:s is a standard. For me, it's a bad decision in your company to use something else.

Not my decision and there's no turning back but as far as i know, since choosing a date format is legit a framework like this should also consider this possibility.

[datamweb]

1. add Time::toDatabase() and make it configurable with Config file for it.

I think this issue should be resolved in the framework, it's not just for Shield and anyone else may have this problem in their own code.

it's a bad decision in your company to use something else.

I agree, but I don't think there should be a limit to any format. I think it doesn't matter why the user uses a different format, the important thing is the coverage of the framework and the ability to make orders, we should allow the user to have the ability to make orders.

[datamweb]

@mikeabbott10 Kenji made some changes to solve this problem, if you can check from the develop branch.

[kenjis]

To fix this issue, we need CI v4.5.0 (https://github.com/codeigniter4/CodeIgniter4/pull/8538), and Shield develop branch.

[datamweb]

Oh, I thought the version released today was 4.5. Sorry.

[kenjis]

@mikeabbott10 This bug should be resolved in CI 4.5.1 and Shield 1.0.3.