-
Notifications
You must be signed in to change notification settings - Fork 53
#646 feat: add seed data for dummy users #647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Was this tested if you got an error? |
e45c656
to
e3abaa3
Compare
48ced72
to
1659c52
Compare
Yep... working through these... tons of |
967094b
to
a888f27
Compare
I noticed this was force-pushed after. Was this still an issue? Don't 100% understand how that could happen! |
a888f27
to
a03a1b7
Compare
The force-push was a result of me making changes, squashing a commit and pushing it up :p |
Rebased... and still err-ing: of note: Full Trace
|
a4c20a3
to
992bb23
Compare
0b9bb53
to
4c4ad92
Compare
I think it's because the .yml is invalid :') |
…ersions and database drivers
…it for psql to be ready
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a seed script for dummy users and wires seed support into the database configuration, local development tools, and documentation.
- Introduces
src/seeds/001_users.ts
to populate initial user data - Extends Knex config and CLI targets (
Makefile
, GitHub Actions) to run seeds - Updates docs (
docs/server-settings.md
,.github/CONTRIBUTING.md
) anddocker-compose.yml
to document and enable environment-driven settings
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/server-settings.ts | Removed stray blank lines |
src/seeds/001_users.ts | New seed script to insert dummy users |
src/database.ts | Registers the seed directory and adds a seed() API |
docs/server-settings.md | Expanded settings table with environment-variable column |
docker-compose.yml | Added env_file reference for .env |
Makefile | Added knex-seed target |
.github/workflows/test.yml | Runs seeds after migrations in matrix CI jobs |
.github/CONTRIBUTING.md | Documented seed usage |
Comments suppressed due to low confidence (3)
src/server-settings.ts:309
- The
%s
placeholder is never supplied an argument here, sorow.setting
won’t appear in the log. Passrow.setting
as a second parameter toconsole.warn
.
console.warn('The setting %s may not be set from the database. We ignored it');
docs/server-settings.md:16
- The new table header includes an “Environment Variable” column, but this row (and some below) has only four columns. Add the appropriate env var or a placeholder (
—
) to keep the table aligned.
| `login.defaultRedirect` | String | `/` | The url that the user will be redirected to after the log in to a12nserver, and no other redirect_uri is provided by the application. It's a good idea to set this to your application URL.|
src/seeds/001_users.ts:35
- The new seed functionality is not covered by any tests. Adding a simple integration test to verify that
seed()
inserts the expected records could prevent regressions.
export async function seed(knex: Knex): Promise<void> {
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets go!
Changes
adds
seeds/
folder and script for opting in to seed dummy usersinclude
env_file
property on docker-compose to read from.env
variable changes a user might make during developmentTesting:
created
[email protected]
, was able to log in with autogenerated password#646
TODO: