Add case insensitivity to email search + test cases #1042
Conversation
rosalieper
changed the title
| SELECT 1 | ||
| FROM {$dbName}.{$table} | ||
| WHERE user_email = :email | ||
| WHERE LOWER(CONVERT(user_email USING utf8mb4)) = LOWER(:email) |
There was a problem hiding this comment.
This looks interesting, and it's not immediately obvious to me why it is needed. Can we add a comment above this line to explain it?
There was a problem hiding this comment.
Looking at the table schema, the datatype of the user_email field is tinyblob, so I guess it makes sense (or is maybe even necessary) for the comparison: https://github.com/wbstack/api/blob/main/database/mw/new/mw1.43-wbs2.sql#L1005
edit:
gave it a quick test:
MariaDB root@localhost:mwdb_8127892d75> SELECT user_email FROM mwt_022411e763_user WHERE LOWER(CONVERT(user_email USING utf8mb4)) = LOWER('DENA@DENA.DENA') \G
***************************[ 1. row ]***************************
user_email | Dena@DENA.dena
1 row in set
Time: 0.002s
MariaDB root@localhost:mwdb_8127892d75> SELECT user_email FROM mwt_022411e763_user WHERE LOWER(user_email) = LOWER('DENA@DENA.DENA') \G
0 rows in set
Time: 0.002s
| } | ||
|
|
||
| public function scopeWhereEmailInsensitive($query, string $email) { | ||
| return $query->whereRaw('LOWER(email) = ?', [strtolower($email)]); |
There was a problem hiding this comment.
Is this required? Looking at the quick test I did the case-insensitive searching worked for the Platform API users table. I believe this is because the email column uses the utf8mb4_unicode_ci collation. The ci stands for case-insensitive.
We should still keep the tests that prove that case-insensitive search works, though.
P.S. an FYI for the future, strtolower() only converts ASCII alphabetic characters. mb_strtolower() is needed to convert non-ASCII character as well.
|
|
||
| class CheckUserEmailExistTest extends TestCase { | ||
| protected function tearDown(): void { | ||
| User::query()->delete(); |
There was a problem hiding this comment.
| User::query()->delete(); | |
| // delete all users | |
| User::query()->delete(); |
|
can confirm case-insensitive search works in the current state: As Ollie pointed out it would be nice to get the search result string in the output and not the input string, but I can see how this is a bit difficult here because it would need to happen in WikiUserEmailChecker.php |
I suggest we get the case-insensitive stuff merged now, so we can do this GDPR request done before the time runs out, and can think about improvements after. 🙂 |
Bug: T415017