Fix ConnectionString forwarded improperly

[DosenSuppe]

Problem:
Using a user other than postgres causes the connection test to fail.
This occurs because the ConnectionString lacks a Database attribute
during the test.
As a result, the npgsql defaults to using the User attribute as the
database name, which leads to a DatabaseNotFound error as no matching
database exists.

Solution:
Using a custom config for testing the database server's connection status
with database being "postgres".

INFO:
This migh lead to an edge-case where the user in unable to create/ delete
databases if the "postgres" database has been deleted.

Also added missing outline to the save button for the database config.

closes #539

[1nf0rmagician]

While your changes are reasonable, they unfortunately do not tackle the actual issue.

Please reread the issue your are referencing and check the provided code reference as to were the improper/not-usage of the connection string for the database test occurs (which is in the .net code of the command center). The issue is specifically that the connection string is not used at all for the connection test but a default user instead. Disabling the save button will not help here, instead it would actually limit the users options to still modify the config even though we don't detect that the database is available.

Moreover, @MathoMathiasCamara whats your opinion on just disabling the save button while the connection test is running? I would have thought a small loading icon where we later show the connected/disconnected icons would be more user friendly, maybe with a hover text explaining that the connection to the db is currently tested.

Lastly, please rebase your changes after you have modified them onto the release/6 branch and start with #583 instead of this one. Our usual modus operandi is: Target lowest release first, then merge/port upwards 😊

[dbeuchler]

I have tested it with the latest versions again, and it seems to be fine. So there is no need for this whole change. Save should always be possible (after test) because on commissioning it is important to prepare the configs and later use the connection.

The only improvements I have, were already mentioned by @1nf0rmagician: Show some kind of busy indicator and disable save button during test.

[1nf0rmagician]

I have tested it with the latest versions again, and it seems to be fine.

I think this is a misunderstanding @dbeuchler, the issue that this change should solve is also visible on the gif you shared. While you have a correct database connection, assuming your user foo exists in the database server. The funny behaviour we observed was that, the test will show an error even if foo is a valid admin. This is because the connection used for the test has no database specified, for some reason it takes the user name as the database name for the test then. Obviously their won't be a foo database, hence the UI will show an error with the connection config.

[dbeuchler]

I think this is a misunderstanding @dbeuchler, the issue that this change should solve is also visible on the gif you shared. While you have a correct database connection, assuming your user foo exists in the database server. The funny behaviour we observed was that, the test will show an error even if foo is a valid admin. This is because the connection used for the test has no database specified, for some reason it takes the user name as the database name for the test then. Obviously their won't be a foo database, hence the UI will show an error with the connection config.

I have investigated a bit more: You must always specify a database name when connecting to PostgreSQL. There is no supported way to connect without one. This is a PostgreSQL server limitation, not just an Npgsql behavior. It is also hidden in the implementation of Npgsql, if the database-name is empty, the username is used: https://github.com/npgsql/npgsql/blob/main/src/Npgsql/NpgsqlConnection.cs#L445

I also have checked ms-sql and there it is also not possible to connect without a database: For server-level checks, explicitly set Database=master in the connection string. The master database always exists and is suitable for administrative queries.

I don't like this "copy" of the DatabaseConfig<T> but I also have no better idea for dev - in future we could provide a separate method, e.g. CreateTestDatabaseConnectionString which creates a connection-string only for the connection test. And remove bool includeModel from CreateConnection-API - I will create an issue for that.

So thanks for the breakdown, I have learned again something 🤣