Skip to content
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

Feat: Seeding database #57

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

JoaoAlem
Copy link
Contributor

@JoaoAlem JoaoAlem commented Nov 7, 2023

This PR solves #8

@JoaoAlem
Copy link
Contributor Author

JoaoAlem commented Nov 7, 2023

Hello everyone.

This PR adds the code and scripts to seed the database.
It's still ins draft, because it's not done yet.
For everyone who wants to contribute in this PR, you can find a seed.ts file in databases folder.
I left a lot of TODO in the code, so i can guarantee you won't be lost in the code 😄

@JoaoAlem JoaoAlem marked this pull request as ready for review November 15, 2023 03:21
@JoaoAlem
Copy link
Contributor Author

Now, this PR is ready to merge.
Feel free to make any critics or do fixes, or ask me to do fixes in the code.

@@ -10,14 +10,16 @@
"db:push": "pnpm dotenv drizzle-kit push:pg",
"dev": "pnpm dotenv drizzle-kit studio",
"format": "pnpm prettier . --check --ignore-unknown",
"format:write": "pnpm format --writer"
"format:write": "pnpm format --writer",
"db:seed": "node --loader esbuild-register/loader -r esbuild-register ./seed.ts"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I highly recommend tsx instead of a custom loader for Node.js. In a perfect world, TypeScript would be treated as a first class citizen and automatically transpiled at runtime (both Deno and Bun achieve this), but Node.js precedes TypeScript by I few years, so these and other module related annoyances are quite common.

Note
Remember to add tsx as a devDependency on the @open-visit/database workspace.

Suggested change
"db:seed": "node --loader esbuild-register/loader -r esbuild-register ./seed.ts"
"db:seed": "pnpm dotenv tsx ./seed.ts"

database/seed.ts Outdated
Comment on lines 15 to 21
const employeeUserData: (typeof user.$inferInsert)[] = []
const condominiumUserData: (typeof user.$inferInsert)[] = []
const condominiumData: (typeof condominium.$inferInsert)[] = []
const companyData: (typeof company.$inferInsert)[] = []
const departmentData: (typeof department.$inferInsert)[] = []
const employeeData: (typeof employee.$inferInsert)[] = []
const schedulingData: (typeof scheduling.$inferInsert)[] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As counterintuitive as it seems, $inferSelect is actually preferable for a better representation of the type annotations here.

This is because, from TypeScript's perspective, if in Drizzle schema.ts we declare that those primary keys are notNull() but also defaultRandom(), the type inference understands that this value can be left as undefined, because it will be autogenerated by the database engine, which is technically a correct inference.

In the seeding script case however, we need to references to these ID's directly because we're sequentially creating relational data, so we need to reference created ID's in subsequent objects.

$inferSelect will always have id: string instead of id?: string | undefined because, from a selection perspective, an id will never be undefined (it was either defined at runtime, or by the database engine).

Suggested change
const employeeUserData: (typeof user.$inferInsert)[] = []
const condominiumUserData: (typeof user.$inferInsert)[] = []
const condominiumData: (typeof condominium.$inferInsert)[] = []
const companyData: (typeof company.$inferInsert)[] = []
const departmentData: (typeof department.$inferInsert)[] = []
const employeeData: (typeof employee.$inferInsert)[] = []
const schedulingData: (typeof scheduling.$inferInsert)[] = []
const employeeUserData: (typeof user.$inferSelect)[] = []
const condominiumUserData: (typeof user.$inferSelect)[] = []
const condominiumData: (typeof condominium.$inferSelect)[] = []
const companyData: (typeof company.$inferSelect)[] = []
const departmentData: (typeof department.$inferSelect)[] = []
const employeeData: (typeof employee.$inferSelect)[] = []
const schedulingData: (typeof scheduling.$inferSelect)[] = []

database/seed.ts Outdated
Comment on lines 6 to 7
import * as dotenv from "dotenv";
dotenv.config({ path: "./.env" });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be omitted, since we're already injecting the environment variable trough the process spawn with the package.json script.

Suggested change
import * as dotenv from "dotenv";
dotenv.config({ path: "./.env" });

database/seed.ts Outdated
departmentId: department.id,
name: faker.person.fullName(),
email: faker.internet.email(),
phoneNumber: faker.phone.number("55###########"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

faker-js/faker is currently in the process of deprecating the usage of phone.number(<string_pattern>). They instead recommend using either string.numeric, or a Regular Expression.

We don't really care much about the validity of those phone numbers for test data, so I believe prefixing the country code and the ninth digit from Brazil's phone numbers with subsequent 8 character long string of random numbers should be enough.

Suggested change
phoneNumber: faker.phone.number("55###########"),
phoneNumber: `559${faker.string.numeric(8)}}`,

@ernestoresende
Copy link
Member

@JoaoAlem Excellent work with this 🚀

Left a few revisions above that hopefully I'll agree with. Let me know if you need any further help 😄

use inferSelect instead of inferInsert
remove dotenv import
use faker.js new format
@JoaoAlem JoaoAlem changed the title Database seed Feat: Seeding database Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants