Committed!
vmarchuk → made their first commit to this issue’s fork.
Committed!
vmarchuk → created an issue.
Committed!
Duplicate https://www.drupal.org/project/commerce_cart_links/issues/3429317 📌 Automated Drupal 11 compatibility fixes for commerce_cart_links Needs review
vmarchuk → created an issue.
vmarchuk → changed the visibility of the branch 3517185-link-the-remote to hidden.
vmarchuk → made their first commit to this issue’s fork.
Added some minor changes to JS URLs as described in the API documentation.
vmarchuk → created an issue.
vmarchuk → made their first commit to this issue’s fork.
Committed!
Also, in src/Plugin/Field/FieldFormatter/AffirmFormatter.php there was a condition:
if ($item->currency_code !== 'USD') {
$elements[$delta] = [];
}
I tested this formatter with CA and USD, and everything works fine, so I removed this condition.
Found a bug with the "monthly_payment_on_add_to_cart" setting, the product has a "default" view mode but not a "full" one. So, the "monthly_payment" form element was never displayed.
Opened MR with the fixes.
vmarchuk → created an issue.
Committed!
vmarchuk → made their first commit to this issue’s fork.
vmarchuk → created an issue.
Added a patch compatible with the latest DEV version.
I can also confirm that the solution suggested in comment #10 helped solve this problem!
Added a patch for compatibility with version 1.3.
@anybody
We're currently working on this and want to launch it soon.
@tonytheferg Thanks for the good explanation.
> Also my issue is with the card element.
You need to switch to Stripe Payment Element as Card Element is no longer receiving updates (only backporting some fixes for Payment Element).
I opened MR with some initial work related to creating a Stripe account for anonymous users (but it still needs some work). But since we need to use the Customer::search() method, it requires the new API version 2020-08-27 (we are still using 2019-12-03), so the work cannot be done at this time.
What's in MR is useless since everything we have in the code works fine.
But I can reproduce this issue using the "Guest Checkout->Continue as Guest" button (on the login step). So in this case the account will not be created and the code in the attachCustomerToStripePaymentMethod() method will not run (most of the code will simply be skipped because the account is not authorized).
Payment profile updates happen here:
if ($is_reusable && $customer_id && $email) {
$payment_method_data = [
'email' => $email,
];
$billing_profile = $payment_method->getBillingProfile();
$formatted_address = $billing_profile ? $this->getFormattedAddress($billing_profile) : NULL;
if (!empty($formatted_address)) {
$payment_method_data = array_merge($payment_method_data, $formatted_address);
}
PaymentMethod::update($stripe_payment_method->id, ['billing_details' => $payment_method_data]);
}
And also an update for the user account:
if (empty($stripe_payment_method->customer)) {
$customer_data = [
'email' => $email,
'name' => $owner->getDisplayName(),
'description' => $this->t('Customer for :mail', [':mail' => $email]),
];
if ($is_reusable) {
$customer_data['payment_method'] = $stripe_payment_method->id;
}
$customer = Customer::create($customer_data);
$customer_id = $customer->id;
}
The biggest problem is that:
$customer_id = $this->getRemoteCustomerId($owner);
So, we have no connection between the Drupal account and the Stripe account.
Theoretically, we could create an account just in Stripe, and then if the customer comes back to the site (one day) and buys something, we would link it to the Drupal account. Otherwise, the account will only be created in Stripe.
What do you think about this?
vmarchuk → created an issue.
vmarchuk → created an issue.
vmarchuk → created an issue.
tomtech → credited vmarchuk → .
@megachriz I agree with your suggestions.
@megachriz
Yeah, this is "Constructor property promotion". Since this module is not that big, it makes sense to refactor everything possible to support all PHP 8 features.
I'm not sure if the failed tests are related to the changes, but I've added changes to the query to use the 2.x commerce way.
vmarchuk → created an issue.
vmarchuk → created an issue.
A new branch "3481620-add-use-of" has been opened with fixes for PHP 8 features, but I don't think it's finished yet. Need to spend time on additional fixes and testing.
vmarchuk → created an issue.
@megachriz
OK, thanks for the reply. I will create at least a few separate issues to address all the issues from the
comment
✨
Use queue for sending emails
Needs review
.
vmarchuk → created an issue.
Hello @megachriz,
I eventually got a fix for options 1-3 from a
comment
✨
Use queue for sending emails
Needs review
. I don't think 4-5 are really needed now and can be done later if possible.
Do you think I can open a new branch in this issue and add a fix to it, or is it better to create a new issue?
Thanks.
vmarchuk → created an issue.
vmarchuk → created an issue.
@jsacksick: The phpstan / phpcs issues are fixed and the issue can be merged.
Added a patch compatible with commerce 2.X.
@megachriz
The module looks much better with this patch (it was like the D7 module before the changes)!
But at the same time, several things are missing:
- Support for PHP 8 features (it would be great to add it now because the entire module is almost the commerce_abandoned_carts_cron() function)
- The mail_abandoned_cart log template was introduced but never used
- In getAbandonedOrders(), it would be better to use "o.type <> recurring" to detect commerce_recurring, so we don't need to join the commerce_order_item table
- In getAbandonedOrders() to determine if an order is still a cart, it is better to use "o.cart = 1"(it's more commerce 2.x way) instead of condition('o.state ', 'draft')
- We must consider the commerce_cart[cart_expiration][number] setting if set, i.e. history_limit must be less than commerce_cart[cart_expiration][number] otherwise all carts can be deleted before sending emails (don't forget about multiple order types?)
- Probably consider using the advancedqueue module (maybe at least when it's installed?). It supports allow_duplicates and max_retries, seems to be what we need for this module (we added core queue and advancedqueue support to this module https://www.drupal.org/project/commerce_email → )
Also, the patch from MR works fine!
The minimum amount for Stripe is 0.5 https://docs.stripe.com/currencies#minimum-and-maximum-charge-amounts, so it is not possible to use the Stripe Payment Element that uses PaymentIntent. Instead, you need to use SetupIntent to collect payment information during checkout. See related issue https://www.drupal.org/project/commerce_stripe/issues/3406817 ✨ Why does PaymentInformation->submitPaneForm handles things differently between Card Element and Payment Element? Active
The attached patch in comment #10 works fine.
+1 for RTBC
TomTech → credited vmarchuk → .
@gilmord
I see no reason not to enable the "Block if CVC verification fails" and "Block if postal code verification fails" rules for the project. I just checked the Radar settings on our side for the projects and we have both options enabled, but not for all projects (it seems like it should be enabled for all) so we can catch errors in the iframe.
Can you send the credit cards you use and I'll run tests on my side with the Radar rules disabled.
This request is out of the scope of this module. The custom tokens can be added to a custom module.
Committed!
vmarchuk → created an issue.
Updated to lcobucci/jwt:5.3.0 and tested locally (composer require 'lcobucci/jwt:5.3.0 as 4.3.0') and it works great. The lcobucci/clock library has been replaced by psr/clock.
Opened MR with the fixes.
There seems to be a problem running the tests:
Problem 1
- lcobucci/jwt[5.0.0, ..., 5.4.x-dev] require ext-sodium * -> it is missing from your system. Install or enable PHP's sodium extension.
- Root composer.json requires lcobucci/jwt ^5 -> satisfiable by lcobucci/jwt[5.0.0, ..., 5.4.x-dev].
To enable extensions, verify that they are enabled in your .ini files:
- /usr/local/etc/php/php-cli.ini
So that should be fixed as well.
vmarchuk → created an issue.
After applying the patch, the order bundle template is selected commerce-order-receipt--entity-print--quote.html.twig.
vmarchuk → created an issue.
We probably need to add more conditions to check if commerce_shipping is enabled, but here is a patch that fixes the problem.
vmarchuk → created an issue.
Added a patch compatible with the latest dev version.
vmarchuk → made their first commit to this issue’s fork.
The patch from #42 works fine.
Because "quantity" is on the order_item level. What you want to do can be done by going through all the order items and summing the 'quantity' field. So it must be a custom token.
vmarchuk → made their first commit to this issue’s fork.
vmarchuk → made their first commit to this issue’s fork.
vmarchuk → created an issue.
Once the issues reported by gitlab-ci are resolved, we'll need to update the .gitlab-ci.yml file with something like this (see example):
#
# Linting jobs are passing so any issue that breaks them should fix them.
#
cspell:
allow_failure: false
eslint:
allow_failure: true
phpcs:
allow_failure: false
phpstan:
allow_failure: false
stylelint:
allow_failure: false
vmarchuk → created an issue.
Added a small fix for Drupal coding standards and opened MR.
+1 to RTBC
vmarchuk → made their first commit to this issue’s fork.
I can also confirm that the patch in comment #5 fixes the issue. What is suggested in comment #8 also works fine, but the solution in comment #5 is simpler to understand what is going on there.
+1 to RTBC
@TomTech Thank you for pointing out any issues that may occur after the update.
From the documentation https://docs.stripe.com/upgrades#2022-11-15
Charge no longer auto-expands refunds by default. You can expand the list but for performance reasons we recommended against doing so unless needed.
Added a fix for this.
If in our case it is due to using PaymentIntent::retrieve($intent_id) without a try-catch.
vmarchuk → created an issue.
@TomTech
Thanks for the hook_update() related fix. I tested the patch locally, it seems to work fine.
The only thing I'm worried about is that after placing the order (and processing the queue) I see the payment_intent.succeeded event succeed. But it's a success because all the conditions we have in case Event::PAYMENT_INTENT_SUCCEEDED were skipped and the webhook did nothing.
Don't you think we should add a new condition and skip it?
Committed!
vmarchuk → created an issue.
@LeDucDuBleuet
Definitely possible. This could be hook_update() with a batch where we can go through all Stripe users and update them accordingly.
Please create a separate issue for this.
Update will be done in this issue https://www.drupal.org/project/commerce_stripe/issues/3403745 💬 Why is stripe-php still stuck on version 7 and Stripe API on 2019-12-03? What does it imply? Active
Update will be done in this issue https://www.drupal.org/project/commerce_stripe/issues/3403745 💬 Why is stripe-php still stuck on version 7 and Stripe API on 2019-12-03? What does it imply? Active
@TomTech
I wanted to test the patch locally, but I can't even apply the code changes:
➜ kickstart ddev drush updb -y
------------------------------- ----------- --------------- -------------------------------------------------
Module Update ID Type Description
------------------------------- ----------- --------------- -------------------------------------------------
commerce_stripe_webhook_event 9101 hook_update_n 9101 - Install the advanced queue, if possible.
------------------------------- ----------- --------------- -------------------------------------------------
// Do you wish to run the specified pending updates?: yes.
> [notice] Update started: commerce_stripe_webhook_event_update_9101
> [error] Drupal\Core\Entity\EntityStorageBase::create(): Argument #1 ($values) must be of type array, bool given, called in /var/www/html/web/modules/contrib/commerce_stripe/modules/commerce_stripe_webhook_event/commerce_stripe_webhook_event.install on line 159
> [error] Update failed: commerce_stripe_webhook_event_update_9101
[error] Update aborted by: commerce_stripe_webhook_event_update_9101
[error] Finished performing updates.
Failed to run drush updb -y: exit status 1
@TomTech @rszrama
Yes, that's why defer/async wasn't added earlier.
I think we need to update the JS to wait for the "Stripe" object to load, as done here (see waitForSdk) https://git.drupalcode.org/project/commerce_square/-/blob/8.x-1.x/js/com...
The patch is ready https://git.drupalcode.org/project/commerce_stripe/-/merge_requests/90.diff
Who can help with testing? Basically, there are not many changes and everything works fine.
vmarchuk → made their first commit to this issue’s fork.