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

NEW support of Stripe Terminal with Takepos #19444

Closed
wants to merge 47 commits into from
Closed

NEW support of Stripe Terminal with Takepos #19444

wants to merge 47 commits into from

Conversation

ptibogxiv
Copy link
Contributor

@ptibogxiv ptibogxiv commented Nov 21, 2021

With availability in Europe (France, UK, Ireland...)
Only support smart readers ie javascript and not only iOs and Android

@eldy @andreubisquerra

@ptibogxiv
Copy link
Contributor Author

PR functionnal with virtual test reader include in stripe SDK and demo card include in code

@ptibogxiv
Copy link
Contributor Author

Terminal ordered with test card and shipping planned for this week
further development in few days

@ptibogxiv ptibogxiv changed the title WIP - NEW support of Stripe Terminal with Takepos NEW support of Stripe Terminal with Takepos Nov 23, 2021
@ptibogxiv
Copy link
Contributor Author

Just need more informations when error and loading

@ptibogxiv ptibogxiv marked this pull request as ready for review November 23, 2021 22:37
@ptibogxiv
Copy link
Contributor Author

@eldy @andreubisquerra now is fully fonctionnal! test with Stripe smart reader bbpos wisepos E

I introduce some notifications for following the actions and errors ie pin error or card declined (can be used by other code)

@eldy eldy added the PR postponed PR is postponed (will be processed later). Ie: feature pushed during a beta or need transition steps label Dec 9, 2021
@meuchels
Copy link

I added this to my configuration and it works swell but the payment screen "pay.php" still shows "Credit Card" as a payment type. I believe credit card still has to be active for this to work but the extra button on the GUI in TakePOS is confusing.


/*
* View
*/

if (!empty($conf->stripe->enabled) && (empty($conf->global->STRIPE_LIVE) || GETPOST('forcesandbox', 'alpha'))) {
Copy link
Member

@eldy eldy May 10, 2022

Choose a reason for hiding this comment

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

If "CASHDESK_ID_BANKACCOUNT_STRIPETERMINAL".$terminaltouse is not set, it means we don't want to use Stripe for payment on takepos. Right ?
If right, can you add a condition here to avoid to make warning "We are in sandbox" when Stripe for Takepos has not bee enabled ?

Copy link
Member

Choose a reason for hiding this comment

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

@ptibogxiv Can you check this ?

@@ -314,7 +521,11 @@ function ValidateSumup() {
),
);
$numpad = $conf->global->TAKEPOS_NUMPAD;

if (!empty($conf->stripe->enabled) && !empty($conf->global->STRIPE_CARD_PRESENT)) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here: If "CASHDESK_ID_BANKACCOUNT_STRIPETERMINAL".$terminaltouse is not set, we should not output the message.

@@ -421,8 +632,18 @@ function ValidateSumup() {
$i = $i + 1;
}

$keyforsumupbank = "CASHDESK_ID_BANKACCOUNT_SUMUP".$_SESSION["takeposterminal"];
if (!empty($conf->stripe->enabled) && !empty($conf->global->STRIPE_CARD_PRESENT)) {
Copy link
Member

Choose a reason for hiding this comment

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

Same. Must be used only if stripe payment enabled for the current terminal $_SESSION["takeposterminal"]

@eldy eldy added PR OK to merge (but suggested fix required) PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published and removed PR postponed PR is postponed (will be processed later). Ie: feature pushed during a beta or need transition steps labels May 10, 2022
@ptibogxiv ptibogxiv requested a review from eldy June 12, 2022 19:11
@ptibogxiv ptibogxiv closed this Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR OK to merge (but suggested fix required) PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants