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

Allow paying with using on-site credit card #22

Merged

Conversation

jakubtobiasz
Copy link
Member

@jakubtobiasz jakubtobiasz commented Sep 10, 2024

closes #21

@jakubtobiasz jakubtobiasz force-pushed the 21-support-on-site-card-payment branch from e8ade80 to 9b21855 Compare September 10, 2024 17:45
@jakubtobiasz jakubtobiasz force-pushed the 21-support-on-site-card-payment branch from d35d5ff to 6272f93 Compare September 11, 2024 17:29
@jakubtobiasz jakubtobiasz changed the title 🚧 Allow paying with using on-site credit card Allow paying with using on-site credit card Sep 11, 2024
assets/shop/js/card_encoder.js Outdated Show resolved Hide resolved
src/Form/Type/TpayCardType.php Show resolved Hide resolved
src/Payum/Action/Api/AbstractCreateTransactionAction.php Outdated Show resolved Hide resolved
Comment on lines +31 to +41
if ('failed' === $response['result']) {
$details['tpay']['status'] = PaymentInterface::STATE_FAILED;
}

if ('success' === $response['result'] && 'pending' === $response['status']) {
$details['tpay']['transaction_payment_url'] = $response['transactionPaymentUrl'];

$model->setDetails($details);

throw new HttpRedirect($details['tpay']['transaction_payment_url']);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

To be checked: make sure it handles all possible cases
Improvement: we might introduce ->isFailed() or isRequiring3DS() to make it cleaner;

templates/shop/scripts.html.twig Show resolved Hide resolved
config/services/form.php Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
phpstan.neon Show resolved Hide resolved

$builder->addEventListener(
FormEvents::PRE_SUBMIT,
[$this->removeUnnecessaryPaymentDetailsFieldsListener, '__invoke'],
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[$this->removeUnnecessaryPaymentDetailsFieldsListener, '__invoke'],
[$this->removeUnnecessaryPaymentDetailsFieldsListener],

Will it also work like this?

src/Payum/Action/Api/PayWithCardAction.php Show resolved Hide resolved
Comment on lines +19 to +25
$form = $this->prophesize(FormInterface::class);
$form->remove('card')->shouldBeCalled()->willReturn($form);
$form->remove('blik_token')->shouldNotBeCalled();

$event = new FormEvent($form->reveal(), ['blik_token' => '123456']);

$this->createTestSubject()->__invoke($event);
Copy link
Member

Choose a reason for hiding this comment

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

It's totally fine now, but I'd like to share an idea to follow AAA pattern. All "should" methods we could move at the end. WDYT?

Suggested change
$form = $this->prophesize(FormInterface::class);
$form->remove('card')->shouldBeCalled()->willReturn($form);
$form->remove('blik_token')->shouldNotBeCalled();
$event = new FormEvent($form->reveal(), ['blik_token' => '123456']);
$this->createTestSubject()->__invoke($event);
$form = $this->prophesize(FormInterface::class);
$form->remove('card')->willReturn($form);
$event = new FormEvent($form->reveal(), ['blik_token' => '123456']);
$this->createTestSubject()->__invoke($event);
$form->remove('card)->shouldBeCalled();
$form->remove('blik_token')->shouldNotBeCalled();

'mapped' => false,
'label' => 'commerce_weavers_sylius_tpay.shop.order_summary.card.expiration_date.month.label',
'choices' => [
'commerce_weavers_sylius_tpay.shop.order_summary.card.expiration_date.month.january' => '01',
Copy link
Member

Choose a reason for hiding this comment

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

    $choices = [];

    foreach (['january, 'february', '...', 'december'] as $key => $month) {
        $choices[sprintf('commerce_weavers_sylius_tpay.shop.order_summary.card.expiration_date.month.%s', $month)] = sprintf("%02d", $key + 1)
    }

WDYT?

assets/shop/js/card_encoder.js Outdated Show resolved Hide resolved
config/config/sylius_fixtures.php Show resolved Hide resolved
config/config/sylius_template_events.php Show resolved Hide resolved

use Symfony\Component\Form\FormEvent;

final class RemoveUnnecessaryPaymentDetailsFieldsListener
Copy link
Member

Choose a reason for hiding this comment

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

Can't we rather base our decision on the state of the payment itself? Only when given payment method will be selected, then we are adding given fields

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this solutions is enough. We base the decision about passed data on a type defined in the gateway (see my screenshot two comments below). Here we only check whether a some data is passed (template is no loaded = field is not shown = we're getting a null); so we just remove it to avoid 500.


$details = $model->getDetails();

return !isset($details['tpay']['card']) && !isset($details['tpay']['blik_token']);
Copy link
Member

Choose a reason for hiding this comment

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

how will we decide in UI which option will be used?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the Admin Panel we can choose a Tpay payment type:
CleanShot 2024-09-13 at 07 25 47@2x

And in the UI we just check this value:
CleanShot 2024-09-13 at 07 24 37@2x

Copy link
Member Author

Choose a reason for hiding this comment

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

At the same time, I believe this type should be a value on payment details, so it can be easily accessed e.g. in the Payum action, so instead this if statement you marked, we'll do something like this:

return $details['tpay']['type'] === 'redirect';

Thanks to this change, once a new method is added, it's not required to modify the CreateRedirectBasedTransactionAction class.

templates/shop/scripts.html.twig Show resolved Hide resolved
@jakubtobiasz jakubtobiasz force-pushed the 21-support-on-site-card-payment branch from c1f43c1 to 8b26e3a Compare September 13, 2024 06:30
@jakubtobiasz jakubtobiasz merged commit c40d0c8 into CommerceWeavers:main Sep 13, 2024
4 checks passed
@jakubtobiasz jakubtobiasz deleted the 21-support-on-site-card-payment branch September 13, 2024 06:42
@jakubtobiasz jakubtobiasz mentioned this pull request Sep 13, 2024
7 tasks
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.

Support on-site card payment
3 participants