add min_weight value #33

gfabrizio · 2017-02-06T19:00:49Z

allisson · 2017-02-06T19:46:54Z

correios/models/posting.py

@@ -331,6 +331,9 @@ def validate(cls,
        if service and service.max_weight and weight > service.max_weight:
            raise InvalidPackageWeightError("Max weight exceeded {!r}g (max. {!r}g)".format(weight, service.max_weight))

+        if service and service.min_weight and weight > service.min_weight:


Aqui não deveria ser weight < service.min_weight

allisson · 2017-02-06T19:49:20Z

É bom adicionar um ou mais testes no test_posting_models.py para esse valor mínimo

gfabrizio · 2017-02-06T19:50:56Z

👍 escrevendo os testes.

lamenezes · 2017-02-06T19:59:20Z

tests/test_user_models.py

@@ -208,6 +209,7 @@ def test_basic_service():
    assert service.get_symbol_filename("png") == os.path.join(DATADIR, "premium.png")
    assert isinstance(service.symbol_image, Image)
    assert service.max_weight == 10000
+    assert service min_weight == 1


não tá faltando um . aqui?

HUAHsduhaushduahsudhASD

georgeyk · 2017-02-06T20:35:16Z

correios/models/posting.py

@@ -331,6 +331,9 @@ def validate(cls,
        if service and service.max_weight and weight > service.max_weight:
            raise InvalidPackageWeightError("Max weight exceeded {!r}g (max. {!r}g)".format(weight, service.max_weight))

+        if service and service.min_weight and weight > service.min_weight:
+            raise InvalidPackageWeightError("The minimum weight is {!r}g".format(service.min_weight))


a correios sempre trabalha com "gramas" como unidade fixada ?

parece que sim, pelo que olhei no código inteiro sim.

gfabrizio · 2017-02-07T11:35:03Z

@allison adicionado testes e corrigido mais uma porradinha de coisas.

rhenter · 2017-02-07T11:57:50Z

correios/models/data.py

@@ -273,6 +273,7 @@
        'description': 'SEDEX 10',
        'category': 'SERVICO_COM_RESTRICAO',
        'max_weight': 10000,
+        'min_weight': 1,


Esse valor 1 ta na listagem do SEDEX? Porque se eu não me engano abaixo de 500g a pesagem fica com a tabela de cartas e mala direta.

http://www.correios.com.br/para-voce/consultas-e-solicitacoes/precos-e-prazos/servicos-nacionais_pasta/carta

foi o que é pedido na task.

e sedex e encomenda é um serviço que você compra, tem peso maximo, e esse minimo.

rhenter

👍

lamenezes · 2017-02-07T12:35:53Z

Olhando a documentação sobre limites dos correios [1] não existe peso mínimo para os serviços de entregas. Então, ao meu ver, colocar o dado min_weight no dict de infos dos serviços não é a solução certa.

Ao meu ver o correto seria fazer essa mesma validação de peso sem alterar os dicts.

georgeyk · 2017-02-07T15:19:58Z

correios/models/user.py

@@ -140,6 +140,7 @@ def __init__(self,
                 display_name: Optional[str] = "",
                 symbol: Optional[str] = None,
                 max_weight: Optional[int] = None,
+                 min_weight: Optional[int] = None,


Acho que mudaria pra zero como default.

georgeyk · 2017-02-07T15:24:51Z

@lamenezes acho meio inofensiva essa mudança, só alteraria o default do peso mínimo ser 0 ao invés de None (se entendi bem o código).
O peso mínimo nos correios pode ser ímplícito, por ex, as menores dimensões de pacote dão um peso volumétrico de 500g (que é tb a primeira faixa de peso para frete) e é usando sempre que o peso real seja menor que o volumétrico.

gfabrizio · 2017-02-07T15:25:31Z

já mudei o valor default p/ zero

lamenezes · 2017-02-07T15:35:08Z

a questão é que quando se altera o dict de serviços no módulo data para colocar o min_weight igual a 1 fica parecendo que existe uma regra dos correios que obriga o peso do pacote ser 1g ou maior para todos os serviços, mas não é isso o que acontece.

lamenezes · 2017-02-07T15:36:19Z

essa validação de peso é realmente necessária e importante para o serviço, só acho que deve ser feita diretamente no posting.Package sem alterar o módulo data

georgeyk · 2017-02-07T15:38:16Z

bom, é um regra "prática"..
Não sei se jogar no posting.Package seja melhor, ai vc tem um regra de max_weight aqui e outra de min_weight lá ?

gfabrizio · 2017-02-07T15:42:25Z

exato @georgeyk foi exatamente por isso que alterei os dicts, p/ a regra não ficar só em um lugar.

lamenezes · 2017-02-07T15:51:49Z

mas as duas regras (de max e min) já estão no posting.Package.validate (que é o método responsável por todas as validações de pacote) a diferença é que a de max_package utiliza dados do módulo data que, na minha concepção, representa as regras de negócios como está nos correios. Enquanto a de min_package me parece mais uma validação de peso inválido do que uma obrigação dos correios.

lamenezes · 2017-02-07T15:54:23Z

a impressão que eu tenho desse PR é: "a olist não aceita pacotes (ou produtos) com peso menor que 1g, então vamos mudar a lib dos correios para injetar uma regra nos serviços SEDEX, PAC, E-SEDEX etc. dizendo que eles não aceitam pacotes com peso de 1g ou menos"

lamenezes · 2017-02-07T15:54:45Z

vou tirar meu review daqui, façam como achar melhor.

birl

gfabrizio · 2017-02-07T16:03:55Z

sua sugestão recapitulando é; retirar do "data" e manter a vlidação somente no posting.package? posso fazer essa alteração, o seu último comentário fez sentido, parece o correto a se fazer.

Sem stress @lamenezes , estamos discutindo pq é o que gostamos de fazer :)

georgeyk · 2017-02-07T16:05:00Z

Não é "façam como achar melhor", somos um time, nós fazemos o que time acha melhor =)

Eu quero entender o que tem de errado com isso, pode não ser uma regra escrita nos docs dos correios, mas é uma regra que tá implícita físicamente (afinal, uma folha A4 > 4g).

georgeyk · 2017-02-07T16:10:10Z

osantana · 2017-02-07T16:10:44Z

Lembrem-se que esse projeto é OSS e aberto... vamos limitar as referências à questões internas da olist aqui nesse espaço, ok? (referência à regras de negócio interna ou à tickets no nosso jira não são boas idéias).

Se for o caso podemos fazer um fork private e exportar para cá somente o resultado final dos PRs, ok?

osantana · 2017-02-07T16:32:31Z

Olhando a documentação sobre limites dos correios [1] não existe peso mínimo para os serviços de entregas. Então, ao meu ver, colocar o dado min_weight no dict de infos dos serviços não é a solução certa.

Veja... existe peso mínimo para encomendas dos correios para fins de tarifação apenas. Eu posso enviar um pacote de 100g mas pagarei por 300g porque é o valor mínimo para preço.

Então, IMHO, a lib dos correios deveria levantar uma exception para peso <=0 porque isso realmente é fisicamente impossível (e isso não precisa ficar nos dicts porque peso > 0 é o limite para qualquer objeto real em um ambiente com gravidade).

Ao mesmo tempo eu sou 👍 para adicionar a informação de peso mínimo para cobrança na lib. Essa informação pode ser usada por serviços de cálculo de frete e como limitador mínimo para cálculos de peso cúbico.

georgeyk · 2017-02-07T16:34:11Z

correios/exceptions.py

@@ -65,6 +65,10 @@ class InvalidPackageWeightError(InvalidPackageError):
    pass


+class InvalidMinPackageWeightError(InvalidPackageError):
+    pass


acho que talvez seja bom usar InvalidPackageWeightError e colocar uma mensagem indicando o peso mínimo na hora de fazer o raise.

gfabrizio · 2017-02-08T08:44:28Z

Bom, eu vou fechar esse PR, seguindo a sugestão do @georgeyk por dois motivos:

não chegamos a conclusão alguma do que é melhor
Temos meta na nossa sprint, então vou fazer essa validação diretamente na nossa API interna, e em momento oportuno, quando necessário, verificamos o que fazer na lib dos correios.

add min_weight value

b1a5877

allisson reviewed Feb 6, 2017

View reviewed changes

lamenezes previously requested changes Feb 6, 2017

View reviewed changes

georgeyk added the wip label Feb 6, 2017

georgeyk reviewed Feb 6, 2017

View reviewed changes

gfabrizio added 4 commits February 7, 2017 09:00

fix typ0

a4c4a02

add invalid minimum wieght package exception

67b8779

fix pep8 issues

b76ad1f

add/fix fix some tests

89bd0bd

gfabrizio removed the wip label Feb 7, 2017

rhenter reviewed Feb 7, 2017

View reviewed changes

rhenter approved these changes Feb 7, 2017

View reviewed changes

georgeyk reviewed Feb 7, 2017

View reviewed changes

change default value to 0

e0c4367

gfabrizio changed the title ~~[BP-190] add min_weight value~~ add min_weight value Feb 7, 2017

georgeyk reviewed Feb 7, 2017

View reviewed changes

gfabrizio closed this Feb 8, 2017

osantana deleted the BP-198 branch February 8, 2017 11:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add min_weight value #33

add min_weight value #33

gfabrizio commented Feb 6, 2017 •

edited

Loading

allisson Feb 6, 2017

allisson commented Feb 6, 2017

gfabrizio commented Feb 6, 2017

lamenezes Feb 6, 2017

georgeyk Feb 6, 2017

georgeyk Feb 6, 2017

gfabrizio Feb 7, 2017

gfabrizio commented Feb 7, 2017

rhenter Feb 7, 2017

gfabrizio Feb 7, 2017

gfabrizio Feb 7, 2017

rhenter left a comment

lamenezes commented Feb 7, 2017

georgeyk Feb 7, 2017

georgeyk commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

georgeyk commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

georgeyk commented Feb 7, 2017

georgeyk commented Feb 7, 2017

osantana commented Feb 7, 2017

osantana commented Feb 7, 2017 •

edited

Loading

georgeyk Feb 7, 2017

gfabrizio commented Feb 8, 2017

add min_weight value #33

add min_weight value #33

Conversation

gfabrizio commented Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

allisson commented Feb 6, 2017

gfabrizio commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfabrizio commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhenter left a comment

Choose a reason for hiding this comment

lamenezes commented Feb 7, 2017

Choose a reason for hiding this comment

georgeyk commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

georgeyk commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

lamenezes commented Feb 7, 2017

gfabrizio commented Feb 7, 2017

georgeyk commented Feb 7, 2017

georgeyk commented Feb 7, 2017

osantana commented Feb 7, 2017

osantana commented Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

gfabrizio commented Feb 8, 2017

gfabrizio commented Feb 6, 2017 •

edited

Loading

osantana commented Feb 7, 2017 •

edited

Loading