-
Notifications
You must be signed in to change notification settings - Fork 243
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
EZP-19123: Make ezuser.login case in-sensitive across databases #1239
Conversation
Without this PR, you can create new user in legacy admin with ez paltform 16.02, please merge. |
@truffo The problem is not to merge it. The code needs to be reviewed. It seems that you tried it which is great. Did everything work as expected, etc ? |
It is also unclear if legacy master should end up as as a 5.x like (e.g. 5.5 or 2014.13) version (meaning no merge as it will then break compatible with 5.x kernels), or if it should be a 6.x version (which will be somewhat misleading since 6.x kernel has broken several things making it less compatible with legacy, and this will most likely continue) |
* | ||
* @return string | ||
*/ | ||
public function normalizeLogin( $login ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be static as it is used above as self::normalizeLogin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Fixed.
|
||
if ( $attr === 'login' ) | ||
{ | ||
parent::setAttribute( 'login_normalized', $this->normalizeLogin( $val ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should call static self::normalizeLogin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, fixed :)
With ezsystems/ezpublish-kernel@bd257ed will this needed any more? |
@wizhippo Probably not, but as per ezsystems/ezpublish-kernel#1682 (comment), support for postgresql should be probably adapted here. |
Legacy counterpart of ezsystems/ezpublish-kernel#1561
Tests: Manual only, by creating a user in legacy and logging in both in legacy and new stack.