search

CCI-MOC / flocx-market

2 stars 9 forks source link

Correct behavior of the Offer REST API #65

Closed larsks closed 5 years ago

larsks commented 5 years ago

Requests to offer/ should return a list of available offers. Prior to this commit, the API would return an error for requests to /offer.

Closes #64

larsks commented 5 years ago

@fvukelic would you mind taking a look at this PR? I've modified flocx_market/api/offer.py to respond correctly to requests for /offer, but I'm not certain I've done it the right way for a flask_restful.Resource.

Also, I think the tests that are part of this PR may be a good model for tests against the Bid and Contract APIs.

codecov-io commented 5 years ago

Codecov Report

Merging #65 into master will increase coverage by 4.45%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   85.28%   89.74%   +4.45%     
==========================================
  Files          26       27       +1     
  Lines         503      556      +53     
==========================================
+ Hits          429      499      +70     
+ Misses         74       57      -17
Impacted Files Coverage Δ
flocx_market/tests/unit/api/test_app_offer.py 100% <100%> (ø)
flocx_market/api/app.py 100% <100%> (ø) :arrow_up:
flocx_market/tests/unit/conftest.py 100% <100%> (ø)
flocx_market/api/offer.py 91.17% <100%> (+57.84%) :arrow_up:
flocx_market/tests/unit/api/test_app.py 100% <100%> (ø) :arrow_up:

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 4747694...e4d83a2. Read the comment docs.

larsks commented 5 years ago

Might have not been the smartest move to also push the offers change to the Bid PR -.- should I remove it?

Yeah, make that a separate PR.

fvukelic commented 5 years ago

Might have not been the smartest move to also push the offers change to the Bid PR -.- should I remove it?

Yeah, make that a separate PR.

The reason I made that a PR is that a bunch of stuff was crashing since I didn't test last time the interaction with the db when we removed the initmethod from offer_api.py. So I fixed that and then I tested and fixed up the return for get as well. Submitted the PR

larsks commented 5 years ago

@fvukelic if you're happy with the state of this pr, you can approve it, and I will merge it.

larsks commented 5 years ago

Thanks for the review!