fix(ui): Send credentials for existing database deployments#56
Conversation
When using an existing database, the modal now sends a databases array
entry with user-provided credentials instead of only the container
name. Also fixes preview defaults to match backend naming convention
({name}_primary_db / {name}_primary_user) and adds password preview.
Multi-database entries now include password for existing/external modes.
Signed-off-by: nfebe <fenn25.fn@gmail.com>
Code Review SummaryThis PR fixes a bug where credentials were not sent when using an existing database. It also improves UI feedback by showing required markers and a password preview mask. 🚀 Key Improvements
💡 Minor Suggestions
|
Deploying flatrun-ui with
|
| Latest commit: |
ba3451e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://371721d1.flatrun-ui.pages.dev |
| Branch Preview URL: | https://fix-existing-db-credentials.flatrun-ui.pages.dev |
nfebe
force-pushed
the
fix/existing-db-credentials
branch
from
3050eed to
35f7607
Compare
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| service: db.service || undefined, | ||
| existing_container: db.mode === "existing" ? db.existingContainer : undefined, | ||
| external_host: db.mode === "external" ? db.externalHost : undefined, | ||
| external_port: |
There was a problem hiding this comment.
When the mode is 'existing', the externalPort might be provided as a string depending on input handling. However, 'existing' database connections typically rely on internal network naming or the existing_container field rather than an external port. If the backend specifically requires a port for existing containers, ensure it defaults safely.
| external_port: | |
| external_port: | |
| (db.mode === "existing" || db.mode === "external") && db.externalPort | |
| ? parseInt(db.externalPort.toString()) | |
| : undefined, |
| service: "", | ||
| existingContainer: form.database.existingContainer, | ||
| externalHost: "", | ||
| externalPort: form.database.externalPort, |
There was a problem hiding this comment.
Passing form.database.externalPort to the mapper when mode is 'existing' might be unnecessary or confusing if that field isn't visible/relevant for existing container connections in the UI. Consider if this should be explicitly set or if the UI ensures it's cleared.
| externalPort: form.database.externalPort, | |
| externalPort: form.database.mode === "external" ? form.database.externalPort : "", |
Database name, username, and password are now required fields when using an existing database. Step validation blocks progress until all three are filled. Preview shows actual values instead of generated defaults. Signed-off-by: nfebe <fenn25.fn@gmail.com>
nfebe
force-pushed
the
fix/existing-db-credentials
branch
from
35f7607 to
ba3451e
Compare
| (db.mode === "existing" || db.mode === "external") && db.externalPort ? parseInt(db.externalPort) : undefined, | ||
| database_name: db.dbName || undefined, | ||
| username: db.dbUser || undefined, | ||
| password: db.mode === "existing" || db.mode === "external" ? db.dbPassword || undefined : undefined, |
There was a problem hiding this comment.
The mapping logic should also include db.dbPassword for the 'create' mode if the backend requires it, or ensure it is consistently handled. Currently, it only sends password for 'existing' or 'external'. If 'create' generates a password on the backend, this is fine, but if the user provides it during 'create', it will be dropped.
| password: db.mode === "existing" || db.mode === "external" ? db.dbPassword || undefined : undefined, | |
| password: db.dbPassword || undefined, |
1 participant
When using an existing database, the modal now sends a databases array entry with user-provided credentials instead of only the container name. Also fixes preview defaults to match backend naming convention ({name}_primary_db / {name}_primary_user) and adds password preview. Multi-database entries now include password for existing/external modes.