"New DB Connection" UI specs review

[Sean Colsen]

Kriti, Brent, Pavish, Ghislaine, and Mukesh,

After having a number of 1/1 conversations with each of you about the DB connections behavior and UI, I’ve made some progress on our next design iteration and have written the following specs that I’d like you each to review:

https://wiki.mathesar.org/design/specs/new-db-connection-form/

Please read and respond to this email thread with questions, concerns, proposed changes, or approval.

My specs doc goes into some important detail about the user-facing behavior when creating new database connections, but it does not address the manner in which users arrive at this form in the first place. Ghislaine is working on the design for the “Home page” which lists database connections and allows CRUD on them. She and I will work together to figure out how best to incorporate my “New Database Connection” form into her home page. We are considering various approaches including a modal, an inline form, and a separate page. Hammering down exactly how we want this “New Database Connection” form to behave (and thus, how big the form really needs to be) will no doubt help us envision the best way to integrate it into the rest of Mathesar.

For the sake of comparison, in Rajat’s PR this “New Database Connection” form is currently implemented as shown below. Consider this to be the starting point, from which we’re working to improve UX:

​

[Kriti Godey]

I think this spec is great, and does a terrific job of simplifying the complexities of all the DB connection choices for the user. Consider it approved.

My only minor quibble is that I don't like the UX pattern of using radio buttons how we're using them in the spec. The radio buttons appear like they're part of the same form, but they're actually being used to show different forms depending on the choices. e.g. "User type", "User name", and "Password" are all items in the form, but "User type" is only used in the UI (to figure out what form to show) but the rest of it is saved. I worry that we're misrepresenting the kinds of data that Mathesar stores behind the scenes. I think all of this would be solved by a different UX pattern instead of radio buttons, but I don't have a suggestion – Ghislaine would be better to consult here.

I'm not sure if I'm explaining my quibble well enough, let me know if it doesn't make sense.

[Sean Colsen]

Thanks for the feedback, Kriti. I totally agree with your quibble, and this was one of the points that I struggled with during the design. I chose to use this pattern because it's a pattern I've seen employed on other web forms (though admittedly, I don't have any clear examples to easily reference). I actually spent some time lamenting the fact that we don't have better web primitives for dealing with this very problem. I'm definitely open to suggestions at how to re-structure that aspect of the form!

[Ghislaine Guerin]

Thanks Sean. The spec looks great, and I agree that it is a very simple solution. We can discuss the UI aspects and integration with the "home page" design tomorrow during our call. 

[Pavish Kumar Ramani Gopal]

Thanks sean. The spec looks mostly good, I have a few concerns.

1. The terminology 'Credentials' is ambiguous and confusing. The first time I looked at the images without reading the spec, it took me a second to realize that it meant the `Database server details`, which include user credentials.

• I would recommend renaming it to `Database server`, or `Database server credentials`.

2. Inorder to create a new database, the user needs to be the superuser or must have `CREATEDB` privilege. Similarly, in order to create new users, the user needs to be the superuser or must have `CREATEROLE` privilege.

• Both of these need to be made clear in the UI.

3. I assume, we are no longer using `MATHESAR_DATABASES` env variable for user DBs (based on previous email threads). The UI where it shows `Credentials to use: mathesar@localhost:5432 (source: .env file)` is confusing.

• In this scenario, it seems to refer to the Internal DB (and not user DBs), I would recommend calling it `Internal Database server` instead of `(source: .env file)`.
• For most quick installations, we don't necessarily have the `.env` file. Docker can pass env variables directly while running the commands, so can other installations like linux packages.

[Pavish Kumar Ramani Gopal]

I missed a point in my last email.

4. In certain installation types like the linux packages, the internal database is not a Postgres database, it could be a SQLite DB. I'm not certain about how far this is implemented (or if the decision was changed), this is what I remember from reading the Installation spec a while back. In this case, using the same Internal DB's Database server would not work.

The UX needs to account for all of these scenarios.

[Kriti Godey]

4. In certain installation types like the linux packages, the internal database is not a Postgres database, it could be a SQLite DB. I'm not certain about how far this is implemented (or if the decision was changed), this is what I remember from reading the Installation spec a while back. In this case, using the same Internal DB's Database server would not work.

 Isn't this already accounted for in the "The credentials info is pre-filled according to the following logic:" section?

[Pavish Kumar Ramani Gopal]

>  Isn't this already accounted for in the "The credentials info is pre-filled according to the following logic:" section?

Ah, yes. It seems to be covered, we're sorted out on question 4.

[Ghislaine Guerin]

Regarding the radio buttons: Sean and I discussed it. I've pointed out other areas in our UI where we already use radio buttons in this way, like in the 'Add Constraints' form to toggle the visibility of additional form inputs. For consistency, we've decided to reuse this pattern.

[Kriti Godey]

Okay, sounds good.

I think we should reassess this pattern throughout our UI at some point, although I agree it is not an immediate priority.

[Sean Colsen]

To address the concerns that Pavish raised earlier within this thread, he and I had a call yesterday, and I also checked in with Brent briefly with some follow-up questions.

Now I’ve updated the spec with some small adjustments to language in order to resolve Pavish’s concerns.

Here’s a side-by-side comparison, highlighting the areas that I changed:

(Pavish: you’ll notice that I went with the simpler of the options we discussed — help text instead of dynamic “disabled” status. This is because Brent and I decided it would not be worth spending the time now to forward permissions info to the front end.)

Notably, the “Create this database…” checkbox field now has help text underneath it to inform users about the extra permission required. In version A of the form (the simplest), we know the user name and so we put it in the help text dynamically. In version B, we omit it. And in version C we include it. This is most important in version C because the form concerns itself with two different postgres users — the “bootstrapping user” (as I’ll call it), and the “new user”. Brent made it clear to me that we actually want to create the database using the bootstrapping user, not the new user. For this reason, we want the help text to explicitly mention that user name.

​

[Pavish Kumar Ramani Gopal]

Thanks Sean, sounds good to me.

[Kriti Godey]

Looks good to me. I find the phrasing "in order to create it" a little awkward, maybe change the text to "This operation requires the [mathesar] database user to have CREATEDB privileges."?

[Brent Moran]

Sorry this took so long; I was trying to be thorough. Overall, this looks pretty good. I do have some notes:

.env file credentials string display

I don't think we should make assumptions about the form of the connection string there. There are other connection and auth methods besides bare username and password that can be specified with the connection string, and avoiding assumptions on its form would be a (nearly free) way to support many other types of auth for that database.

The back end seems over-specified

I can kind of see the specification that the .env file exists and is non-editable through the UI as relevant, but the rest (specifying the model definition) seems out-of-scope and constraining.

Credentials need to include a Database

To be clear, you can't actually connect to a database server directly. You have to connect to a database on the server. We're already including the host and port. The database is also required to fully identify a connection to the database. For me, the term "credentials" implies just user and password, but once we're specifying 4/5 parts of the string, why not specify the 5th part? This is relevant to the spec where credentials are shown, e.g.

Admittedly, in that case, we could store the DB to connect to and simply avoid showing it to the user. However, in the next example, including the database is necessary to fully specify a connection:

It feels inconsistent to me to bounce back-and-forth on that concept.

Regarding deduplication of credentials

For .env file vs. internal storage credentials, I think we should make an effort to logically separate these in the user's mind. This means in "Credentials to use", we may show the same user name and password if one is derived from the .env file and the other from a stored model instance. This lets us more clearly delineate which credentials were input through the UI and which weren't. In cases where those credentials are misconfigured, this hint would be quite relevant to a user trying to fix the issue. Further, I think simply showing both in the drop-down if we have the same username for the same database, one in the .env file and the other in internal storage is acceptable, since

• That would be an extreme edge-case, and
• Probably should look like a problem to the user (since it probably is).

The reason that's a problem is that having those credentials (including database, as I noted above) in both places implies they're connecting to the internal Django DB as a user tables database. While technically possible, it's unlikely that this would be a good idea for most users.

Also, see my note above about the .env file connection string form. 

However, I completely agree that we should deduplicate the credentials in internal storage at least as shown in the UI. For that, I'm ambivalent w.r.t. whether we should actually deduplicate them in storage:

• There's certainly a use case for keeping old credentials (rollback).
• This may not be a convincing enough use-case to justify the increased complexity of deduplicating through some means other than improved data model.

If we want to de-duplicate in storage, it's not too hard.

Prefilling info

I suggest "mathesar" or something similar for the prefilled user database name. "db" is pretty nondescript, and won't help a user connecting via a different client.

Manually entering new credentials

As noted above, we need to have them specify a database on the server to connect. You have to connect to a database to create a new database. Admittedly, this is a bit awkward w.r.t. the creation of a new database option as presented.

Schemas to install

Nice presentation. We should consider whether it's worth the effort to pull this info from the service layer, rather than hard-coding it. In that case, we should avoid the number "three" in the copy. We might also consider a dropdown for the optional schemas, since we could add more of them.

Closing thoughts

Depending how "future-proof" we want this to be, we should consider:

• making most fields optional,
  
• adding a "connection options" field, and
  
• Gracefully handling under-specified connections.

This combination would let users specify other connection and authorization types easily, and also gives admins some flexibility w.r.t. storing some or part of the relevant credentials as a .pgpass file. The value of the former is obvious, and the latter would let an admin maintain and update credentials in bulk without having to go through the Mathesar UI.

Brent

[Kriti Godey]

Sean – it looks like you had a call with Brent to work through this today, I assume you'll send an updated spec.

[Sean Colsen]

I’ve updated the “New DB Connection” UI specs again in response to critique from Kriti and Brent.

At this point I’ll be moving forward with implementation. We can still modify the spec, but I’m assuming that any further concerns will be small. Don’t hesitate to raise more concerns, but please know that any concerns which substantially alter the structure of this form will incur additional costs now that implementation will shortly be underway.

• Kriti, I made the small change you suggested to the help text. However, note that I wrote “PostgreSQL user” instead of “database user”. This is because we also decided to change the database name to “mathesar”, and I felt that using “database user” opened the potential for mis-reading the help text while skimming because it juxtaposed “mathesar” with “database”. This could be splitting hair though. Hopefully this is not a big deal. Help text is always easy to adjust later, including during PR review, so I’d like to move forward with this for now.

• I made a number of small changes to the spec after talking with Brent. You can see a diff of my text changes here.

• Here’s a visual diff of the changes I made to the mockups. If you read over the text diff above, the rationale for all these visual changes should hopefully be apparent. I welcome questions and critique, but please read the text diff first, as it explains a number of things.

• 

  

  

​

[Kriti Godey]

Looks good to me.