2022-Spring-NYU-DevOps-Shopcarts / shopcarts

Shopcarts squad.
Apache License 2.0
2 stars 2 forks source link

enable behave and implement button for creating empty shopcarts #92

Closed kdokm closed 2 years ago

kdokm commented 2 years ago

As the title

Besides: Port is changed to 8000 as prof said. Some lines of the acceptance criteria need to be change because they don't work well with bdd.

kdokm commented 2 years ago

For anyone who want to reference the frontend change: just see index.html and rest_api.js

kdokm commented 2 years ago

Thanks for the extensive work Yuwen! I just have a minor doubt about using parseInt for user_id, what if the input user_id is of type float, e.g. user_id = 3.5, what will happen in this case? I am thinking that would it be better if we use a flash message to indicate that the input should be an integer?

It will return the error message we specified in routes if it catches this error. If you want to specify that it should be an integer, maybe we should change the error message in routes?

codecov-commenter commented 2 years ago

Codecov Report

Merging #92 (ca30439) into main (0456dee) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #92   +/-   ##
=======================================
  Coverage   98.94%   98.94%           
=======================================
  Files           7        7           
  Lines         380      380           
=======================================
  Hits          376      376           
  Misses          4        4           
Impacted Files Coverage Δ
service/routes.py 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0456dee...ca30439. Read the comment docs.

kdokm commented 2 years ago

Thank you! I would add flash message indicating invalid input later

It will show "invalid user id" now but not integer hint. If you want, do you think it's better to specify here or in the routes?

yjjw commented 2 years ago

It will return the error message we specified in routes if it catches this error. If you want to specify that it should be an integer, maybe we should change the error message in routes?

I see! This makes sense, we have the check in routes already, so we do not need to check it again in the js file. Thanks!

yjjw commented 2 years ago

I agree that we can leave the error message in the routes

yjjw commented 2 years ago

Thank you! I would add flash message indicating invalid input later

It will show "invalid user id" now but not integer hint. If you want, do you think it's better to specify here or in the routes?

Actually, wouldn't it be nice if we have the error in flash message as well? The status part does not make the message explicit for users. What do you think? We can rephrase the message.

To be honest, status in UI is a little weird if we want to make the app for users.. I guess it is copied from the template, easier for us to spot the problems, so we should not touch it for now anyways.

TimothyXu commented 2 years ago

Thank you! I would add flash message indicating invalid input later

It will show "invalid user id" now but not integer hint. If you want, do you think it's better to specify here or in the routes?

Actually, wouldn't it be nice if we have the error in flash message as well? The status part does not make the message explicit for users. What do you think? We can rephrase the message.

To be honest, status in UI is a little weird if we want to make the app for users.. I guess it is copied from the template, easier for us to spot the problems, so we should not touch it for now anyways.

The professor may have said this UI is intended for a system administrator not a user... not sure if I remember correctly about that. And thank you Yuwen this is very helpful for us to learn from