Recent comments

🇺🇸United States rszrama

Looks good to me, and we can certainly update the project page images. What's best practice with respect to Project Browser? Should we be showing administrative interfaces or perhaps other visuals? Happy to look at examples that look great in context and do likewise.

🇺🇸United States rszrama

Even if Stripe owns it, the integration would require a separate module.

🇺🇸United States rszrama

We've been running fine on D10 for a while, though there were known issues with 10.3. It could ultimately be a filesystem issue or else have been resolved by work to improve 10.3 compatibility. Not clear.

🇺🇸United States rszrama

The "field_block:commerce_product_variation:physical:price" was not found is what shows in the logs.

🇺🇸United States rszrama

Added you as a maintainer today.

🇺🇸United States rszrama

Absolutely! Haven't touched this in forever, can't even remember why we developed it. Enjoy. : D

🇺🇸United States rszrama

I really don't understand all caps and bolded "yelling" in a forum thread like this one, especially as Kristen is just trying to be helpful. What's even more confusing to me is how you state Drupal Starshot is the wrong answer to a problem you've diagnosed, and then you go on to lay out its exact product strategy in your response as though Starshot is missing the point. Perhaps you need to read the strategy document for yourself before assessing it as aiming in the wrong direction?

https://dri.es/introducing-drupal-starshot-product-strategy

Just to quote one relevant portion from Dries's summary in that blog post,

Prioritizing Drupal for content creators, marketers, web managers, and web designers so they can independently build websites. A key goal is to empower these marketing professionals to build and manage their websites independently without relying on developers or having to use the command line or an IDE.

Are you not aware of its Automatic Updates Initiative ? Are you aware not aware of its Experience Builder Initiative ? It's fine to have opinions about the merits of these initiatives, but you're essentially yelling at Drupal for not doing the very things it's doing. 🤷‍♂️

🇺🇸United States rszrama

Yeah, really sorry for jerking you around on this. The maximum utility approach is actually to store the value in a field that can be more easily embedded / used in other contexts. I'd propose customer_comments provided by commerce_order, with the Checkout module providing a checkout pane for populating this field and an event subscriber for recording the contents of this field at checkout completion to the order log. This provides some auditability while still permitting the field to be used for other purposes.

If a headless site then wants to populate this field through some other means, or if the field is made editable or whatever, then those use cases would be responsible for their own event subscribers that write the contents of the field to the log at their own consequential times.

Additionally, since we're still copying this into the activity stream, we can still also use it in concert with some theoretical log message emailing system in the future, in a view of comment related logs, etc.

🇺🇸United States rszrama

Agreed re: reacting to checkout completion; makes more logical sense, too.

Let's also remember to have the event subscriber delete the message out of the order data array when it's transferred... unless, would it be easier to keep it in the data array to also be able to define an order comments token / theme template variable to make it easier to include in emails and user interfaces?

🇺🇸United States rszrama

Another option in the case of there being potentially multiple licenses created from an order would be to combine the licenses created into a single log message. The problem is our templates don't really provide for a variable number of token replacements ... almost like we'd need to just compile all license IDs and labels into a single replacement for a "Licenses created from order: ## from order item label." where everything after the colon is a single replacement.

That said ... I'm not sure we really need to optimize for this to start. We can ship with one log message per license created and perhaps add later a log template that consolidates multiple created licenses into a different template.

🇺🇸United States rszrama

Unfortunately, this is not supported by the 3.x branch. The module adds a license reference field to any order item with the OrderItemLicensed trait with a single value cardinality, meaning you cannot use a single product variation to grant multiple licenses. I don't know why the module was writen that way, as I can envision multiple scenarios where granting multiple licenses would be desirable, but the only way to accomplish this right now would be through a custom license type.

That said, if you want a no code option, you could use a Buy X, Get Y promotion to add a free product to the cart that represents one or the other of the licenses ... thus when the customer completes checkout, they actually have two order items that each generates a license.

🇺🇸United States rszrama

Unfortunately, this is just not how the Role license type works. It exists to grant a role to a user upon license activation, not to revoke a secondary role. Your options are to create your own license type that supports this logic, react to license activation to toggle that secondary role in an event subscriber, or hook into some other means of observing when roles change to make it happen. It's possible the ECA module would have features to support this without code, but I haven't looked into it myself.

🇺🇸United States rszrama

I've added documentation to the user guide:

https://docs.drupalcommerce.org/v2/user-guide/dashboard-inbox.md

This includes a full listing of the various settings you can use to govern what does or doesn't appear throughout the Commerce backend from a dashboard / promotional standpoint. It scopes the messages a bit as in our blog post on the 2.37 release, though the real answer is, "We aren't using it much ... in fact, we haven't used it at all since launch." We'd like to, though, and we would definitely encourage any contributed module maintainer who adds or changes significant features to create messages via update hooks as in Commerce Core:

https://git.drupalcode.org/project/commerce/-/blob/8.x-2.x/commerce.inst...

If you're running a marketplace you'd likely need serious customization to make this useful ... e.g., ensuring that the store filter is applied to the current user's own store and the selector is hidden (or only shows stores they have access to manage). It's minimal by default, though, so you can either extend it (e.g., via hook_commerce_dashboard_page_build_alter()) or interact with it at will!

🇺🇸United States rszrama

Ahh, thanks! I didn't look too deeply into where the checkbox came from. I'm going to relate this one to another shipping UX improvement I proposed for the order edit form, too.

🇺🇸United States rszrama

I do think there's some small risk here that subscriptions payments could be impacted, but I'm struggling to conceive the hypothetical scenario.

For example, let's say the "close order" queue was populated and then the module gets updated before all queued items can be processed. The update then updates all those draft recurring orders to use the new billing period... well, there's no double checking in the queue processor to see if it's really the right time to close the order. They'd close like normal.

Additionally, let's say the order got updated right around the time it was supposed to be enqueued for closing. (Or let's say the queue processor actually did double check to make sure it's the right time for an order to close.) Well, the coordinated change to the logic regarding when a prepaid subscription's draft recurring order should close (switching from the start to the end of the billing period) will be in place. No harm, no foul.

I think the concerns (closing previously enqueued orders vs. updating draft orders) are separate enough that there's little risk. Maybe we just need to make sure we leave non-draft recurring orders for prepaid subscriptions alone ... that will create some apparent billing duplication, but that's easily explained if it becomes a customer service issue.

Can anyone else think of a scenario where a payment may actually be double charged or skipped? Maybe we just need to ensure the enqueueing of an order for closure can't happen twice? (But even then, if closure was attempted a second time, the default implementation in RecurringOrderManager will skip the order if it is completed or paid.)

🇺🇸United States rszrama

Looks fine to me. It seems we have varying types of support for "Commerce Recurring without Commerce Product," though in practice I bet there are approximately 0 people using it that way. 🤣

Still, if moving this to optional doesn't bother anything, works for me!

🇺🇸United States rszrama

+1'ing this issue, as we've just encountered it. The scenario is exactly the same: we're using a prepaid billing schedule and are seeing draft recurring orders that appear to be for the previous billing period, the one that was already prepaid. That said, I'm not sure I fully understand the proposed solution or latest patch.

The original post states that "the first recurring order should be treated as the trial order and its price should be set to 0." This is either imprecise or just not correct for all scenarios. In our case, we still charge the customer the current month's prorated fee upon checkout, so it would not be appropriate for either the initial order or the first recurring order to be set to $0.

Additionally, the patch separates the single Cron::enqueueOrders() function into separate functions for prepaid vs. postpaid billing schedules. I don't see why this is necessary. Can we not just use a single query that uses condition groups to find those prepaid orders whose start date has passed and postpaid orders whose end date has passed? This will reduce code duplication, given we have to perform the same checks on the loaded orders for cancellation regardless.

At the end of the day, what we need to see are:

  1. Subscriptions with prepaid billing schedules: my draft recurring order should be for the billing period that ends after my current one. Thus, if I subscribe today with a fixed monthly billing schedule that renews on the 1st of the month, my draft recurring order should show that it is for August 1 - September 1.
  2. Subscriptions with postpaid billing schedules: my draft recurring order should be for the current billing period. Thus, if I subscribe today with a fixed monthly billing schedule that renews on the 1st of the month, my draft recurring order should show that it is for July 1 - August 1.

There are some nuances we need to explore with respect to trial periods, but the gist of the fix for the initial order placement is that prepaid billing schedules should result in the next billing period being generated after the first is. (In the patch, I'm not sure I follow the logic of the change for this in RecurringOrderManager::startRecurring().)

I'm also not sure what we should do for the billing period with to fixed prepaid schedules and free trials, as the adjusted start date code is a little funky ... but maybe the answer is that free trials in these cases just aren't super appropriate? If a subscription always renews on the 1st of the month, what's the use case for a free trial vs. a 100% off coupon for the first month or something? 🧐

🇺🇸United States rszrama

5px works for me. Matches what we're doing elsewhere in the template.

🇺🇸United States rszrama

Yeah, this is a weird one. I don't know why order placement was selected instead of checkout completion. That would seem the more natural event to subscribe to for sending a receipt.

🇺🇸United States rszrama

Yeah, the text could be made more dynamic. By default, without auto user creation, you'd use the "pick a password" style form to create an account upon checkout completion, so it makes sense for the default configuration.

🇺🇸United States rszrama

I think this may need to be tested against payment gateway integrations like the Stripe Payment Element. They are technically implemented as offsite payment gateways even though all payment occurs on site. I haven't applied this patch to review the before / after, but I wonder if that use case was considered when this patch was written.

🇺🇸United States rszrama

You would typically do this in your theme, not a custom module:

https://www.drupal.org/docs/develop/theming-drupal/twig-in-drupal/workin...

To override another module's template with your module, you'd need to alter the theme info I believe.

🇺🇸United States rszrama

Sorry this went unanswered; this is still the best option out there, will be looking to land a full release this summer.

🇺🇸United States rszrama

Yeah, main issue is just getting a full tagged release, which we're doing planning for right now to attempt in July. 👍🏼

🇺🇸United States rszrama

Interesting. What checkbox are you talking about? It may be heavily payment gateway dependent, as there is no such checkbox in Commerce Core. What's the use case for creating a subscription in the first place if you're not actually expecting recurring payments?

🇺🇸United States rszrama

I can't reproduce this unfortunately, even on the latest dev version. Is there perhaps something else interacting with your form?

🇺🇸United States rszrama

It appears we'd essentially want to create a script in this module (or another) that scrapes https://chromium-i18n.appspot.com/ssl-address to fetch country subdivision level postal code patterns for reference.

🇺🇸United States rszrama

Thanks, this is great. Definitely needs to be in for the exact use case mentioned.

A couple minor tweaks I'd request before we commit:

  1. Rename the condition current_user_role.
  2. Rename the class CurrentUserRole to match (along with the related comments).
  3. Relabel the condition User role.
  4. Let's introduce a new more general category called Current request.
  5. I don't think we need the weight; I'd just remove it.

My rationale for the machine name / label mismatch is that I think "Current request" makes sense as a group (think it's better than "Global" or "Context"), and we don't need the redundant "current" in the label ... but it still makes sense to identify in the code.

My only other consideration is that since we're adding this new, we might like the opportunity to build operator selection into it. Right now the configuration element is titled Allowed roles, but we could make this two elements:

  1. Matching strategy radios element with options User must have a selected role, User must have all selected roles, and User must have none of the selected roles
  2. Roles checkboxes element with the various roles as options.
🇺🇸United States rszrama

Enh, I didn't look at the Element class to confirm apparently. Then perhaps this feature request is just completely unnecessary?

🇺🇸United States rszrama

I'm not entirely sure of the approach here. Is it truly a given that reassigning the initial order of a subscription should always reassign the subscription and every order ever associated with it? I may not understand the use case, but this seems like a pretty big assumption. At the very least, we're talking about exposing the PII of the previous owner to the new owner without ever providing an opportunity for this decision to be reviewed.

Also, why wouldn't the original user still own their prior orders? If this were a donation to a 501(c)3, for example, the original donor would need to maintain access to their historical order record for tax deduction purposes.

I think I'd feel a lot more comfortable with this being governed by an opt-in checkbox on the order reassignment form. In that case, the behavior would be explicitly enabled, and we would even be able to spell out exactly what would be assigned to the new initial order owner along with the order itself. We might even consider multiple checkboxes: one for reassigning the subscription and another for reassigning all recurring orders associated with the subscription.

Finally, I'm not sure what it means for the subscription to be reassigned but the payment method be someone else's. It could be the act of reassignment needs to be paired with a new payment method reference - one already owned by the new user. Maybe knowing more about the current use cases could clarify what should happen there, but moving a payment method should definitely require an explicit opt-in and may also need to support selecting a different payment method associated with the new user.

🇺🇸United States rszrama

Oh, interesting ... thinking there's some kinda caching issue here where one subscriber doesn't know what a previous subscriber has done to the order? Not just a matter of priority, is it?

🇺🇸United States rszrama

Really no clue, I've never used the webprofiler module ... but if there's a circular reference, can definitely confirm it isn't a regular issue in Commerce sites. 😅

🇺🇸United States rszrama

Unfortunately, this feature request wasn't specific enough to be actionable. I'm going to interpret it a bit and retitle the issue. Ultimately, I think what's needed is for us to add the ability to apply access control to the options shown in and input accepted by our commerce_entity_select widget, which lets you select the store a product is published to, a promotion is available for, etc.

There are certainly other areas of marketplace requirements that have been advanced through other modules, but this seems to be the path forward for this particular issue. Others would require their own specific feature requests.

🇺🇸United States rszrama

I built a similar View, and while I can reproduce the issue in the preview of a View, if I add a page display and visit the page, every row has a form element and works as expected. My initial hunch was that the preview would just be stripping the form to avoid embedding a form within a form, but that doesn't account for subsequent rows having form elements.

Ultimately, I'm going to guess this is an upstream issue - something in the way Views itself is processing these rows, doubtful that it has anything to do with our display formatter. Can you track this down any further to confirm?

🇺🇸United States rszrama

Yeah, this should be an easy fix. The one thing I'm not clear on is the language of the Shipping module itself. Typically, I'd talk about "discounting shipping" in the abstract or discounting the rate specifically, but a Shipment does have a field called "Amount" that apparently would reflect both the rate and any additional adjustments.

The problem, as with most things in Drupal, is different sites may use the system in wildly different ways. Perhaps it's best just to talk about "Fixed amount off shipping" or "Percentage off shipping"? Regardless, I agree that shipment could be a confusing word to use here.

🇺🇸United States rszrama

Drupal Commerce has a coupon reference field on orders that stores which coupons have been applied to the order. It's not an exact match to Google's data model, which appears to expect a single coupon, but we can grab the first one and add it to the purchase event template in:

https://git.drupalcode.org/project/google_tag/-/blob/2.0.x/src/Plugin/Go...

Would be nice if one of the options I've tagged into the "Deprecate Commerce GTM" list could be used to give merchants the opportunity to alter which coupon code gets used in the event of there being more than one. We could always sort them by their relative promotion sort orders so there's at least a consistent default with some UI based influence.

🇺🇸United States rszrama

Tagging and cross-referencing Add alter event processing to collector Active as one or the other (or both?) of these approaches will be needed. A more general solution for altering event data seems to make sense, though ... for example, determining what coupon code should be present on a purchase event when multiple coupon codes have been applied to an order.

🇺🇸United States rszrama

A less invasive solution may be proposed in Allow to alter order item data for commerce events Needs review ; cross-linking here.

🇺🇸United States rszrama

Not specifically sure the old module supported event level alteration, but it's a good idea and something I'm tagging to make sure we review / help push forward as part of that initiative.

🇺🇸United States rszrama

Sorry, this is standard composer stuff. We can document it a bit more, I suppose, but the output is telling you the issue - you need to match the PHP version expected by the packages, many of which are well beyond our control.

As I just posted in #3432848: DDEV installation instruction doesn't work , another option if you know the runtime environment will use the correct version is to append --ignore-platform-reqs to your composer command.

🇺🇸United States rszrama

If you can't match the required PHP version in your system (I use Homebrew and will often swap the active version), you just need to append --ignore-platform-reqs to the command.

🇺🇸United States rszrama

Should we try to close the gaps first? Will marking it deprecated generate warnings or anything in the Drupal back-end for module users?

🇺🇸United States rszrama

Here are the patches, one for each branch. I do wish we could actually surface the exception message in the UI, but ultimately I think we may just need some form enhancement to make it clear that the number would always be required and the file is optional. Not really sure of the intent behind the current form, to be honest.

🇺🇸United States rszrama

If I may expand the scope of this issue just a bit ... I actually think this module should be deprecated, and we should recommend everyone move over to Google Tag. Aside from having much greater adoption and development support, it's also already doing most of what this module was previously doing.

Given Google Tag already supports most features of this module, I think we should:

  1. Identify any remaining gaps via issues in that module's queue.
  2. Work with the maintainers to get them incorporated into a release.
  3. Provide documentation for folks to follow to migrate their configuration or custom code to the new module.

I began this by tagging a couple of issues with "Deprecate Commerce GTM" related to using placed vs. paid for the purchase event and expanding support for checkout tracking. If there are any other gaps, can we get them in issues using the same tag?

https://www.drupal.org/project/issues/search?issue_tags=Deprecate%20Comm...

🇺🇸United States rszrama

(I'm marking this and another issue with a "Deprecate Commerce GTM" tag. These are issues that fill gaps for us to formally recommend closing that module and migrating sites to this one.)

🇺🇸United States rszrama

Ahh, good catch. We might also just prepend the / if we see it's missing when parsing the link as a safeguard.

🇺🇸United States rszrama

Recategorizing. I've never seen this behavior, so it must be something environmental or on the site in addition to Commerce Core itself.

🇺🇸United States rszrama

@poker10, sure, I don't mind starting or joining such a discussion. Honestly, until 5 minutes ago, I didn't even know that page existed, and it doesn't actually offer any means of enforcement or appeal beyond "open an issue" if someone contravenes what's only described as a best practice, not a requirement.

The page is overbroad and outdated, and I don't think it's been referenced in a decade, at least not since a whole host of discussions have been held around developing sustainable business models for open source. It would've been nice for that to have been acknowledged and for Stavros to start this as a discussion about whether or not it was even intended to address cases like ours, not merely using it to support an a priori prejudice.

(The original context, I'm sure, was the preponderance of "Powered by X" links being added to front-end site footers automatically, because the company I worked for previously was part of the problem with the Ubercart footer link. 🙈)

I'll open a separate issue for that discussion if it doesn't exist, and in the meantime, we won't be changing anything here other than updating the module to respect the Commerce Core pattern added in Implement a settings pattern for disabling partner banners Active .

🇺🇸United States rszrama

@vensires, fair point; it wasn't developed in secret, and we didn't consider it to be too onerous to remove (or even necessary to remove in most cases, given this is almost always a one-time configuration interface), but until we had tagged a release that generated some feedback in Slack, the idea for a single set-it-and-forget-it kill switch for all such promotions just hadn't come up. I'm happy to take ideas / feedback prerelease, but reacting to what's in a new release is usually the way things like this come up. 😅

@skourak, I'm not persuaded by your points. I can't imagine a single awkward conversation generated by someone with store administrator permissions seeing their eCommerce platform integrates with some of the most popular related SaaS products in the world. I'm actually not concerned if other module ecosystem maintainers decide to do as we have - e.g., I'm already on the record of fully supporting Jake's efforts to advertise his services and sponsorships in the Webform UI. I even think the DA should experiment with more ways to raise funds for Drupal development and infrastructure maintenance, including messaging in the back-end, but that's a subject for another day...

@s.messaris, just to be clear, yes, Drupal Commerce is "owned" by Centarro in the sense that we've been the sole maintainers and primary developers (by far) for 15 years. That doesn't mean we should abuse our position, which we've never taken for granted, but it does mean that we are able to market our services and promote our partners in ways that people who merely use or occasionally contribute to the project cannot. I honestly don't think this should be controversial.

The only difference here vs. what we've been doing at least since the release of Commerce Kickstart 2.0 in 2012 is that we've promoted an integration that isn't already in a site's codebase from a relevant administrative interface. However, because it fills a prominent gap in the platform over which merchants have left, we want people to know exists. If we're able to retain merchants or grow our footprint in a partner's ecosystem and thus generate more funds for Commerce development from them, literally everyone will benefit.

We know that acting in bad faith as maintainers would harm the project as surely as not having critical features or integrations would. I hope our track record over the last 15 years buys us at least some credibility as good faith members of the community. However, we also know that funding the next 15 years of Drupal Commerce development is going to require millions more dollars. I believe the Drupal community should support maintainers experimenting with ways to find that funding.

🇺🇸United States rszrama

Obviously, I disagree that we're doing anything untoward by linking to useful, popular third-party services from relevant portions of the Drupal Commerce administration interface. We do it here and in the tax type interface, where we link to a free sales tax nexus tool and a free trial offer from a tax calculation service called AvaTax. I'm sure we'll do it again in the future, including for our own products and services.

Just because these services aren't useful to some merchants doesn't mean they aren't useful and important to cross reference for merchants in other parts of the world. For the sake of the platform's own attractiveness and health, we need people to know integrations like these exist - otherwise they leave for other platforms. Doing so like we have is an unobtrusive way to make that happen without requiring every merchant to give us an email address before installing Drupal Commerce, which is how other platforms would message these things. 😛

As to the policy you linked, I don't know what instigated its creation (probably theme spam from the early era of theme stores), but it hasn't been updated through any number of trends in Drupal contributed module maintenance. For example, Webform has very prominently featured advertisement in its interface for years, and I applaud Jake for taking steps to find a path toward financial sustainability. Without that, projects of Webform's or Commerce's size and scope simply can't exist, which is why we've even attempted to establish bi-directional Drupal Commerce partnerships with popular service providers. (Notably, the Free Software Foundation has always viewed commercialization of free software as critical to the movement's long-term success. I think we should generally be experimenting more, not less, while still respecting our community's norms.)

I'm happy to propose revisions to that policy, but I also think it's fair to question what it even means. 🤷‍♂️ It's framed as best practices, which, generally speaking, are bound to evolve and don't have to be followed. I think it's fair to warn people that their experiments may be poorly received if they violate certain community standards, but if we're going to attempt to enforce them at all, they can't be buried and forgotten for 15 years either. 😅 (For example, I think there's a big difference between contextually appropriate advertisements about integration modules that might enhance a store aimed at administrators vs. behavior that gets presented to a site's end users or otherwise disrupts their UX.)

In any case, your solution to simply remove the link to ShipStation wholesale is a non-starter for us. Elsewhere I've discussed adding a setting to Commerce Core to disable these sorts of third-party service banners and then updating our modules to respect them. This would allow a site developer to disable it once and never have to worry about future plugs, e.g., if we were to add a fraud prevention integration to the payment methods page. I'll add that feature request to Commerce Core now, and I'm happy to either just leave this issue lingering here or convert it to a task to respect the core setting once it lands.

🇺🇸United States rszrama

Generally speaking, this is a perfect case for a custom PromotionOffer plugin, because the UI to support every possible permutation of "only give free shipping for some products" would be quite unwieldy. The primary way I could see Commerce Shipping supporting this right now would be for stores using the FlatRatePerItem shipping method type, since we can easily recalculate the rate from the offer after filtering out specified items from the shipment before counting the quantity.

My proposal would be something like this:

  • Define a new ShipmentFixedAmountOffPerItem and ShipmentPercentageOffPerItem promotion offers.
  • The idea is that these will calculate either a fixed or percentage based discount off of the price per item as set in the shipping method configuration.
  • When calculating the discount amount, the plugin will basically need to determine how many items on the shipment should be discounted, sum up the total of the discount, and apply it as an adjustment.
  • The primary option for determining how many items to discount should be an "Item matching method" radios with options labeled:
    • "Charge full price for a specified quantity of items", which then prompts you to enter the number of items that should be discounted. This number could be 0 or greater, meaning you could use this for a free shipping method based on shipping per item.
    • "Discount the shipping per item for specific products", which then prompts you to select a "Product matching method" of "Any of the following" vs. "Any except the following", and has you reference products via a multi-value autocomplete.
    • "Discount the shipping per item for specific product variations", which is like the above but at the variation level.

This would give you the tools you'd need (assuming you're calculating the shipping rate by item quantity) and be flexible enough for other scenarios, especially the idea of giving free shipping on a subset of the product catalog but still charging for other items. This won't really help folks just using a flat rate for the whole order, but that's where a custom promotion offer plugin comes in.

🇺🇸United States rszrama

I think current_variation works, and maybe default_variation is an additional token we should consider supporting. 🧐

🇺🇸United States rszrama

Oh, and for what it's worth, we do use the pattern of a variable set in settings.php bypassing update hooks. Typically this involves field changes to entity types we expect may have a tremendous number of records in the database, allowing a site owner to skip the change in update.php and complete it some other way. It's not a bad solution, though I do expect the same sites that would disable it would tend to have Update Status disabled, too.

🇺🇸United States rszrama

fwiw, I'd advocate strongly in favor of automatic installation. (Naturally, as pointed out above. ; )

Here's why: I think the need for the Drupal project, and in cases like ours, maintainers of significant ecosystems, to communicate proactively with end users outside of module update hooks outweighs the potential concerns. Install any other piece of software, including many an open source project, and you're going to get proactive project messaging. (Reference a WordPress installation as one example; we referenced WooCommerce during our design phase.)

Building this into our software does not violate any social contract that I'm aware of, especially given the long history of RSS feeds in general. (In fact, if I had any gripe with the Announcements module, it's that it doesn't do more, e.g., to show the messaging in-site, allow me to dismiss messages I've read, etc.)

One concern raised in our issue that I think is worth considering is the idea of a man-in-the-middle attack getting bad information into the feed. I'm not sure what to say or do about that ... though I tend to think if drupal.org itself were compromised, we'd have much bigger issues to worry about than whether or not some graffiti made it into the announcements feed. 🤷‍♂️

The other concern raised was the potential for IP logging when the feed is consumed. I think this can be handled with a privacy policy. I tend to think IPs are generally of limited utility, given most are going to resolve to an AWS block, not a specific domain or organization. But it is possible if they were logged and those logs were disclosed that targeted attacks could be attempted against those machines somehow. Very theoretical. A good countermeasure is just don't log those IPs.

Thinking about the fractional increase in risk, too, most of these sites are already reporting in to d.o for update notifications, so seeing a machine fetch a feed is no different. (In fact, it would be more useful to attempt to track referer URIs from people who actually click on links in the feed, again, an action that could be disclosed and or prevented by a privacy policy.)

If we do need a compromise position, though, we could have an update hook choose whether or not to install the Announcements module based on whether or not the Update Status module is enabled. At that point, we're at least not increasing a site's exposure to d.o. For the sites where it isn't installed, we can put a nag either in an update hook or in the back-end. (i.e., we could add a dismissible message across admin routes strongly suggesting the module be enabled and / or a permanent message atop the status report page.)

fwiw, I'm not naive re: the fact that some of our users prefer maximum control over things like these; I just think they're in the minority position of being both vocal and highly active in Drupal already (meaning, they're the least likely to need the messaging, which is why they'd disable it in the first place). I would just recommend a solution that serves for the majority use case and makes the uninstall step clear for people who don't want the module.

🇺🇸United States rszrama

I'm not sure I understand the proposed solution here, as it appears we're now storing the CVV, which is not something we should be doing. (We also wouldn't use a patch that just comments things out like in the JS file here.)

Why is the CVV not being transmitted directly to the server?

🇺🇸United States rszrama

Good thought re: a privacy policy on the project page, and sorry for not doing something like that in advance of the release.

We were torn about a full on configuration form for the various dashboard components, to be honest. But looking at Webform now, I see that it does offer checkboxes for all manner of things that appear in the UI advertising support / development services.

(And I'll add this to the discussion in the governance queue, but we should consider the potential for referer URI logging of clicks on outbound links, whether to a module maintainer resource or even third party resources. I wonder if there's some way to prevent that?)

🇺🇸United States rszrama

Looks great to me! And just as a note, we can't easily include the uid in the autocomplete suggestion, because it would be duplicated on selection. I think it's fine for the suggestions to just be John Doe <john.doe@example.com>. Tested as working on the order create and reassign forms.

🇺🇸United States rszrama

This is not a bug in Commerce Core, but we appreciate your contribution! The docs are currently being migrated to ReadTheDocs.io, which means these PRs may need to be ported later.

🇺🇸United States rszrama

Always run update.php after updating modules...

Production build 0.71.5 2024