larkinds / recurse-randomice

A webstore for a hypothetical ice cream store that combines adjectives and nouns for silly flavors
MIT License
4 stars 2 forks source link

Toppings and flavor endpoints #64

Closed eric-barch closed 1 year ago

eric-barch commented 1 year ago

Addresses Issue #39

larkinds commented 1 year ago

Addresses Issue #39

  • Add MongoDB schema and create router for Toppings
  • Populate Toppings collection
  • Create GET endpoint to retrieve list of available toppings

    • Sample request: localhost:3003/api/icecreams/
    • Sample response:
      [
      {
      "_id": "64fb6f6912f1493fb63d0ddf",
      "name": "seventh",
      "isUserGenerated": true,
      "userId": "dgfbnogrb",
      "__v": 0
      },
      {
      "_id": "64fb733ee3e373b0b6126544",
      "name": "abominable snowman",
      "isUserGenerated": true,
      "userId": "leewiz",
      "__v": 0
      },
      {
      "_id": "6500bbaabd5bd2af1ef72c2b",
      "name": "jumbo staircase",
      "isUserGenerated": true,
      "userId": "leewiz",
      "__v": 0
      }
      ]
  • Create GET endpoint to retrieve x number of flavor suggestions

    • Sample request: localhost:3003/api/icecreams/suggestion/2
    • Response returns 2 ice creams:
      [
      {
      "_id": "6500bbaabd5bd2af1ef72c2b",
      "name": "jumbo staircase",
      "isUserGenerated": true,
      "userId": "leewiz",
      "__v": 0
      },
      {
      "_id": "64fb733ee3e373b0b6126544",
      "name": "abominable snowman",
      "isUserGenerated": true,
      "userId": "leewiz",
      "__v": 0
      }
      ]
    • Sample request asking for more ice creams than exist: localhost:3003/api/icecreams/suggestion/7
    • Response returns all ice creams in database:
      [
      {
       "_id": "64fb6f6912f1493fb63d0ddf",
       "name": "seventh",
       "isUserGenerated": true,
       "userId": "dgfbnogrb",
       "__v": 0
      },
      {
       "_id": "64fb733ee3e373b0b6126544",
       "name": "abominable snowman",
       "isUserGenerated": true,
       "userId": "leewiz",
       "__v": 0
      },
      {
       "_id": "6500bbaabd5bd2af1ef72c2b",
       "name": "jumbo staircase",
       "isUserGenerated": true,
       "userId": "leewiz",
       "__v": 0
      }
      ]

This is a gorgeous pull request comment, great job Eric & Lee! One comment: the first get request says that it's for toppings, but the sample request and response say icecream.

eric-barch commented 1 year ago

Thank you! Got so focused on the formatting that I messed up the content 😅 I updated the comment and also made a few further cleanup changes to the code. Let us know of any further comments!

eric-barch commented 1 year ago

Great work, @eric-barch and @Leewiz! Very clean and concise code! One thing I wonder if we need - is the orderItemId array field inside the Toppings schema to link to orders these toppings were used in. I don't see any other way to integrate toppings for now, but please let me know if you think we need this at all? If yes, we might want to add the fields here and in the order item schema in a separate PR.

Thanks! And I'm not sure! Usually with a one-to-many relationship like one Topping to many Icecream/OrderItems, I would put a field in the "many" record that references the "one" record. So each Icecream and/or OrderItem would have a single toppingId field, rather than Topping having an array of many orderItemIds.

It seems like it would be more common to have an Icecream or OrderItem and want to get its Topping than to have a Topping and want to get a list of all of its OrderItems. But if we anticipate doing the latter a lot then we probably should add an orderItemIds array.

That's just my first thought and curious what others think. I also feel like I'm not 100% clear on the schema yet and could be misunderstanding.

Leewiz commented 1 year ago

i don't remember officially planning a schema for toppings so we went with the simplest approach of getting toppings into the shop. if what was implemented is missing a field required for core features, i'm all for updating it for this PR. otherwise, maybe we hold off until we have a MVP before adding to it.

larkinds commented 1 year ago

I

Great work, @eric-barch and @Leewiz! Very clean and concise code! One thing I wonder if we need - is the orderItemId array field inside the Toppings schema to link to orders these toppings were used in. I don't see any other way to integrate toppings for now, but please let me know if you think we need this at all? If yes, we might want to add the fields here and in the order item schema in a separate PR.

Thanks! And I'm not sure! Usually with a one-to-many relationship like one Topping to many Icecream/OrderItems, I would put a field in the "many" record that references the "one" record. So each Icecream and/or OrderItem would have a single toppingId field, rather than Topping having an array of many orderItemIds.

It seems like it would be more common to have an Icecream or OrderItem and want to get its Topping than to have a Topping and want to get a list of all of its OrderItems. But if we anticipate doing the latter a lot then we probably should add an orderItemIds array.

That's just my first thought and curious what others think. I also feel like I'm not 100% clear on the schema yet and could be misunderstanding.

I agree - seems like

i don't remember officially planning a schema for toppings so we went with the simplest approach of getting toppings into the shop. if what was implemented is missing a field required for core features, i'm all for updating it for this PR. otherwise, maybe we hold off until we have a MVP before adding to it.

Totally -

i don't remember officially planning a schema for toppings so we went with the simplest approach of getting toppings into the shop. if what was implemented is missing a field required for core features, i'm all for updating it for this PR. otherwise, maybe we hold off until we have a MVP before adding to it.

Agreed - it's not in the schema. I feel fine about holding off on this update for the sake of the MVP - we don't care about tracking toppings at this point, since they don't show up in the hall of fame page anyways

pearlescence-m commented 1 year ago

Thanks! And I'm not sure! Usually with a one-to-many relationship like one Topping to many Icecream/OrderItems, I would put a field in the "many" record that references the "one" record. So each Icecream and/or OrderItem would have a single toppingId field, rather than Topping having an array of many orderItemIds.

It seems like it would be more common to have an Icecream or OrderItem and want to get its Topping than to have a Topping and want to get a list of all of its OrderItems. But if we anticipate doing the latter a lot then we probably should add an orderItemIds array.

Makes sense! In MongoDB terms, we could have both fields and make them update each other if necessary (which totally might not be). I also agree with Lee's point that for the sake of MVP we can skip this integration for now :)

eric-barch commented 1 year ago

This all looks great, and works perfectly.

One note from me is that the topping's controller doesn't look tested - could you add a file for that with an api route in the requests folder?

thanks! we added a test for toppings in the requests directory

eric-barch commented 1 year ago

I think this is ready to merge into staging unless anyone finds new issues.