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

Created Product and Inventory #19

Closed
wants to merge 22 commits into from
Closed

Created Product and Inventory #19

wants to merge 22 commits into from

Conversation

KiranJoji
Copy link

I created Product and Inventory models and implemented CRUD operations. I also ensured that inventories were created upon the creation of products.
For now, I set all default values to 0 or empty strings.

Copy link
Contributor

@Austin2Shih Austin2Shih left a comment

Choose a reason for hiding this comment

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

This looks really good! As long as you tested the API then everything should be fine since the code itself looks good! Just change the price stuff in inventory to be Floats. Once everything is tested also remove the code for Product. The Product code is good and I wish I could merge it in but I unfortunately assigned this to someone else since I needed more work to distribute at the time. I'll try to avoid having duplicate code written in the future.

id String @id @unique
product Product @relation("product_inventory", fields: [id], references: [id], onDelete: Cascade)
available_quantity Int
cost_of_production Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Costs might want to be floats or whatever is closest

prisma/schema.prisma Show resolved Hide resolved

export default [User, Playlist, Song];
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for removing the template

… does not work (update on products for servies creates a user) or files are named improperly (Products instead of product)
@KiranJoji
Copy link
Author

I adjusted the code but realized I couldn't remove the product code because the current product code was off. The names of the files are wrong and the update for products is creating a user.

Copy link
Contributor

@brandonw504 brandonw504 left a comment

Choose a reason for hiding this comment

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

Everything looks like it's good, except the small change about ensuring that when you query an Inventory you can see the product property. Just make sure you run the migration to add inventories, since you've only done it for products so far. Maybe rename the migrations to be more descriptive, something like init_products.

import Inventories from "../services/Inventories";

const resolvers = {
Inventory: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if you've implemented the fix I mentioned but just make sure you fill out the product field so that when you query an inventory you're able to also see the product field. It's kind of like what you did in Product.ts.
Product: { inventory: ({ id }) => Inventories.find({ id }), },

Copy link
Contributor

@brandonw504 brandonw504 left a comment

Choose a reason for hiding this comment

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

Tag looks mostly good, there should just be a few changes to the data model that I noted down. Once you're done with that just run the schema generation (that's how I saw the thing with the tag name vs. ID) and the migration. Also, when you think you're fully ready to merge, please merge main into your branch to resolve the conflicts. Good work!

product_id String @unique
product Product @relation("product_tag", fields: [product_id], references: [id], onDelete: Cascade)
tag_name String @unique
tag Tag @relation("tag_product", fields: [tag_id], references: [id], onDelete: Cascade)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might have meant to use tag_name instead of tag_id in your relation (or the other way around).

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, after looking around more I realized you were probably going for referencing by name rather than ID, so you can just change tag_id to tag_name and id to name.

model ProductToTag {
product_id String @unique
product Product @relation("product_tag", fields: [product_id], references: [id], onDelete: Cascade)
tag_name String @unique
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that both product_id and tag_name shouldn't be unique because we might have products with multiple tags and tags with multiple products in this table.

fixing the undefined error
@KiranJoji
Copy link
Author

There is some issue with passing the product_id and tag_name to different queries with ProductToTag. For example, if you try to query Product and Tag through manyPTTs (finding all ProductToTags), the find functions for Product and Tag are passed undefined values. I've spent a long time trying to figure out why but I can't figure it out. It may be connected to how, when creating or deleting ProductToTag, you can't use const { productId, tagName, } = input; when its fine for other model creates. It thinks productId and tagName is undefined, but it works if you use product_id and tag_name (which breaks the lint check since it is not camel case).

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.

3 participants