-
Notifications
You must be signed in to change notification settings - Fork 128
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
DateTime hardcoded format Y-m-d H:i:s
drives to "Error converting data type varchar to datetime"
#608
Comments
Thank you for reporting. Can SQL Server accept |
Yes, it can but my company wants it to not be modified. It's standard notation for some locales, not for all |
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 |
We thought
I'm not sure, but
|
I've just noticed that codeigniter 4 hardcodes the format too. They call "datetime" the "SQL datetime format" as you can see here . 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 |
Yes, the datetime (and date only) format for database is related to data from and to database, |
Y-m-d H:i:s
drives to "Error converting data type varchar to datetime"
@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 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. |
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. |
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.
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. |
@mikeabbott10 Kenji made some changes to solve this problem, if you can check from the develop branch. |
To fix this issue, we need CI v4.5.0 (codeigniter4/CodeIgniter4#8538), and Shield |
Oh, I thought the version released today was 4.5. Sorry. |
@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.
shield/src/Models/LoginModel.php
Line 71 in 697f570
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.
The text was updated successfully, but these errors were encountered: