🇺🇸United States @morbus iff

I was just scanning core and the block of code in \Drupal\taxonomy\TermViewsData::getViewsData is a mixture of both styles.

I would agree that that needs to be improved to use only one style EITHER single-spaced or whitespace-aligned. Does "mixing of both styles" mean "in a single code block" (like your example array)? If it does, I would support a rule that says "use either single-spaced or whitespace-aligned within a single code block; don't mix them." This would still allow both options overall, but with consistency for a single instance.

@quietone: Your example would NOT be forbidden with the proposed revision, as array spacing is not mentioned (that is: "=>" is not part of "All binary operators (operators that come between two values), such as +, -, =, !=, ==, >, etc. need to have exactly one space before and after the operator, for readability. Alignment with extra whitespace is not allowed.". The proposed revisions are specifically about operators, not arrays.

tonytheferg → credited morbus iff → .

It looks like we have found an issue that people are very much set in their ways about. This is however something that we really need to find a rule for, because having both styles mixed in a codebase is not great in my opinion.

Huh? *No one* in the "allow whitespacing" camp is saying all grouped variables MUST be separated by columnar whitespace.

@quietone, yeah, that's the one I mention in #14.

It specifically *allows* more-than-one space BEFORE the operator, as I quoted above.

Running this against core would allow and NOT FLAG the following:

  // Multiple assignment with whitespace beforehand.
  $variable       = 1;
  $variableLonger = 1;

Thus, changing the code style docs to MUST "only 1 whitespace" would mean we have _no lint that checks for it_.

This is still a desirable feature.

If you head to commerce/config/avatax and uncheck "Validate addresses in checkout", the test scenario above works with no issue.

rszrama is suggesting this has something to do with Avatax.

morbus iff → created an issue.

Closing in favor of earlier ✨ Address validation Needs review .

Closing in favor of earlier ✨ Address validation Needs review .

Seems like we have two different patches going for this. Here and ✨ Address validation Needs review ?

majmunbog was the person who originally suggested the fix and I tested it and it definitely works, so RTBC.

morbus iff → created an issue.

It looks like this was removed (ID "6") in https://git.drupalcode.org/project/commerce_usps/-/commit/5633175f32e46c... as part of 🐛 USPS RateV4 API Changes on 9 July 2023 Needs review , but "Media Mail Parcel" still appears to be an option in the v4 API: https://www.usps.com/business/web-tools-apis/rate-calculator-api_files/r...

morbus iff → created an issue.

Some code comments:

1. I'd remove the #max entirely.
2. I think I'd change the promotion label to "Percentage off each matching product with fixed quantity". Or potentially shorter: "Percentage off each matching fixed quantity product".
3. Same "to" to "with" change for the file documentation header.
4. We're likely gonna need some tests.
5. Too much closing bracket indent spacing in defaultConfiguration().
6. I think I'd labeled "Max quantity" to just "Quantity". Maximum is implied.
7. We can likely get rid of the #description too - it's all implied as part of the promotion label.
8. The "Quantity must be a positive number." message could be replaced with "Quantity must be higher than or equal to 1." which is a similar error that is tested against in AddToCartFormTest.php.

morbus iff → created an issue.

@anybody: aye, this issue still existed for @vipin.j and I because we ran into it during our own QA of our platform.

@jsacksick: Does this contain the ability for an admin to communicate back to the customer via comments? (I've been, and still remain, on a holiday as I'm moving doing the college move-in thing for a kid, so haven't had a chance to follow along with this recent flurry of dev.)

jsacksick → credited Morbus Iff → .

Morbus Iff → created an issue.

@vipin.j, tests are failing.

@vipin.j, we need to update this patch. Core's username and password no longer have #descriptions (see https://www.drupal.org/node/3377041 → ), so we don't need those in this patch anymore, but the "Log in" and "Create new account and continue" tweaks should stay.

As you say, you have multiple issues, but "flat + increase by item" is available as a patch here: ✨ Add Shipping method with combination of Flat rate + Flat rate per item Needs review

@AlfTheCat, #16, you're almost there. This is what we use:

/**
 * Hide third-party messaging from the Commerce dashboard.
 * Example URL: admin/commerce
 */
.commerce-dashboard {
  display: block;
}
.commerce-dashboard--inbox {
  display: none;
}
.commerce-order-dashboard-metrics-form {
  margin-top: 2rem;
}

nags, please see #143, where future development is happening.

(A small wrinkle that I hesitate to mention because it sounds like I'm angry or annoyed and I'm not:) The first inbox fetch is in commerce_install(), which makes it difficult to stop from happening before "it's too late". That is: if you're concerned about sending data to Centarro, the "commerce_dashboard_fetch_inbox_messages" setting doesn't help very much (especially if you're installing Commerce as part of an enterprise installation profile like we are, where settings.php doesn't exist until AFTER the installation profile has finished). By the time you have a chance to disable it (after the installation profile is finished, after you enable the module "just to try it out", etc., etc.), you've already pinged them.

Morbus Iff → created an issue.

With this patch committed, looks like #40 is still an issue for me.

Thanks @jsacksick!

Here's what we see when we apply this patch - more than 10 options.

(Note that we're also running a Commerce License patch of ✨ Integrate with Commerce Email for license renewal and expiration Needs review .)

Nevermind. See https://www.drupal.org/project/commerce/issues/3413778 🐛 No link in dashboard to product attributes page Fixed .

Morbus Iff → created an issue.

Marked as wontfix because a) this causes all the tests to fail under D9, b) the revised YAML uses stuff from D10 (thus causing the D9 error), c) Commerce for 9.x is still being supported. Morbus and Vipin will maintain this patch (which we're running locally) going forward, and resubmit when/if 9.x support is dropped.

Closed because of the test failures. Which appear to be caused by core:

https://www.drupal.org/project/drupal/issues/3400522 🐛 TimestampFormatter / time_diff missing config schema Needs work

Likely related to https://www.drupal.org/project/commerce/issues/3401590 📌 Dispatch the ORDER_PAID event for free orders when OrderStorage is destroyed Active .

This post does not have even a hint of enough detail to respond to.

Closing.

@jsacksick and others: note that this patch is currently "recommended" in Commerce License's README: https://git.drupalcode.org/project/commerce_license/-/blob/688ec78bb4935...

#3 satisfies my use case for in #2. Yay!

@jsacksick, thanks for the pointer.

Does Commerce Core always expect YYYY for the payment expiration date?

I'm not sure calculateExpirationTimestamp() is the _only_ answer, at least, because that also expects a YYYY, but Authorize.net is only asking for (from the user, in Accept.js UI) a YY and also only returns a YY after all is said and done. That then gets saved into commerce_payment_method__card_exp_year.card_exp_year_value. The AcceptJs code DOES call calculateExpirationTimestamp(), but I'm guessing it's only passing in YY.

We'll be looking into this a little more closely in the next few weeks.

This is also happening on user/#/payment-methods/#/edit, where Authorize.net is returning (and Commerce is saving into the database) YY, but the select dropdown for year expects YYYY. The month displays fine, but the year will always be the "first item in the dropdown" (i.e., no selection whatsoever).

Morbus Iff → created an issue.

Is a "license renewal" different from "the first time a license was created"? Internally, we're specifically looking for some customizable way of sending an email when a license is created (either via a normal checkout workflow, or because an admin has manually added a licensable order item to an order).

For existing sites, this new functionality is... crazy.

Whether the DSM override is enabled or not, the DSM module takes over the Commerce Order types UI (admin/commerce/config/order-types/*/edit), so that an admin can no longer set Commerce's native BCC and Subject line fields. When it IS enabled, it seems to create multiple policies (a default one for ALL/ALL, then an imported one for Receipt/orderTypeID) and NEITHER of those show up in the Commerce Order types UI.

When the DSM override is enabled, the "sensible Body default" seems to use the "current user's permissions" to send out the email, such that an admin testing an order will see *admin-only comments* in the email AND see form controls for adding comments (as an admin would normally see on admin/commerce/orders/#).

And I've yet to get "have the DSM override enabled, but allow continued use of Commerce's default twig template for the Body". If I delete the Body override element entirely, it somehow still uses the "sensible Body default", sending out the bad unstyled HTML (caused by another issue: Warning: file_get_contents(default/files/css/css_I5WrM2XNc4EO21P353X1Tp3NJ4d0oh87oqBOCBGEP18.css?delta=0&language=en&theme=edv_bootstrap&include=eJxLTSmLT8rPLykuKUos0E_NTczM0SmuzE3Lz6uMB3FSi_ST83NzU4uSU-Pzi1JSiwDuVRPD): Failed to open stream: No such file or directory in Drupal\symfony_mailer\Plugin\EmailAdjuster\InlineCssEmailAdjuster->postRender() (line 78 of modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/InlineCssEmailAdjuster.php). ) with admin comments.

@tBKoT: patch branch needs to be rebased.

This is a very similar problem to https://www.drupal.org/project/radix/issues/3378737 🐛 Class rename broke included block classes Fixed - variable naming was changed, without actually changing the parent functionality to match.

Morbus Iff → created an issue.

Morbus Iff → created an issue.

@Anybody: we use both, aye. That module can work for "when the customer leaves the admin a comment"; this issue (and #3267366) handles the other end - an admin responding back to a customer about their order.

Closing. Breakpoints for Bootstrap are provided in the subtheme for both Bootstrap 4 and 5.

Closing due to lack of OP response.

I think just

@import '~bootstrap/scss/variables-dark';

would be the better approach, right?

The bigger concern seems to be that switching to OAuth requires switching to their new REST API - currently, we're using a library that uses the XML API.

I had the same worries about the #ajax stuff and was going to bypass it by formatting the options as "view_id: display_id".

Cause is somewhere in https://www.drupal.org/project/commerce_license/issues/2943888 ✨ Add the ability to re-purchase a license to extend it Fixed .

This patch appears to be partly responsible for critical error in 3.0-rc1: https://www.drupal.org/project/commerce_license/issues/3362663 🐛 RC1: Can't add more than one digital product to cart Fixed

Confirmed same issue here. A dealbreaker.

Ah! Got it. An echo of #36.

I admit I was confused by this one, at least until I wrote a big long comment explaining why I was confused and then realized: we're talking about a site with multiple Order types, right? If I've got five different Order types with five different needs for Order Item display, we DO NOT want to "make five different custom Views", but rather add four more displays to our currently selected View?

Gotcha. That makes sense.

@jsacksick: What do you mean? There is a setting:

+    $elements['order_item_view'] = [
+      '#type' => 'select',
+      '#title' => $this->t('Order item table view'),
+      '#description' => $this->t("Only views tagged with commerce_order_item_table are displayed."),
+      '#options' => $available_views,
+      '#required' => TRUE,
+      '#default_value' => $this->getSetting('order_item_view'),
+    ];

@DamienMcKenna, to be clear: the node count feature that was in previous versions of Taxonomy Menu is now out-of-scope for Drupal 9+? Were there previous discussions on removing this feature that I could read?

While #23 works for me without any of the problems other folks have experienced (yet), I think I have an example of #39 too. On product A, I have a "Similar products" entity reference field, which references product B. Now, if I make a view called "Similar Products", intended to be displayed on product A, then it'll take the current entity as the argument (A), relationship load the entity from A's Similar Product field (B), then relationship load the variants from that relationship (B's variants). At this point, if I display the product variant view field "Image", then B's variant's images will be displayed (both in the View preview and on the product A entity). But, if I tweak the view from Fields display to Rendered entity, then I will see the correct B's variant's image on the View preview but, on the product A entity, the image will always show Product A's Image instead.

#19 works for me.

Has anyone gotten this to work with POSTING a new node, with a layout_builder__layout configuration? Every time I add my own layout_builder__layout configuration in the node/TYPE POST request, the node is submitted successfully, but the layout builder configuration is ignored entirely.

#23 works for me. I've not been able to get the MR to apply.

Morbus Iff → created an issue.