Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] fix mobile for roulier laposte #107

Open
wants to merge 1 commit into
base: 8-roulier
Choose a base branch
from

Conversation

sebastienbeau
Copy link
Member

I still have an issue with mobile.
Indeed if there is no mobile the field will be not set to an empty value
@bealdav @hparfr

@hparfr
Copy link
Member

hparfr commented Aug 9, 2017

It's a breaking change : if the client's base contains only "phone" and not mobile it will break.

@codecov
Copy link

codecov bot commented Aug 9, 2017

Codecov Report

Merging #107 into 8-roulier will increase coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff              @@
##           8-roulier     #107      +/-   ##
=============================================
+ Coverage       58.9%   58.94%   +0.03%     
=============================================
  Files             33       33              
  Lines           1628     1627       -1     
=============================================
  Hits             959      959              
+ Misses           669      668       -1
Impacted Files Coverage Δ
delivery_roulier_laposte/models/stock.py 36.53% <0%> (+0.68%) ⬆️

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 15c7809...7d88a5d. Read the comment docs.

@florian-dacosta
Copy link
Member

You can't do something like :
if address.get('mobile'):
address['phone'] = address.get('mobile')
elif not 'phone' not in address:
address['phone'] = ''
?
Or I did not get the problem, but I also think the this fix may break things in cases the customer has only a fix phone

@hparfr
Copy link
Member

hparfr commented Aug 9, 2017

Here is the difference :

mobileField phoneField Before this patch After this patch
no yes phoneField None
yes no mobileField mobileField
yes yes mobileField mobileField
no no None None

Therefore,

mobileField phoneField before after
06.. 07 01...05 OK OK
None 06...07 OK KO

|

@bealdav
Copy link
Member

bealdav commented Apr 19, 2018

on fait quoi du coup là ?

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.

4 participants