Skip to content
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

Adicionado status PAYMENT_REVIEW para novos pedidos e habilitado rotina de checagem de status de pedidos por padrão #309

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

eneiasramos
Copy link

Adicionado status PAYMENT_REVIEW para novos pedidos e habilitado rotina de checagem de status de pedidos por padrão

Copy link
Owner

@r-martins r-martins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agradeço se puder dar um feedback nos meus comentários. Restaram dúvidas. ;)
Obrigado pelo PR de qualquer forma. Parte dele já está aprovada.

*/
private function _isHashable()
{
return function_exists('hash')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O método original usa || (OR) para verificar a ausência do suporte de qualquer um dos tipos de hash. Receio que deva fazer o mesmo aqui.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ajustado fe248c1

@@ -164,6 +164,7 @@
<active>0</active>
<title>Cartão de Crédito - via PagSeguro UOL</title>
<order_status>pending</order_status>
<paid_status>paid</paid_status>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A alteração do status do pedido se dá definindo o status padrão para aquele state. A maioria dos módulos de pagamento usa isso como padrão.
Porque um pedido via Pagseguro + Cartão de credito passaria a usar outro status?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

É muito útil para integrações de pedidos, caso seja necessário utilizar um status personalizado para pedidos pagos.

Ou para visualizações de pedidos no admin personalizadas como o do Toluca Store.

Adicionei um fallback para utilizar o status atual caso um novo não for configurado 8698dd7

<address_street_attribute>street_1</address_street_attribute>
<address_number_attribute>street_2</address_number_attribute>
<address_neighborhood_attribute>street_2</address_neighborhood_attribute>
<address_complement_attribute>street_3</address_complement_attribute>
<address_neighborhood_attribute>street_4</address_neighborhood_attribute>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boa. Não sei como deixei isso passar.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aconteceu meu caro :)

@eneiasramos eneiasramos requested a review from r-martins June 17, 2022 15:14
@r-martins
Copy link
Owner

A rotina de remover acentuação só se faz necessária pra Sandbox. No ambiente de produção você não terá problemas com acentuação.

@eneiasramos
Copy link
Author

@r-martins bora remover esse trem :)

@r-martins
Copy link
Owner

r-martins commented Aug 16, 2022

Obrigado pelo PR @eneiasramos .
Só pra entender e conseguir testar, em que endpoint podemos obter o session id?
Hoje temos o /pseguro/ajax/getSessionId (controller) pra isso. Entendo que do seu jeito parece estar até melhor. :)

PS: Provavelmente aplicarei isso em um commit separado, pois seu PR contém multiplas melhorias em diferentes areas no mesmo PR.

@eneiasramos
Copy link
Author

Obrigado pelo PR @eneiasramos . Só pra entender e conseguir testar, em que endpoint podemos obter o session id? Hoje temos o /pseguro/ajax/getSessionId (controller) pra isso. Entendo que do seu jeito parece estar até melhor. :)

PS: Provavelmente aplicarei isso em um commit separado, pois seu PR contém multiplas melhorias em diferentes areas no mesmo PR.

@r-martins tudo bom meu caro?

A ideia é obter o sessionID da loja para ser manipulado de forma transparente em um APP por exemplo...

Neste cenário, a geração do card token, junto com o session id retornado, ficariam a cargo do APP.

Copy link
Owner

@r-martins r-martins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obrigado pelo pull request!!
Tem muita coisa boa aqui, @eneiasramos.
No entanto, algumas das mudanças de status apresentadas não me parecem corretas e receio que possam ser vistas como bug. Dê uma olhada nos meus comentários e vamos falando.

@@ -316,6 +324,8 @@ protected function _order($payment, $amount, $ccIdx)
{
$order = $payment->getOrder();

$order->setState(Mage_Sales_Model_Order::STATE_PAYMENT_REVIEW, true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nem todo pagamento começa como Payment review. Isso poderia confundir o comprador. Ex: Pedido com Boleto deve entrar como Pending e não review. Review só ocorre no caso de análise de fraude de um pagamento realizado/capturado com sucesso.

<customer_cpf_attribute>taxvat</customer_cpf_attribute>
<address_telephone_attribute>telephone</address_telephone_attribute>
<customer_cpf_attribute>customer|taxvat</customer_cpf_attribute>
<address_telephone_attribute>cellphone</address_telephone_attribute>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Não temos atributo cellphone por padrão. (pelo menos nao no openmage 20.0.13 que estou usando pra teste)

@@ -679,7 +679,8 @@ protected function _confirmPayment($payment, $notification)

$order->addStatusHistoryComment
(
sprintf('Fatura #%s criada com sucesso.', $invoice->getIncrementId())
sprintf('Fatura #%s criada com sucesso.', $invoice->getIncrementId()),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se passar como false ele não mudará o status do pedido depois da fatura criada. Não consigo ver sentido neste ser o comportamento padrão. Pedidos com fatura gerada viram Processing (ou outro status associado como padrao pro Processing).

@@ -25,6 +25,13 @@ $multiCcEnabled = Mage::helper('ricardomartins_pagseguro')->isMultiCcEnabled();
</div>
</li>
*/ ?>
<?php if ($this->getInstructions()): ?>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O que o motivou a ter instruções adicionais pra pedidos com cartão? (curiosidade)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants