feat: Allow user to select a different database type #124
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deloristhompson
added
the
work in progress
…' into deloris/issue47/allow-user-to-select-a-different-database-type
…se-type' of https://github.com/echobind/bisonapp into deloris/issue47/allow-user-to-select-a-different-database-type
deloristhompson
changed the title
deloristhompson
changed the title
deloristhompson
changed the title
cball
reviewed
Jan 8, 2021
| break; | ||
| default: 5432; | ||
| break; | ||
| } |
There was a problem hiding this comment.
If we make the suggested change above, we can change to:
{
default: (answers) =>. {
const database = SUPPORTED_DATABASES.find(d => d.value === answers.db.dev.databaseType);
if (!database) return SUPPORTED_DATABASES.POSTGRES.value;
return database.value;
}
}I'm not sure the default port as postgres actually buys us anything, but it's good to be defensive about it 👍
| choices: [ | ||
| { name: "Postgres", value: "postgres" }, | ||
| { name: "MySQL", value: "MySQL" }, | ||
| { name: "SQLite", value: "SQLite" }, |
There was a problem hiding this comment.
Thoughts on adding an array of supported databases as a constant? I haven't tested this, but the following should work:
const SUPPORTED_DATABASES = {
postgres: { label: "Postgres", defaultPort: 5432 },
mysql: { label: "MySQL", defaultPort: 3306 },
sqlite: { label: "SQLite", defaultPort: "file:./dev.db" },
}
// use it to create the choices
{
choices: Object.values(SUPPORTED_DATABASES)
}There was a problem hiding this comment.
It changes some of the methods, but instead of just checking against postgres everywhere we can keep the information in one place.
| type: "confirm", | ||
| message: "Visit https://github.com/echobind/bisonapp/blob/main/docs/postgres.md for intructions to setup a new database.\n\bContinue? Press [Enter] for YES", | ||
| description: "Provide link to help install Postgres", | ||
| when: (answers) => answers.db.dev.localDatabasePrompt == false && answers.db.dev.databaseType == "postgres", |
There was a problem hiding this comment.
Suggested change
| when: (answers) => answers.db.dev.localDatabasePrompt == false && answers.db.dev.databaseType == "postgres", | |
| when: (answers) => answers.db.dev.localDatabasePrompt === false && answers.db.dev.databaseType === SUPPORTED_DATABASES.POSTGRES.value, |
|
we'll rework this with the CLI rework. |
cball
deleted the
deloris/issue47/allow-user-to-select-a-different-database-type
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Screenshots
List of Database Types
User.toggles.between.database.types.mp4
Database Port set Based on Selected Type
Checklist
-[X] Requires adding MySQL and SQLite to app creation
Fixes: Issue128