Make this compatible with plentymarkets #263
Conversation
Plentymarkets uses CamelCase for token responses See here: https://developers.plentymarkets.com/rest-doc/introduction
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
In general this is reasonable, but I have some concerns about the approach. Thanks for the PR!
| token['refresh_token'] = refresh_token | ||
|
|
||
| token['token_type'] = 'Bearer' | ||
| del token['tokenType'] |
There was a problem hiding this comment.
Is there any reason not to just build a new dictionary doing this mapping? I'm nervous about this fix being a bit brittle in the face of changing dictionaries.
There was a problem hiding this comment.
Oh no problem I rewrite it.
I copied the facebook fix and rewrite it for plenty, I just wanted to make it work.
token dict in plentymarkets.py remains untouched fixed tests for the plentymarkets compliance_fixes
| if refresh_token is not None: | ||
| fixed_token['refresh_token'] = refresh_token | ||
|
|
||
| fixed_token['token_type'] = 'Bearer' |
There was a problem hiding this comment.
Sorry, I should have been clearer.
What I was going for here was more like:
fixed_token = {}
for k, v in token.items():
fixed_token[to_snake_case(k)] = vThis should resolve the problem in its entirety, and also defends against this changing in the future. Does that make more sense?
More generic approch to convert the Token to snake case.
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
One tiny note and then we're good to go I think!
|
|
||
| def _compliance_fix(r): | ||
|
|
||
| # Plenty returns the Token in CamelCase instead with _ |
| from .slack import slack_compliance_fix | ||
| from .mailchimp import mailchimp_compliance_fix | ||
| from .weibo import weibo_compliance_fix | ||
| from .plentymarkets import plentymarkets_compliance_fix |
There was a problem hiding this comment.
Ah, sorry I missed this: can we get a newline at the end here?
There was a problem hiding this comment.
there are also 2 spaces. Cleaning this up.
Removed double-space and added newline
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
Cool, I'm happy with this. Thanks for the work!
3 participants
Plentymarkets uses CamelCase for token responses
See here: https://developers.plentymarkets.com/rest-doc/introduction
The Token Responses are from their docs, they are not mine.