vgrange/contact modes #306
Conversation
dejafait
force-pushed
the
vgrange/contact_modes
branch
from
9070457
to
8af2897
Compare
|
review stp @regisb FTR il me reste encore à valider/compléter avec les métiers les wordings de |
dejafait
force-pushed
the
vgrange/contact_modes
branch
from
1f36137
to
f3b012b
Compare
|
|
||
| from labonneboite.common import encoding as encoding_util | ||
| from labonneboite.common import scoring as scoring_util | ||
| from labonneboite.common import hiring_type_util | ||
| from labonneboite.common.contact_mode import (CONTACT_MODE_MAIL, CONTACT_MODE_EMAIL, |
There was a problem hiding this comment.
C'est pas ouf de faire plein d'imports comme ça. Il vaut mieux importer directement le module : from labonneboite.common.contact_mode.
En fait, ces imports sont réalisés dans plusieurs modules différents. Du coup, est-ce que ça ne vaut pas la pein de créer quelques fonctions utilitaires dans labonneboite.common.contact_mode ?
| @property | ||
| def recommended_contact_mode_label(self): | ||
| try: | ||
| return CONTACT_MODE_LABELS[self.recommended_contact_mode] |
There was a problem hiding this comment.
return CONTACT_MODE_LABELS.get(self.recommended_contact_mode, '')
| @@ -17,7 +17,7 @@ | |||
|
|
|||
|
|
|||
| def has_text_content(s): | |||
| return s is not None and len(s) > 0 and not s.isspace() | |||
| return s and s.strip() | |||
There was a problem hiding this comment.
On pourrait utiliser cette fonction dans pas mal d'endroits pour simplifier le code. A déplacer dans un module utils.py peut-être ?
| # office has a contact_mode suggesting email and has data | ||
| office = Office() | ||
| office.contact_mode = 'Contactez nous par mail' | ||
| office.email = '[email protected]' |
There was a problem hiding this comment.
Est-ce que ça t'embête de remplacer tous les "pouac" et "plonk" par autre chose stp ? Ce sont les mots clés que j'utilise quand je débugge, et j'ai des git hooks pour être sûr de ne jamais envoyer "pouac" en prod.
| ('phone', 'Contactez l\'entreprise par téléphone'), | ||
| ) | ||
| CONTACT_MODES = dict(CONTACT_MODES_ITEMS) | ||
| # FIXME |
| elif self.website and self.website.strip(): | ||
| return CONTACT_MODE_WEBSITE | ||
| else: | ||
| return None |
There was a problem hiding this comment.
Je suggère de faire un peu de DNRY:
addresses = {
CONTACT_MODE_EMAIL: self.email,
CONTACT_MODE_PHONE: self.tel,
CONTACT_MODE_WEBSITE: self.website,
}
if has_text_content(self.contact_mode):
for contact_mode, keywords in CONTACT_MODE_KEYWORDS.items():
for keyword in keywords:
if keyword in self.contact_mode:
if contact_mode not in addresses:
return contact_mode
address = addresses.get(contact_mode)
if has_text_content(address):
return contact_mode
for contact_mode in [CONTACT_MODE_EMAIL, CONTACT_MODE_PHONE, CONTACT_MODE.WEBSITE]:
if has_text_content(addresses[contact_mode]):
return contact_mode
As ''.isspace() == False o_O the former expression would evaluate as True.
dejafait
force-pushed
the
vgrange/contact_modes
branch
from
f3b012b
to
33c9fd6
Compare
No description provided.