NH, USA
Account created on 25 January 2007, about 19 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States damienmckenna NH, USA

I created a MR that adds an "enable logging" option for the main settings page, and provides an update script to set it for existing sites.

🇺🇸United States damienmckenna NH, USA

Done.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

I just ran into this, the patch/MR fixed it for me. Thank you!

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

Done.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

A quick thought: how about we split this into two tasks - first convert the group plugins as there are only a few of them, then in a separate issue convert the tag plugins. We also need to retain backwards compatibility for now and drop the old ones in Metatag 3.

🇺🇸United States damienmckenna NH, USA

A temporary solution is to install the Default Content module , that takes care of the missing pieces and is 100% compatible with the exports of the core command.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

Moving to the main Commerce module as that's where the UI comes from.

🇺🇸United States damienmckenna NH, USA

In my case the shipping methods were nested like this:

  • condition 1
      • condition 2
  • condition 3

Once I fixed the nesting it worked correctly.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

Done.

🇺🇸United States damienmckenna NH, USA

Committed, and I updated the project page. Thank you.

🇺🇸United States damienmckenna NH, USA

The README.md file already includes a section of "Recommended / related modules", so the new items should have been listed there.

This is what I thinking of, and thanks to jwilson3 for suggesting the ecosystem page.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

I tagged 8.x-1.2 including this fix, thank you for the reminder.

🇺🇸United States damienmckenna NH, USA

I tagged this release so that there's a stable version available that supports Drupal 11.

Done: https://www.drupal.org/project/maillog/releases/8.x-1.2

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

An alternative approach would be to leave it enabled but add a line like this in your settings.php file on production:

$config['reroute_email.settings']['enable'] = FALSE;
🇺🇸United States damienmckenna NH, USA

Agreed that it would be cleaner to change this via a text file or a cspell setting.

🇺🇸United States damienmckenna NH, USA

Seems straight forward.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

When you add the module via "composer require 'drupal/commerce_usps:2.x-dev@dev'" the "@dev" part tells Composer to ignore the normal "minimum-stability" setting and install the dev version, it should just work that way.

🇺🇸United States damienmckenna NH, USA

2. Install the dev version of 2.x
This may require you to edit your composer.json file. Change the line
"minimum-stability": "stable"
to
"minimum-stability": "dev"

You shouldn't do this. This sets the default allowed release on all dependencies, so the next time you do "composer update -W" you'll give it permission to install dev versions of everything it wants to, which is a bad idea.

The single "composer require" line is enough to make your site download the dev version.

🇺🇸United States damienmckenna NH, USA

After a cache rebuild this is working.

🇺🇸United States damienmckenna NH, USA

This is how Commerce displays the field:

      $form['mode'] = [
        '#type' => 'radios',
        '#title' => $this->t('Mode'),
        '#options' => $modes,
        '#default_value' => $this->configuration['mode'],
        '#required' => TRUE,
      ];
🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

This needs to be added as a starting point for all other changes.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

After setting up the full ATK system via the recipe, etc it has created the following files and directories:

  • composer.json - Copied from ATK/module_support/development/composer.json.
  • node_modules/* - Comes from the npm system used to run Playwright. Should be ignored in the repository via .gitignore.
  • playwright-reports/* - Created by Playwright when it runs the tests. Should be ignored in the repository via .gitignore.
  • test-results/* - Created by Playwright when it runs the tests. Should be ignored in the repository via .gitignore.
  • tests/atk_* - Copied from ATK's codebase, the "playwright/e2e" directory's subdirectories are copied here..
  • tests/data/* - Copied from ATK, example data for use in the tests.
  • tests/support/atk_*.js - Copied from ATK's codebase, the "playwright/support" directory's files are copied here.
  • tests/support/loginAuth.json - Generated by ATK, contains site-specific cookies & details for this specific environment, used in the ATK authentication commands.
  • package.json - Lists all dependencies for Playwright. Copied from ATK'/module_support/development/playwright.package.json when using Playwright.
  • package-lock.json - Generated by "npm install", lists the packages currently in use.
  • playwright.config.js - Primary configuration for Playwright. Contains several lines specific to the project. Copied from ATK's module_support directory and then modified by Automated Testing Kit Demo's event subscriber onRecipeApplied() method
  • playwright.atk.config.js - Custom variables for Playwright defined by ATK for use in the tests. Copied from ATK's module_support directory and then modified by Automated Testing Kit Demo's event subscriber onRecipeApplied() method
🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

Working on this today.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue. See original summary .

🇺🇸United States damienmckenna NH, USA

Thank you for the update. Is there a parent issue where the modified UX is being planned out?

🇺🇸United States damienmckenna NH, USA

I updated the MR to fix two minor issues.

🇺🇸United States damienmckenna NH, USA

damienmckenna made their first commit to this issue’s fork.

🇺🇸United States damienmckenna NH, USA

I created a MR of the patch, once the tests are committed in Add installation tests and functionality tests Needs work I can expand the tests to highlight this problem.

🇺🇸United States damienmckenna NH, USA

The tests pass now.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

damienmckenna changed the visibility of the branch 3322061-test-coverage to hidden.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

An older issue about changing the addressbook UI: #3207910: Improve Addressbook UI

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

Needs to be redone as a merge request.

🇺🇸United States damienmckenna NH, USA

Thank you for the patch.

As a reminder, once you're done with the changes for an issue please set the "Assigned" field to "Unassigned".

Setting this to "needs work" as the maintainers will likely want it turned into a merge request.

🇺🇸United States damienmckenna NH, USA

Shouldn't the composer.json restrictions be enough to control the module versions?

🇺🇸United States damienmckenna NH, USA

This seems to work just fine with both Telephone Validation and Telephone Formatter, and fixes a spacing problem with the field.

🇺🇸United States damienmckenna NH, USA

I missed one of the files from the MR.

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

@jsacksick: Would that be a change in Address, Commerce Shipping or Profile? And do you know when the change might be worked on?

🇺🇸United States damienmckenna NH, USA

That would be a huge UX improvement!

🇺🇸United States damienmckenna NH, USA

This doesn't seem to trigger the validation if other fields on the profile type are modified too, e.g. if I add a phone number field and that number is changed.

🇺🇸United States damienmckenna NH, USA

This seems to resolve the problem.

🇺🇸United States damienmckenna NH, USA

Standardized issue summary.

🇺🇸United States damienmckenna NH, USA

Moving to the Fedex module's issue queue.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

Rerolled for 8.x-2.x.

🇺🇸United States damienmckenna NH, USA

The tests pass.

🇺🇸United States damienmckenna NH, USA

I'm running into a related problem - if you add a new shipping address there's no way of saving the address so that the site can be then used to check the shipping address, and if you click "Continue to review" it gives an error that a shipping method hasn't been selected.

🇺🇸United States damienmckenna NH, USA

I was about to report this myself, so thank you for the fix.

It works great for me, thank you.

🇺🇸United States damienmckenna NH, USA

A simple patch to fix the Queensland item.

🇺🇸United States damienmckenna NH, USA

On our site, the order that hit this bug had this response from PayflowPro:

Payflow server response:
Array
(
    [RESULT] => 0
    [SECURETOKEN] => [token]
    [SECURETOKENID] => [tokenid]
    [RESPMSG] => Approved
)

The visitor's cart didn't complete correctly, so they submitted it again, with the same response.

🇺🇸United States damienmckenna NH, USA

Ran into this on a new site (that was upgraded from D7) with v2.0.0.

🇺🇸United States damienmckenna NH, USA

The shipment items copy over the item's SKU as the label. On D7 this was rendered as the product label, not the sku, so on D10+ we should decide if we want to either a) change the shipped item migration to copy the product label, b) leave it as-is and change the shipment item (via preprocessing?) to show the label.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

I opened a PR to fix the problem: https://github.com/WhatArmy/FedexRest/pull/43

You can add this to your composer.json file's "extras/patches" section like this:

            "whatarmy/fedex-rest": {
                "#42: Fix for Queensland.": "https://patch-diff.githubusercontent.com/raw/WhatArmy/FedexRest/pull/43.patch"
            }

Of course it is recommended to download the patch file and store it in your repo, then adjust the path accordingly.

🇺🇸United States damienmckenna NH, USA

I opened an issue on the Fedex-Rest repo: https://github.com/WhatArmy/FedexRest/issues/42

🇺🇸United States damienmckenna NH, USA

.. At the same time, if it's just the data submitted to FedEx then it's probably fine.

IT might be worth changing this in the Fedex-Rest library directly.

🇺🇸United States damienmckenna NH, USA

Removing data from the record seems like the wrong approach - why not just expand the column to allow larger addresses?

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

Committed.

🇺🇸United States damienmckenna NH, USA

This fixes it.

🇺🇸United States damienmckenna NH, USA

Thank you for working on this so quickly.

I downloaded the new version, added "iframe" to the no_load setting, I still get the error in the console:

SecurityError: Blocked a frame with origin "https://example.ddev.site" from accessing a cross-origin frame. Protocols, domains, and ports must match.
ed11yInitializer — editoria11y-drupal.js:219
(anonymous function) — editoria11y-drupal.js:619

In my browser I added a breakpoint on line 103 of editoria11y-drupal.js. While the debugger is sitting with the breakpoint I see the error logged *twice*.

🇺🇸United States damienmckenna NH, USA

And now the tests pass.

🇺🇸United States damienmckenna NH, USA

This was already fixed in 🐛 The entity type domain_path_redirect does not have a "published" entity key Active , our site just needed to rerun the update script.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

This works as a starting point - thank you!

🇺🇸United States damienmckenna NH, USA

I've added this to a site, and the two Commerce patches, but I'm not seeing any UX differences during checking- should I?

The checkout flow is set up to have the Payment Information pane on the Order Information step, an Authnet Hosted payment option shows on that step, selecting the payment method and clicking "Continue to review" takes the visitor to the review page and then shows the Authnet Hosted iframe. Is that expected?

🇺🇸United States damienmckenna NH, USA

Should the setting be namespaced, e.g. "commerce_use_two_step_add_payment_form"?

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

FWIW I've found that the iframe is inconsistent in how it loads on page, most specifically sometimes it doesn't load with the correct height, so the "pay" button isn't visible, or even the recaptcha isn't visible:

🇺🇸United States damienmckenna NH, USA
🇺🇸United States damienmckenna NH, USA

A simple check:

diff --git a/src/FedExRequest.php b/src/FedExRequest.php
index b4a78f3..e83c5b9 100644
--- a/src/FedExRequest.php
+++ b/src/FedExRequest.php
@@ -17,9 +17,15 @@ class FedExRequest implements FedExRequestInterface {
    * {@inheritdoc}
    */
   public function getToken(array $configuration): string {
+    $api_key = $configuration['api_information']['api_key'] ?? '';
+    $api_password = $configuration['api_information']['api_password'] ?? '';
+    if (empty($api_key) || empty($api_password)) [
+      throw new AuthorizationException("API key or password is empty.");
+    ]
+
     $auth = (new Authorize())
-      ->setClientId($configuration['api_information']['api_key'])
-      ->setClientSecret($configuration['api_information']['api_password']);
+      ->setClientId($api_key)
+      ->setClientSecret($api_password);
 
     if ($this->getMode($configuration) === 'live') {
       $auth->useProduction();
🇺🇸United States damienmckenna NH, USA

Please reach out to me via my contact page and I'll see if I can get to the bottom of this.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

FWIW the error happened on our site because the client secret wasn't saved in the settings. But it's still a bit annoying that the error message is truncated like that.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

Our client has also asked about being able to filter the list of available rates, similar to #41.

It might be worth marking this issue fixed, tagging a beta release, and then doing separate issues for improvements, maybe add a plan issue to coordinate the list of tasks that need to be completed.

🇺🇸United States damienmckenna NH, USA

damienmckenna created an issue.

🇺🇸United States damienmckenna NH, USA

+1 for the MR, it resolved the problem for me on a new project.

🇺🇸United States damienmckenna NH, USA

I've done some testing with the module and it seems to be working correctly - in "test" mode at least, API requests are returning shipping options for both domestic and international orders. Great work tBKoT!

Production build 0.71.5 2024