🇺🇸United States @rhovland

I can't figure out how to fix this because PHP automatically casts array keys to int if they look like one and there is no way to control this behavior. The $services variable is fed into the form options as an array.

I tried the following

    $services = [];
    foreach ($this->services as $service) {
      $services[(string) $service->getId()] = $service->getLabel();
    }

This piece of code seems to be using the array key instead of the service id and that results in automatic type casting to integers

   $services = array_map(function ($service) {
      return $service->getLabel();
    }, $this->services);

rhovland → created an issue.

One bug we noticed today: The created time of the old product gets duplicated. The creation time should be set to when the duplicate is created.

I just wanted to make sure the default core behavior is at least addressed in this theme. The other buttons are outside the scope of this issue.
Users can edit the template of course, but the hours of research to figure out what was wrong and fix it for something as simple as "button vs primary button" isn't a good first use experience for this theme.

rhovland → created an issue.

Tests are now passing

Merge request added. Change is very simple.
Since it appears container elements with no content are 0 height divs this should not affect anyone's layout that was relying on line spacing of empty divs.

rhovland → created an issue.

dcam → credited rhovland → .

So I asked in slack and the infrastructure maintainers told me it's because those are DrupalCI tests that are running automatically. Once this is committed they recommend disabling DrupalCI

Huh weird I realized that the display here in this issue of what merge request test versions ran (D9.5) doesn't match what actually ran (D10.2)

So it seems there is already an exclusion rule for translations in annotations here https://git.drupalcode.org/project/coder/-/blob/8.3.x/coder_sniffer/Drup...

The rule looks for "Translation" not "translation". I fixed the capitalization and now it passes.

It might be something we want to get fixed upstream in the base config since I'm sure there are other instances of annotations being longer than than the max length allowed in the rules. Unfortunately it seems we cannot inherit rules for phpcs so if we define our own we have to keep it in sync with upstream.

rhovland → created an issue.

rhovland → created an issue.

International shipping rates are working. If they aren't for you then more details are needed to successfully debug why.
There is API logging functionality that can be turned on in the shipping method. Provide the request and response of a failed shipping request.

Has been 5 years and the library is no longer in use. Closing.

Upstream was fixed in 📌 Dependency vinceg/usps-php-api is abandoned Fixed and it includes a default timeout now https://github.com/GeNyaa/USPS-php-api/blob/df1925e619f84e29cf09e012f39d...

This issue is no longer needed as the timeout is handled in the USPS PHP library

While this affects this module the source of the issue is actually in commerce_shipping where the packing plugin is essentially a placeholder until someone makes a real one. Packers can also be plugins provided by third party modules.
I agree this is a huge issue for commerce shipping modules, and especially impacts things such as flat rate boxes and dimensional weight.

The following issue may help with flat rate boxes at least ✨ Condition for Dimension Needs review . I have not tested the patch in it though and I can't attest to if it works.

I also made this issue for making a dimensional packer to get the ball rolling on a feature we've needed for years ✨ Create a dimensional packer Active

rhovland → created an issue.

There is a lot of work that needs doing for this module and very little of it requires maintainer access. Providing quality merge requests for issues with tests goes a long way towards getting them committed.

There are separate profiles for the order and the user. If the user enters a shipping profile and saves it then keeps the "Billing same as shipping" box checked then a billing profile will never be created for the user, only the order. The shipping profile will be saved to their account though.
Is this the behavior you're seeing?

I don't think negative amounts are allowed. Besides the implication of a negative number in a payment system, the API simply does not allow it
Go here:
https://developer.authorize.net/api/reference/index.html#payment-transac...
Click on "Request Field Description" below the API example and scroll to unitPrice

You can also try editing the request JSON in the example to a negative number and hit send. You'll get an error response.

So since authorize.net does not support negative numbers how is the module supposed to handle negative item prices?

Should it set the price to 0? Which is the worst outcome? A transaction failure or line items not adding up? Are you trying to make payments for negative balance orders or is this negative line item a "discount"? Why not use adjustments for that?

You wouldn't happen to be trying to do this? ✨ [Child] Store transaction details on the payment Active

Like #3154898: E00039 A duplicate customer payment profile already exists → we need a way to reproduce this reliably if we can ever hope to fix it.

This may now be a duplicate of ✨ Expand Accept.js to include Accept.js UI Needs review since that is a pure JS implementation meant to minimize compliance requirements but I'm unsure what the PCI DSS implications of the current codebase for acceptjs is.

rhovland → made their first commit to this issue’s fork.

Fixed some of the most obvious errors provided by the automated patch.

However there are still many items here that need someone with more experience with javascript to evaluate the remaining changes.

Also need to evaluate if the project wants to follow the rules from prettier or not because it makes some sweeping changes to the javascript files.

Replacing jquery's .val() with vanilla .value has some pitfalls to be aware of and I am not qualified to make those decisions so I left those alone.

rhovland → created an issue.

Putting this up for review. Has tests and upgrade hook for the added fields.

Maybe we want to create a new issue for the 3.x branch for having different payment types (credit card, giftcard, check, ACH, crypto, etc).

cspell test now passing

rhovland → created an issue.

I screwed up dependency injection in the patch. Here's the correct version

Here is an updated patch to account for changes in the latest release.
Switched commerce_payment.order_updater to use dependency injection
Fixed a couple minor errors in the comments

PHPUnit tests are now passing. Please review for commit.

Note one of the errors was in
tests/src/FunctionalJavascript/ConfigurationFormTest.php/testCreateAcceptJsGateway()
where it was asserting if Ajax completed but the option it was selecting is the default option in the default installation so ajax never runs. I changed the test to only click on the radio and assert ajax ran only if the radio button wasn't already selected.

The other functional javascript errors were related to setting the status radio to a non-valid type. They were all changed to '1' (Enabled).

The kernel test failures were a D9 depreciation where getModuleUpdateFunctions() needed to be changed to getUpdateFunctions()

Tests failing. They are the same failures experienced with Drupal CI manually configured to test D10 with PHP8.1

Failing Drupal CI test results:
https://www.drupal.org/pift-ci-job/2886512 →

Failing GitLab CI test results:
https://git.drupalcode.org/issue/commerce_authnet-3443033/-/jobs/1411481

Also the default branch in gitlab for this project is 7.x-1.x instead of the current branch

rhovland → created an issue.

I have reviewed the module using drupal-check and there are no more Drupal 10 depreciations other than what is covered in the patch / merge request.

+1 How did this get missed during the upgrade for D10 compatibility?

rhovland → changed the visibility of the branch 3432633-query-Interface-access-check to active.

rhovland → changed the visibility of the branch 3432633-query-Interface-access-check to hidden.

The accept.js integration uses the Customer Information Manager (CIM) to store card details for reuse.

The API documentation for that is here https://developer.authorize.net/api/reference/index.html#customer-profil...

It appears there are no special fields beyond what is already being sent by the module.

This email appears to be notifying customers that they need to be using either ARB or CIM for stored payment details to be compliant.

I cannot find any information about these supposed "new API fields" mentioned.

Before:

After:

rhovland → created an issue.

Yeah the UI is definitely borne out of my ability level. Ideally tax information would be displayed on the order view screen and there would be a recalculate taxes button there.

rhovland → changed the visibility of the branch 3042258-allow-products-to to active.

rhovland → changed the visibility of the branch 3042258-allow-products-to to hidden.

This is caused by changes to AddToCartForm.php

If you have a module that modifies the cart form such as a contrib module like commerce_pado or commerce_vado then check if there is an update that addresses this error.

If you have a custom module that modifies the add to cart form then the code needs to be fixed there.

Agreed. However I think the crux of the issue is that workflows can't be defined from the UI. The skillset needed to define a custom workflow is way bigger than what's needed to build a commerce site.

Updated patch for latest version of module

It appears that tests are failing to run due to commerce missing from the requirements section in composer.json
https://www.drupal.org/project/commerce_pado/issues/3249818 📌 composer.json is missing dependency on commerce Active

Waiting for that to be committed.

Also this branch will probably need to specify drupal/commerce:~2.37 in the composer.json because the fix will probably make this module incompatible with older versions of commerce.

Tests are failing, needs work

The changes are now in core 2.37 and this results in pado being incompatible with the latest version of commerce until this is fixed.

Ok I have information about how to reproduce this.
If you have automatic redirect creation enabled and have saved an entity with the url alias set to the system path for the entity. Go back and change it to something else. A redirect will be created that results in an infinite redirect loop.

Eg:
Url alias for a product set to /product/1465 changed to /product/1465-a-pretty-name

I am also encountering this on our site. I have pathauto installed. It was added later after a bunch of products were already created with urls such as `/product/2398`. When I go to enable automatic aliases for those products a redirect is automatically created for the path, creating an infinite loop.

While the redirect UI to add a redirect validates these issues and refuses to create aliases that would cause infinite loops it seems it's missing validation when redirects are created programmatically.

A patch to do this validation when redirects are created automatically is needed to prevent this from happening. The currently posted patch does not address this as it only checks for node paths.

I get this too, but only on Firefox in my dev environment (no caching). It seems to happen when I make a css change that causes drupal to calculate something in the background. Seems almost like a browser caching issue trying to use old files that no longer exist or something. I have never seen it in production or on Chrome.

Patch adding these config items to the default install configuration

rhovland → created an issue.

If you go to /admin/config/bootstrap-layout-builder/layouts you can add options to the list of choices for each layout including what you describe above

Oh apparently these changes are present in 🐛 🎨 Clean up UI for 1x branch RTBC

Patch adds the selectors to the scss file and changes complied to the theme.

rhovland → created an issue.

Tested on my Bootstrap 5 based site. Setting no gutters now functions as expected.

I'm not sure if the version configuration item is really needed though? Why not set both classes? The one that matches the current bootstrap version will work, the other won't do anything.

Are there other differences between them that apply to this module, not bootstrap_styles, that would benefit from the version configuration item?

Can confirm #5 works on Drupal 10.1.

Strange this wasn't caught when marking the project D10 compatible considering this error even shows up in drupal-check and similar depreciation checking tools.

Here a version of the patch that works on 10.1

Maintainer of gabrielbull/php-ups-api seems to be no longer working on the project. There hasn't been any activity for more than a year.
We need to fork it. Drupal 10 requires at least PHP 8.1

There's also https://github.com/wpdesk/php-ups-api which is being actively maintained but their branch diverged from the main repo a while ago and there's a lot of differences between them. I don't know enough about the codebase to tell if those differences are important or not. The code does appear to be used in production though. Might be worthwhile swapping them and see what breaks?

In the meantime anyone here needing a stopgap fix, you can apply this patch to gabrielbull/php-ups-api using composer to get PHP 8.1 compatibility.

This patch does not touch permissions for editing payment methods. The url is built using the route for the payment entity. The only permission I had to grant to anonymous was "edit own payment methods". The code also checks for this permission before displaying an edit link to the user.

Is the url being generated in the format of /user/the-user-id-number/payment-methods/payment-method-id-number/edit?

This is a feature request not a bug report. I'm aware of why it doesn't sort correctly. I want to implement smart sorting as a feature at some future point. If this is not a feature the commerce maintainers want in commerce core then they can say so and close this as Won't Fix

rhovland → created an issue.

I attempted to contact the maintainer via slack and have received no response. Someone who can opt into security coverage needs to apply to be a maintainer of this module.

The above patch in #45 does not include the latest fixes from the merge request.

There's an extra whitespace after the * in one of the files which is making it impossible to apply as a patch

 * @endcode
 * 
 * @see \Drupal\ckeditor5\Annotation\CKEditor5Plugin

https://git.drupalcode.org/project/embed/-/merge_requests/2/diffs#777e0e...

I see a duplicate issue was opened and fixed
🐛 Update from 1.2 to 1.3 breaks existing order receipt emails Fixed

I was working on testing this, unless the automated tests passing is good enough.
I have completed testing it in my project. Should I test it on a stock install of commerce too?

rhovland → created an issue.

Here is my first attempt at addressing this.

Because the list of available payment methods is a Form API radios element each option only accepts a plain string. Therefore I cannot attach links on the stored payment method entries.

So instead if a stored payment method is selected and the customer has permission to edit it a button will appear below that says "Edit selected payment method". This will take them to the payment method edit form where they can either make changes or press the delete button to delete it. The checkout step they were on before going to the edit form is set as the return destination and they will be sent back there upon saving or deleting. The customer can then continue through checkout.

rhovland → created an issue.

rhovland → created an issue.

I'm moving this to 3.x because the concept is a breaking change and is more appropriate for a new major version

Realized I didn't finish the modifications to the payment view. Now displays the payment method details properly.

Here is a patch that adds fields to store card payment details and adds it to the payment view.

I considered adding an update hook to copy payment method details to payments from existing orders. However the payment methods could have been edited since then and it would be copying over incorrect information.

I will attempt to do some more debugging on this but I'm still unclear what code handles commerce order email sending when the override is disabled. It definitely is not using the existing twig template like it did in 1.2.

Ok that makes sense. I was unaware there was a config override happening. Today while messing with it I discovered that the resend email button is in fact still missing when I went back and changed emails to enabled so I think it's safe to close this issue as user error on my part. Apologies.

Added further explanation and tested in a base install with only commerce and symfony_mailer