2022-Spring-NYU-DevOps-Shopcarts / shopcarts

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

Add Action Route for Resume for Purchase #121

Closed lemadmax closed 2 years ago

codecov-commenter commented 2 years ago

Codecov Report

Merging #121 (0f48a67) into main (812133e) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
- Coverage   99.10%   99.09%   -0.02%     
==========================================
  Files           7        7              
  Lines         447      440       -7     
==========================================
- Hits          443      436       -7     
  Misses          4        4              
Impacted Files Coverage Δ
service/utils/error_handlers.py 100.00% <ø> (ø)
service/models.py 100.00% <100.00%> (ø)
service/routes.py 100.00% <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 812133e...0f48a67. Read the comment docs.

TimothyXu commented 2 years ago

Thank you for doing this! There are gonna be conflicts between you and Jingwei's story since he's currently refactoring the code to Flask-RESTX. I don't know how we should do this, perhaps wait till he's done then reformat this PR? Since this story comes after the refactoring (we did it hoping that we wouldn't have to do double work life this, hoping that the refactoring would be finished first then can just do this easily)

Adora2401 commented 2 years ago

I have resolved the existing conflicts but not sure if my changes exactly match Zhengrui's mind, maybe someone will help check this part.

TimothyXu commented 2 years ago
  1. Need to refactor into Flask-X/swagger (including creating a new swagger data model now that we have this extra hold column, and add swagger docs to the route)
  2. Per Professor's comments need to change the route to shopcarts//items//resume
  3. Missing behave BDD tests (criteria already in ZenHub)
lemadmax commented 2 years ago

Alright, I will cancel this pull request and fix everything. Maybe we should merge all pull requests before we start developing.

kdokm commented 2 years ago

Alright, I will cancel this pull request and fix everything. Maybe we should merge all pull requests before we start developing.

Ok, you can work on the latest version. I believe most of the things you added are kept. Also, I think big conflicts between different pull requests are either because of the dependency of stories, so maybe we should use block settings again. Small conflicts are fine, which we can easily solve manually.

TimothyXu commented 2 years ago

Yes block settings on ZenHub will help. Also thank you for pointing out my mistake @kdokm and please ignore what I said about 3. missing behave BDD tests