-
Notifications
You must be signed in to change notification settings - Fork 6
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(tiny): Updating with https://github.com/ecomplus/app-tiny-erp #218
Conversation
const tinyOrder = parseOrder(order, appData); | ||
logger.info(`${orderId} ${JSON.stringify(tinyOrder)}`); | ||
return postTiny('/pedido.incluir.php', { | ||
pedido: { | ||
pedido: tinyOrder, | ||
}, | ||
}).then((response) => { | ||
// TODO: middleware ? |
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.
// TODO: middleware ? |
// https://github.com/ecomplus/app-tiny-erp/blob/d5eef0c6be83485805ec94ba801a8cf111d24ed6/functions/lib/integration/export-order.js#L91 | ||
const updateTrackingCode = global.$tinyErpUpdateTrackingCode; | ||
if (updateTrackingCode && typeof updateTrackingCode === 'function') { | ||
return updateTrackingCode(response, order, postTiny); |
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.
return updateTrackingCode(response, order, postTiny); | |
return updateTrackingCode({ response, order, postTiny }); |
Especialmente nesses middlewares a gente não sabe se vai ter que adicionar parâmetros depois, muito granularizado pra cada app, melhor convencionar sempre um argumento só, sempre um objeto.
const validStatusesTiny = { | ||
aberto: true, | ||
cancelado: true, | ||
aprovado: true, | ||
preparando_envio: true, | ||
faturado: true, | ||
}; | ||
|
||
if ( | ||
validStatusesTiny[tinyStatus] | ||
&& (!order.fulfillment_status || order.fulfillment_status.current !== 'ready_for_shipping') | ||
) { | ||
logger.info(`${orderId} skipped with status "${tinyStatus}"`); | ||
return null; | ||
} |
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.
Esse trecho aqui deveria ser um switch case né, foi copiado? Nem sabia que ele existia no outro repo...
cc @matheusgnreis
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 é um erro aqui, só um comentário especialmente pra uma ocasião parecida no futuro
const setWeightUnitProduct = global.$tinyErpSetWeightUnitProduct; | ||
if (setWeightUnitProduct && typeof setWeightUnitProduct === 'function') { | ||
setWeightUnitProduct({ product }); | ||
} |
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.
Esse aqui tá esquisito, é só pra setar uma unidade padrão pro peso? Se for deveria ser só um metafield no config.json em vez de um middleware
@@ -124,6 +127,11 @@ export default (order: Orders, appData) => { | |||
} | |||
} | |||
|
|||
const middlewareOrderParser = global.$tinyErpOrderParser; |
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.
Um middleware tá nomeado no imperativo (UpdateTrackingCode) e outro não, é bom tentar manter o padrões...
const tinyOrder = parseOrder(order, appData); | ||
logger.info(`${orderId} ${JSON.stringify(tinyOrder)}`); | ||
return postTiny('/pedido.incluir.php', { | ||
pedido: { | ||
pedido: tinyOrder, | ||
}, | ||
}).then((response) => { | ||
// https://github.com/ecomplus/app-tiny-erp/blob/d5eef0c6be83485805ec94ba801a8cf111d24ed6/functions/lib/integration/export-order.js#L91 | ||
const updateTrackingCode = global.$tinyErpUpdateTrackingCode; |
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.
Além do tempo verbal no nome das funções, na verdade isso aqui nem necessariamente é pra tracking code inclusive, talvez global.$tinyErpOnNewOrder
?
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.
valeu @wisley7l 👍🏾
No description provided.