FIX: TYPEORM postgresql (...Emails) syntax bad handling in #1205

[hexaltation]

Context

PR #1205 introduced a regression with postgreSQL.
The present PR addresses it.

Proposed solution

As a workaround of TYPEORM postgresql (...Emails) syntax handling. The code now returns an empty Array if there is no emails given to get existing users.

Related issues

#1005

Has this been tested?

yes locally with a psql database.

[fflorent]

Thanks!

Tests are failing, these are due to a regression already present on main.

That's another subject, but what worries me is that it may be a shadow upgrade of some component that breaks stuffs. The first commit which shows the problem is unrelated to the code:
0b0ca2c

Anyway, that looks good to me!

[berhalak]

Hi @hexaltation,

Can you add a test case to the ./test/gen-server/lib/homedb/UsersManager.ts ?

[fflorent]

Hi @berhalak

Can you add a test case to the ./test/gen-server/lib/homedb/UsersManager.ts ?

The problem is that we may add the test case, Sqlite will recognize the IN () clause whereas Postgresql won't. And this test file only test using Sqlite. So the test won't detect any regression.

I attempted to make the tests run using Postgresql in #1278 but many tests already fail.

[hexaltation]

Hello @berhalak,

Can you add a test case to the ./test/gen-server/lib/homedb/UsersManager.ts ?

Yes I can do it, nevertheless as @fflorent says the test will never fail as long as the CI only tests sqlite.
Does your fabrikator toolchain tests the code against postgre?
It could be a great improvement to add it to the github CI.

[hexaltation]

And I realize that the new function getUsersByLogins() is never tested.
So for sure I add tests for it.

[berhalak]

Hello @berhalak,

Can you add a test case to the ./test/gen-server/lib/homedb/UsersManager.ts ?

Yes I can do it, nevertheless as @fflorent says the test will never fail as long as the CI only tests sqlite. Does your fabrikator toolchain tests the code against postgre? It could be a great improvement to add it to the github CI.

Yes, our internal CI should run this test against postgres.

[fflorent]

Few comments, globally LGTM

[berhalak]

Thank you!

[jordigh]

I have rebased the branch to re-run the CI on main after #1283

[hexaltation]

Thank you @berhalak & @jordigh