Remote freight calculation using Correios API #71
Conversation
|
Finalmente consegui finalizar isso aqui... |
osantana
requested review from
adlermedrado,
georgeyk,
caiocarrara,
daniellbastos and
lamenezes
|
todo mundo ignorando meu PR gigante :( |
|
valeu @adlermedrado! estou anotando no meu caderninho de amigos aqui o nome das pessoas que revisaram meu PR :P |
|
opa! mais um do @cacarrara! :) queria um review do @lamenezes que já trabalhou bastante por aqui também... :) |
| if extra_services is None: | ||
| extra_services = [] | ||
| else: | ||
| extra_services = [ExtraService.get(es) for es in extra_services] |
There was a problem hiding this comment.
eu removeria esse if/else e usaria or numa linha só, mas é 💄
There was a problem hiding this comment.
eu preciso da list comprehension e do [], ou seja, preciso do if e do else... ou faria um if-expression ou teria que usar and e or:
extra_services = [ES.get(es) for es in extra_services] if extra_services else []
# or...
extra_services = extra_services and [ES.get(es) for es in extra_services] or []Mas acho a legibilidade em ambos os casos bem ruim.
(além disso deve ter um bug no mypy que alerta para uma iteração em um objeto None)
| from_zip: Union[ZipCode, int, str], to_zip: Union[ZipCode, int, str], | ||
| package: Package, | ||
| value: Union[Decimal, float] = 0.00, | ||
| extra_services: Optional[Sequence[Union[ExtraService, int]]] = None): |
correios/models/posting.py
Outdated
| @@ -336,6 +337,10 @@ def volumetric_weight(self) -> int: | |||
| def posting_weight(self) -> int: | |||
| return Package.calculate_posting_weight(self.weight, self.volumetric_weight) | |||
|
|
|||
| @property | |||
| def freight_package_type(self) -> int: | |||
| return [3, 1, 2][self.package_type - 1] # see documentation for freight calculation | |||
There was a problem hiding this comment.
eita.. meio esquisitinho isso daqui
There was a problem hiding this comment.
correios amigo... correios... package_type para uma API são 1, 2, 3 e para outra API é 2, 1, 3 :)
There was a problem hiding this comment.
pensei em explicar isso nos comentários mas a coisa é tão idiota que só apontei para a documentação dos correios...
There was a problem hiding this comment.
É fácil de achar isso lá ? pq não é nada óbvio isso
There was a problem hiding this comment.
tá... vou explicar nos comentários então... melhor, né?
There was a problem hiding this comment.
refiz a implementação... acho que ficou um pouco melhor agora...
|
|
||
| def calculate_freights(self, | ||
| posting_card: PostingCard, | ||
| services: List[Union[Service, int]], |
There was a problem hiding this comment.
será que não seria uma boa ter algum default de serviço ?
There was a problem hiding this comment.
qual você sugeriria? :/ poderia ser PAC... mas tem uns 500 tipos de PAC :/
| @@ -294,6 +332,9 @@ def __init__(self, username, password, timeout=8, environment="production"): | |||
| self.websro_client = SoapClient(self.websro_url, timeout=self.timeout) | |||
| self.websro = self.websro_client.service | |||
|
|
|||
| self.freight_client = SoapClient(self.freight_url, timeout=self.timeout) | |||
| self.freight = self.freight_client.service | |||
There was a problem hiding this comment.
será que não seria interessante isolar essa parte de frete em outra classe ?
Se eu entendi bem, o acesso é público pra frete e pra instanciar essa classe eu preciso ter um username/password.
There was a problem hiding this comment.
Não é público. Também achei que era... pode ver que no método de calculo eu passo self.password para a api...
correios/models/posting.py
Outdated
| service: Union[Service, int], | ||
| error_code: Union[str, int], | ||
| error_message: str) -> None: | ||
| super().__init__(service, 0, Decimal("0.00")) |
There was a problem hiding this comment.
💄 eu particularmente prefiro usar o formato com nome=valor, acho que fica mais simples de ler
Flake8 does not understand imports used for "comment" type annotation and complains about unused imports.
osantana
force-pushed
the
remote-freight
branch
from
0b79cb7
to
21c0dd7
Compare
Remote freight calculation using Correios API
No description provided.