Skip to content

Fix 'projects[]' parameter in project.py get_bids function.#13

Closed
chaayac wants to merge 2 commits intofreelancer:masterfrom
chaayac:fix_get_bids_sdk_bug
Closed

Fix 'projects[]' parameter in project.py get_bids function.#13
chaayac wants to merge 2 commits intofreelancer:masterfrom
chaayac:fix_get_bids_sdk_bug

Conversation

@chaayac
Copy link
Copy Markdown

@chaayac chaayac commented Dec 19, 2017

'project_ids[]' parameter should be 'projects[]' in project.py get_bids function. Fixes issue #11 .

Copy link
Copy Markdown
Contributor

@amitsaha amitsaha left a comment

Choose a reason for hiding this comment

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

Can you add/update the test please?

@chaayac
Copy link
Copy Markdown
Author

chaayac commented Dec 19, 2017

test/test_projects.py doesn't need to be updated (it uses a FakeGetBidResponse to validate, so it will never receive the invalid parameter error from the Freelancer API).

@amitsaha
Copy link
Copy Markdown
Contributor

amitsaha commented Dec 19, 2017

test/test_projects.py doesn't need to be updated (it uses a FakeGetBidResponse to validate, so it will never receive the invalid parameter error from the Freelancer API).

Can you please look at https://requests-mock.readthedocs.io/en/latest/ and see if we can do a better job of testing the API calls that actually get made when we make a request? That will give us better sanity checking. For now, you can just modify the relevant test and later we can roll it out to others.

@chaayac https://stackoverflow.com/questions/47723797/assert-body-of-http-requests-using-requests-mock - something similar, in addition matching for the URLs would be useful as well. Can you take a look when you get a chance?

@amitsaha
Copy link
Copy Markdown
Contributor

Fixed by #13 #11

@amitsaha amitsaha closed this Feb 23, 2018
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.

2 participants