-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat(correios v2): Create app to integrate Correios new APIs #251
Conversation
@leomp12 portei o app do correios v2, até aqui sem muitos problemas. Já com o fill-database fiz bastantes alterações (se esse era mesmo o caminho), pq vi que a api v2 do correios os paramentros para o calculo de frete inclui também as dimensões (que acabou gerando laços de repetição encadeados).
Então quando puder fazer a revisão aqui poderia ser mais criteriosa, para ver se fez ou não sentido as modificações feitas. |
packages/apps/correios-v2/lib-mjs/correios-db/fill-database.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Não entendi porque mudou tanto (não em sintaxe, lógica mesmo) a implementação do https://github.com/ecomplus/app-correios-db/blob/master/functions/lib/integration/fill-database.js , esse negócio é menos trivial que parece e na prática enfileirar tantos eventos acaba dando num monte de BO, eu acho que era mais seguro algo mais parecido com um copia e cola da implementação que já funciona...
packages/apps/correios-v2/lib-mjs/correios-db/fill-database.mjs
Outdated
Show resolved
Hide resolved
packages/apps/correios-v2/lib-mjs/correios-db/fill-database.mjs
Outdated
Show resolved
Hide resolved
packages/apps/correios-v2/lib-mjs/correios-db/pubsub/run-database.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Antes chamava calculate-correios.mjs só pra evitar ter mil arquivos "complexos" (não só um index.ts importando tudo) com o mesmo nome.
Quer dizer, calculate-shipping.mjs pode ser de qualquer app de envio, e aqui como é um monorepo estão todos no mesmo codebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfeito.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tem mais um BO aqui que eu vi e vai dar erro na prática...
Quando preenche o DB não pode haver custo por serviço adicional (a request provavelmente tem que ir sem as flags do Correios lá inclusive), mas no cálculo do frete esse custo tem que ser posteriormente checado, se ativo, pelo subtotal do carrinho.
Então aqui está errado de duas formas:
- Quando você faz a request "sob demanda" e salva no banco ela é salva com o valor declarado;
- Quando você busca o resultado do banco o valor declarado (e outros serviços) não são posteriormente tratados como acontece aqui https://github.com/ecomplus/app-correios-db/blob/master/functions/routes/ecom/modules/calculate-shipping.js#L345-L363
Na prática, se a mesma loja receber dois carrinhos com mesmo peso e faixa de CEP, um por um produto de R$ 5.000,00 e outro por um produto de R$ 10,00, o segundo cálculo vai ficar errado buscado do banco por causa da diferença da taxa do valor declarado.
if (correios) { | ||
const { nuContrato } = correios.$contract; | ||
if (nuContrato) { | ||
id = `${nuContrato}_${id}`; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Na verdade não deveria retornar um id sem número de contrato nunca, aliás em teoria nem deveria chegar aqui sem contrato...
O contrato é requerido
} | ||
|
||
if (addServiceVl.vlMinDeclarado) { | ||
const realValue = (vlDeclarado - addServiceVl.vlMinDeclarado); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Porque essa subtração ? 🤔
nuRequisicao, | ||
tpObjeto: '2', | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
// /* | ||
try { | ||
const psObjKg = parserWeight((pkg.weight.value) / 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Não necessariamente é o peso físico, o que vale é o maior entre o peso físico e o cúbico
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se atente a todos os parsers desse calculate-shipping do app de referência, por favor
weightValue = weight.value; | ||
break; | ||
case 'mg': | ||
weightValue = weight.value / 1000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weightValue = weight.value / 1000000; | |
weightValue = weight.value / 1000; |
mg pra g né?
// convert weight to kilograms | ||
if (weightValue > 0) { | ||
weightValue /= 1000; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converte o peso pra gramas e depois volta pra kg? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Não era melhor você manter o weightValue
somado em g e setar o cubicWeight
também em g (aka 1000 vezes menor)?
if (!configApp.free_no_weight_shipping || weightValue > 0) { | ||
psObj += quantity * (cubicWeight < 5 || weightValue > cubicWeight | ||
? weightValue : cubicWeight); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mas esse parâmetro não era pra ir em gramas? Não tá indo em kg aqui?
// no need to delete undefined fields | ||
|
||
Object.keys(obj).forEach((key) => { | ||
if (!obj[key]) delete obj[key]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!obj[key]) delete obj[key]; | |
if (obj[key] === undefined) delete obj[key]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deixa passar string vazia, talvez valor 0...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bora 👍🏾 😄
return calculateWeights.map((psObj) => { | ||
const promises = []; | ||
|
||
const calculateParams = { ..._calculateParams, psObjeto: psObj * 1000 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tá meio confuso isso, por que não usar sempre gramas já que a API deles recebe em gramas agora?
Refs.:
https://github.com/ecomplus/app-correios-db
https://github.com/ecomplus/app-correios-v2