Fix to make shipping address from data passed from PayPal, per spec

[dorianfm]

Fix to issue #152

[SalahAdDin]

Can i forked this in my fork? I update many things from paypal, but this guys doesn't want merge it.

[dorianfm]

Please do!

[SalahAdDin]

Ok, i did.

[codecov]

Codecov Report

Merging #153 into master will decrease coverage by 0.62%.
The diff coverage is 4.34%.

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage   76.62%   75.99%   -0.63%     
==========================================
  Files          43       43              
  Lines        1664     1679      +15     
==========================================
+ Hits         1275     1276       +1     
- Misses        389      403      +14

Impacted Files	Coverage Δ
paypal/express/urls.py	100% <ø> (ø)	⬆️
paypal/express/views.py	66.79% <4.34%> (-3.79%)	⬇️

---

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 c55701b...225e8b5. Read the comment docs.

[dorianfm]

it’s not if you are trying to see what is sent to paypal, as this view is usually only called by paypal.

…

-- excuse brevity, sent from my phone.

On 12 Apr 2019, at 07:05, Alexander Gaevsky ***@***.***> wrote: @sasha0 requested changes on this pull request. In paypal/express/views.py: > payload = urlencode(pairs) + logger.debug(payload) I think it's redundant, since we return payload in the response. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[sasha0]

Ok, then the log message should be properly formatted.

[dorianfm]

That’s quite an open suggestion, I can’t find any documentation for django-oscar-paypal or oscar which specified a specific way to present log messages, so what are you expecting by ‘properly’ here? That there is some happy text informing what the string is, eg `logger.debug(Basket #%s - returning postage costs payload = ‘%s’ ` % basket.id, payload) following the format of other log messages in that view, or that the whole list of pairs is dumped a more clearly formatted KVP list, or something else in between those extremes? My personal experience is that I want to see exactly what is sent to PayPal without any additional formatting applied that may be confused as being part of the payload so I can easily extract that from a the log file and post-process as necessary if I need it to be more readable, so what I’ve done works for me. I’m happy to make changes, but I’d just be blindly making change after change without clearer guidance.

…

On 12 Apr 2019, at 08:06, Alexander Gaevsky ***@***.***> wrote: Ok, then the log message should be properly formatted. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQvb0d4cjYfAKv4_JInUCCM4rdfuHNPks5vgDBngaJpZM4LCZEM>.

-- theUsefulArts() { web design + creative technology } { web: theusefularts.org <http://www.theusefularts.org/>, twitter: @theusefularts <https://www.twitter.com/@theusefularts>, phone: +44 1294 823 600 }

[sasha0]

That there is some happy text informing what the string is, eg logger.debug(Basket #%s - returning postage costs payload = ‘%s’ % basket.id, payload) following the format of other log messages in that view

Yep, this should be enough.

I’m happy to make changes, but I’d just be blindly making change after change without clearer guidance.

Right. I'll be more explicit next time. Thanks for your efforts to update this PR and for the contribution.

[dorianfm]

Great, thanks very much for the feedback. Have committed that change now. I will make sure any future changes adhere to these kinds of standards, as best as I am able.

…

On 15 Apr 2019, at 10:12, Alexander Gaevsky ***@***.***> wrote: That there is some happy text informing what the string is, eg logger.debug(Basket #%s - returning postage costs payload = ‘%s’ % basket.id, payload) following the format of other log messages in that view Yep, this should be enough. I’m happy to make changes, but I’d just be blindly making change after change without clearer guidance. Right. I'll be more explicit next time. Thanks for your efforts to update this PR and for the contribution. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQvbySl40hIqhMd9dm-oWAG4OKlMrhJks5vhEJygaJpZM4LCZEM>.

-- theUsefulArts() { web design + creative technology } { web: theusefularts.org <http://www.theusefularts.org/>, twitter: @theusefularts <https://www.twitter.com/@theusefularts>, phone: +44 1294 823 600 }

[sasha0]

All's good, except the lint warning. Would you have chance to look at it? Otherwise, I'll fix it by myself.

Thanks for your efforts and time to update this PR!

[dorianfm]

Happy to fix, but not sure about what lint warning you mean - I can’t see anything in my linter… Maybe a GitHub PR noob issue with me :-)

…

On 1 May 2019, at 10:37, Alexander Gaevsky ***@***.***> wrote: All's good, except the lint warning. Would you have chance to look at it? Otherwise, I'll fix it by myself. Thanks for your efforts and time to update this PR! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACC63532GHEP2WMBT6BPULPTFQHPANCNFSM4CYJSEGA>.

-- theUsefulArts() { web design + creative technology } { web: theusefularts.org <http://www.theusefularts.org/>, twitter: @theusefularts <https://www.twitter.com/@theusefularts>, phone: +44 1294 823 600 }

[sasha0]

E.g.:

flake8 paypal tests setup.py
paypal/express/views.py:408:9: F841 local variable 'country_code' is assigned to but never used
paypal/express/views.py:430:5: E303 too many blank lines (3)
paypal/express/views.py:485:69: W291 trailing whitespace

[dorianfm]

Great, thanks. I was relying on my Editors linter.

…

On 1 May 2019, at 11:35, Alexander Gaevsky ***@***.***> wrote: E.g.: flake8 paypal tests setup.py paypal/express/views.py:408:9: F841 local variable 'country_code' is assigned to but never used paypal/express/views.py:430:5: E303 too many blank lines (3) paypal/express/views.py:485:69: W291 trailing whitespace — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACC636OVAVYTSN73MHZBLLPTFW55ANCNFSM4CYJSEGA>.

-- theUsefulArts() { web design + creative technology } { web: theusefularts.org <http://www.theusefularts.org/>, twitter: @theusefularts <https://www.twitter.com/@theusefularts>, phone: +44 1294 823 600 }

[dorianfm]

Thanks. Done and pushed to fork.

…

On 1 May 2019, at 11:35, Alexander Gaevsky ***@***.***> wrote: E.g.: flake8 paypal tests setup.py paypal/express/views.py:408:9: F841 local variable 'country_code' is assigned to but never used paypal/express/views.py:430:5: E303 too many blank lines (3) paypal/express/views.py:485:69: W291 trailing whitespace — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#153 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACC636OVAVYTSN73MHZBLLPTFW55ANCNFSM4CYJSEGA>.

-- theUsefulArts() { web design + creative technology } { web: theusefularts.org <http://www.theusefularts.org/>, twitter: @theusefularts <https://www.twitter.com/@theusefularts>, phone: +44 1294 823 600 }

[sasha0]

Alright, we're good now.