🇺🇸United States @rszrama

rszrama → created an issue. See original summary → .

rszrama → created an issue.

rszrama → created an issue.

Works great for me!

Nice review, Dmytrii! I agree on all counts. One suggestion would be that we treat this similar to shipping method selection in the checkout form. Essentially, have a button as part of the widget itself, since we control the widget, that refreshes the form when clicked to show options from the currently selected customer. Then use JavaScript to hide the button and "click" it when the customer changes.

I don't think we really need to care about the scenario of there being no customer. If there is no customer, we can't reasonably select a reusable payment method anyways. In that scenario, I tend to think it's fine for this form to just not submit, since there would be no options for selection of a required field ... it would be on the person customizing the form to sort it out then.

Agreed re: changing which base widget we're extending.

Sorry about that; redeployed the app and made a note to ensure it doesn't expire again!

Awesome!

Yeah, there's that one and then a Learn more about credit attribution → in the "Attribute your contribution" fieldset.

rszrama → created an issue.

rszrama → created an issue.

Looks good to me! Thanks for including the test!

Nice refinement for #6! I like it. 😄

On testing just now, though, I noticed an odd bug. When you add a new order item to an order, draft or placed, and save, the order total does not get recalculated. This typically occurs during order presave, so all I can figure is there's some mismatch between the inline entity form process and the new widget regarding when order item totals are being calculated and / or order items are being saved to the order relative to when the order total recalculation occurs.

I took a video I'll share in Slack as I'm not sure how to embed them in tickets here. 😝

Cool, I really like the implementation! I think it establishes a good pattern for others to follow when customizing their stores, too. We'll need to document it. 😅

Minor suggestion for improvement here. I like the idea of adding the SKU to the input, but not necessarily as a new column. Perhaps we can make it as a sub-line on "Title"? This will also more elegantly degrade in the event an order item doesn't have a SKU; you won't end up with just an empty cell in that column.

Additionally, I think we can make the form more concise by removing the order item type. We don't show that on the view page anyways, and it's an implementation detail a CSR shouldn't care about. This will likely result in more natural inlining of the operations buttons, too.

(That said, it might also just make more sense to get these into a drop-button with "Edit" as the default, particularly once you enable "Duplicate.")

Mocking up all of the above (except the drop-button), I'm imagining:

Other minor changes needed:

1. We should use the previous labels for checkboxes in the widget settings form: "Allow users to add new order items." and "Allow users to duplicate order items." with that casing and punctuation.
2. The summaries for these settings should be: "New order items can be added." and "Order items can be duplicated." respectively. If they are disabled, they would be: "New order items cannot be added." and "Order items cannot be duplicated." respectively.
3. Custom styling on the SKU sub-line in my case was just appending a br tag and wrapping the SKU line in a pre tag. I made this a relative font-size (e.g., .85 rem) and gray (e.g., #999), but we can ask majmunbog for a better recommendation if needed.
4. Instances of $purchase_entity in the code should be updated to $purchased_entity to match the function name / other users in Commerce Core.
5. Similarly, anywhere the code references "purchasable entity types," we should be using that language instead of "purchased entity types." This would be the function name getPurchasedEntityTypes() -> getPurchasableEntityTypes() and $purchased_entity_types -> $purchasable_entity_types. Same for other parts of the element and autocomplete.
6. I think we should tweak the logic in buildAddNewItemAction() when there is no form state input yet. Instead of defaulting to the first item in the array, if product variation is an option, we should always default to that, and only if it isn't should we default to the first option.

Note: I did not have a local instance of a module specifying an alternative purchasable entity type, so I'll trust you that the code there works. 😄

Recurring doesn't care where the payment comes from, so you'd just need a payment gateway module that can facilitate the transaction.

rszrama → created an issue.

rszrama → created an issue.

fwiw, in Commerce 1.x we had this pattern of not prefixing the module name with "Commerce" if it was in the "Commerce (contrib)" package. But closing this out as outdated anyways.

This needs a reroll to apply on 3.x now. Additionally, can we get screenshots to see the new behavior?

For example, I can't tell if the new widget still requires you to select an order item type first before selecting a product variation, which in the current scenario would happily allow you add a product variation to an order with the wrong order item type. In an ideal UX, you'd be selecting the product variation first, and the order item type would be selected from its type settings. (And where other types of purchasable entity are supported for order item referencing, the selection would be the purchasable entity type, not the order item type.)

That said, I'm not sure our current implementation would support alternative purchasable entity types anyways, so it may be a moot point. Our standard should be replacement level functionality, but I'd like to improve the UX here at a minimum.

Quick fix needed: the logic of the use_two_step_add_payment_form setting is backwards. If it's set to TRUE, then we should use the old style two step form, not if it's set to FALSE.

From an update standpoint, I think it's fine for the new version of the form to be the default. Howver, it is fair to question whether or not that breaks backend interfaces or test suites for existing sites. We'll certainly document the change in the release notes, but let's also add an update hook that doesn't actually change anything but displays the following message to users who update:

Your payment add form has been updated to a new single page format. If you depend on the old two step method, you will need to set a variable in settings.php to preserve it. See the release notes for Commerce Core x.y.z for more information.

Otherwise this looks and functions great! Still ways we can improve the styling of the form (e.g., fixing order total summaries, limiting the form width, etc.), but let's consider that out of scope for this ticket.

I think if we're going to roll with this, it should be generalized to a set of checkboxes of all adjustment types, not expect an administrator to type in adjustment names. Adjustment types are all knowable, so generating a complete list of options is not complicated.

Is there a rationale for why someone might ask not to tax shipping, though? That is often locale dependent ... but perhaps in flat rate scenarios the store is just backing the tax amount out of it?

Works for me. Will defer to Jonathan on the commit.

Node links being the "Read more" / "Add new comment" links in question.

Is this still relevant for 2.x?

rszrama → created an issue.

rszrama → created an issue.

rszrama → created an issue.

Updated the MR via GitLab's fancypants web IDE. I'm so hip.

I believe we also need to update this for the default configuration imported on installation, otherwise it will not apply to default orders. I'd also propose shortening the help text a little:

Cannot be directly set for draft orders; updates based on checkout input or user email address.

Just revising the title for specificity.

Committed, patch autogenerated by Cursor after just copying the relevant if statement from the order module and entering a new line in the subscriber. It detected from my clipboard what I intended to do and wrote the comment and then adjusted the setter. Magic!

rszrama → created an issue.

Got it! Thanks for the explanation. 😊

rszrama → created an issue.

rszrama → created an issue.

Thanks for the info! And makes sense re: auto attribution. So basically as a maintainer, once I close an issue, I just need to ensure that I go and create the record? Or will it be automatically creating the records, too, based on each active user's contribution settings?

(I guess it needs human review to ensure that people who just commented without contributing are removed?)

rszrama → created an issue.

rszrama → created an issue.

rszrama → created an issue.

rszrama → created an issue.

Sorry for the slow turnaround on my part, Tiffany. I think many of my questions / concerns were indeed addressed by 🌱 RFC: The architecture and philosophy of site templates Active , largely in the direction I was hoping it would go. For the remainder of the discussion,

A6. Thinking of a Site Template as a product, any similar product I buy includes direct advertisement of complementary products or services I might want to use with the product. WordPress qua WordPress advertises directly to end users; Shopify advertises directly to merchants; other SaaS tools will happily upsell me seats or higher plan levels. Similarly, Site Template creators should be free to include the presentation of complementary products and services to its customers.

I think a prohibition on advertisement on the front-end makes fine sense, and we can offer examples of tasteful incorporation of advertisements on the back-end. Ultimately, I'd expect the market to determine what's acceptable here ... if people aren't buying because the template is too pushy, that's a clear signal that it needs to be refactored. Perhaps we need a way for folks to signal why they chose not to buy a template after evaluating it prepopulated with reasons, which can include things like "Not the right fit," "Too spammy," "Chose another," etc.

B1. I think the proposal should ultimately be based on what we're trying to incentivize. I can only say a 10% commission on the first year's worth of upsells is a disincentive to launch a Site Template strategy focused on a DA controlled marketplace. I suppose the question is it in the DA's best interest to maximize revenue from every sale or to be maximally enticing to creators?

(Additionally, a 10% commission on services revenue disadvantages the template creator vs. other people who advertise services against those templates. I can very easily imagine someone advertising services that are complementary to specific site templates, and those people would not be subject to a commission rule.)

I think I'd just drop it or else specifically scope it to services that are qualified and opted into as part of the purchase process.

B2. Got it re: what's considered in scope for commission.

Re: the ecosystem fund, I don't think it will make a meaningful difference to module creators, but I do see it as a disincentive for template creators. (For example, for a template built almost entirely around Drupal Commerce, I'd be paying 10% to whom? Why would it not mostly go to Centarro? The best support I could get as a module maintainer would be more merchants / clients directly commissioning features to be added to those modules, not a derivative percentage of a small pool of revenue from a marketplace.)

Or to put it another way, I think the marketplace strategy and pricing should not be conflated with the broader topic of sustainable open source development. We risk splitting our focus too many ways the more we try to do through the system besides focus on adoption.

B3. "In no event would such a framework impact what a maker charges for their site template." Got it. I interpreted the proposal as about sale price of templates, not participation price in the program. Thanks for clearing that up. : )

rszrama → created an issue.

rszrama → created an issue.

rszrama → created an issue.

Well, I hoped for a fast fix, but unfortunately, I ran into an issue where apparently Affirm will combine our pseudo line items for "positive adjustments" (i.e., fees) if they have the same SKU, even if they have different prices. I installed Commerce Fee to test the patch, had a $25 fee and a $0 fee, and Affirm combined the two adjustments into one line item of $25 with a quantity of 2, charging double as a result.

I suppose we may need to append an increment value to the adjustment type to avoid this.

rszrama → created an issue.

Patch attached that upon review likely needs to be improved to support multiple "anonymous" adjustments of the same type. Right now in the promotion adjustments section of the same function we basically sum adjustments with the same array key together. I think we can default to that for unknown adjustment types as well.

(Note: this isn't a problem for unknown "positive" adjustments, as we don't have to provide an array key for line items.)

rszrama → created an issue.

rszrama → created an issue.

Revised.

rszrama → created an issue.

rszrama → created an issue.

Reviewed by jsacksick, committing. Will also open relevant Commerce Core issues.

I've implemented this as described and included a container div in the checkout form to make it easier for specific targeting, as the payment method add form doesn't actually wrap everything in its own container.

Settings form:

Front-end instructions:

The only oddity in the current approach is that we don't have weights on elements of this form, so it's hard to position things appropriately. I'll ask jsacksick re: revising the checkout pane in core to at least ship some default weights.

In fact, instead of using a simple textarea, let's make this a text_format to permit the use of WYSIWYG in the message preparation if desired.

rszrama → created an issue.

True, in the case of Bootstrap Basic Image Gallery, I bet we can just fix the functionality via one of our "assets" modules that we tag specifically to provide code we think a recipe will need. And the rationale makes sense re: apply once and not complicate updates.

Re: what "is" the site template in the Commerce Kickstart paradigm, then, perhaps we need to update our project page to clarify the various pieces and point specifically at https://www.drupal.org/project/commerce_kickstart_base → as the site template.

I'm still a little confused how to categorize https://www.drupal.org/project/commerce_kickstart_demo → against this spec, as it theoretically could be the site template someone builds from (just deleting the demo content manually) and does do more than base on its own, but it also contradicts, "Similarly, a site template SHOULD NOT be built on top of another site template." (In this case, maybe we just say we made an exception under SHOULD NOT permissibility and call them both site templates in the same family?)

Thanks for the update! A couple follow-ups...

In the marketplace proposal → shared out a couple weeks back, patching was indicated as not allowed, but this specification makes no mention of patching one way or the other. I suppose the difference is whether or not a template would be eligible for inclusion in a Marketplace, not whether or not a template can use patches.

1. Can we get a clarification, perhaps in "Site templates are built atop common parts." that "Site templates using their own project templates MAY include patches to included projects." If it needs to be more specific, happy to wordsmith the language. I also don't think it would be bad to indicate the patches must be hosted on d.o, e.g., "... MAY include patches to included projects that are attached to issues in those projects' issue queues." (This can avoid a future supply chain attack where someone swaps out the patch after installation to something nefarious.)

The meaning and intent of "Site templates MUST NOT, on their own, affect the installer." is still a bit ambiguous to me. I can imagine various reasons why a template may not use Drupal CMS as its base, for example. But even with what we've done in Kickstart, I do think we're compliant. We have:

• Commerce Kickstart project template
• Commerce Kickstart install profile based on Recipe Installer Kit
• Commerce Kickstart Base recipe (which I believe you would regard as the site template in this formulation)
• Commerce Kickstart Demo recipe (which optionally builds out the product types, catalog, and imports product content)

Related to my point of clarification re: patching, part of the need here is for us to be able to patch a Bootstrap Basic Image Gallery for things to work well in our use of Bootstrap / Layout Builder. Maybe that goes away once we can switch to XB and we don't need to patch and don't care to maintain our own project template, but this is the current state of affairs.

Finally, if we're going to require "Site" as the type, then in our case I suppose it would just be Commerce Kickstart Base? What about Commerce Kickstart Demo, which depends on the Base recipe. I suppose that could also be considered a "Site" recipe? The problem is that then violates the principle of "Similarly, a site template SHOULD NOT be built on top of another site template."

Can we get the various verbs in the RFC formatted according to https://datatracker.ietf.org/doc/html/rfc2119? Almost every point in here uses "should" in a way that means "we expect it to, but it doesn't necessarily have to" according to RFC 2119. This makes requirement levels of certain bullet points ambiguous. (For example, saying a template needs to do something but only should conform to the point is contradictory.)

Sorry, issue forks are still inscrutable to me. All I did was click a button to create the fork, and it already has several irrelevant commits in it and was based on the wrong branch. Not going to bother using that system to make a link fix.

Patch attached against 2.0.x.

rszrama → created an issue.

A lot of questions come to mind reading this, but I'll summarize things into bite sized bullet points for now. My "bottom line up front" is that without more clarity around hard technical requirements, I can't imagine participating until the MVP phase is complete, and without a perpetual "freedom of pricing" guarantee, I would hesitate to participate at all.

A. Items that need more clarity:

1. The biggie will be those "emerging standards" re: recipes, design systems, and XB that I know we don't have yet, so I'll hold my peace in that regard.
2. However, why must a template be an extension of Drupal CMS / use the CMS installer if it satisfies similar objectives? I understood site templates to be for the promotion of Drupal writ large, not Drupal CMS alone.
3. How should accessibility self-certification work when I might include any contributed projects that do not conform or provide configuration options that make it very easy for the end user to violate WCAG standards?
4. Self-certifying GDPR compliance is ambiguous. How do we determine if we are applicable, and to what extent must we support compliance? i.e., is this really the responsiblity of the template creator vs. the end user who believes they are subject?
5. Why can we not patch contributed modules? Sometimes it is necessary, e.g., to incorporate compatibility patches or usability improvements that are in progress in the issue queue but not yet incorporated into a release.
6. Is the prohibition on advertisement in the template restricted to the front-end or also include the back-end? e.g., can I not use the administrative interface to offer support or services to my customers?
7. How can we conduct a performance test in the abstract w/o a production server environment?
8. What scope is implied by “monitor and respond to user-reported issues in a timely manner” when the issue may be a usability, compatibility, or other issue in a contributed module incorporated into the template?

B. Last, some notes on the proposed commercial arrangements:

1. I'd be unlikely to agree to a perpetual revenue sharing agreement. Past a certain point, the continued business of a customer is on me, the introduction having long past. If someone launches on my template, grows on it for two years, and then decides to engage my team on an expensive systems integration project, the current language would require me to send 10% of that to the DA. Not only is that a higher commission than I’ve paid to similar lead generation / referral services, they all had an expiration timeline and were rarely even 10%.
2. It isn’t clear what happens once templates are sold directly through a marketplace maintained by the DA either. Is the 30% perpetual? The 40%? Just the 10% ecosystem support fund? For what it’s worth, I would not utilize a marketplace that cost me 40% even one time - or at least I'd bake that into my pricing and price even higher. Even with an expiration timeline for referral fees, I wouldn’t participate at 30%.
3. I don’t know what a “global pricing equity framework” is, but I doubt I would submit to it. If I have fixed costs for producing and supporting a template, I cannot not be in control of how much I sell my product for regardless of where my potential customer might live or what financial constraints they have. If the DA wanted to cover part of the cost of the product to ensure people had access to it, that’s obviously fine, but I shouldn’t be forced to sell my product at a loss to participate in a marketplace.

Given the proposal mentions the potential for a pricing equity framework, I would need guaranteed pricing autonomy before beginning the formal production process of a template now. I have to work according to some financial model, and while I can scale my ambitions up or down to manage my costs to make my template a better fit for the marketplace, I cannot be told I can sell a template for $1,000 today, incur all of the up front expenses, and then 1 year from now be told I must also make it available for $250 to people in X country or with Y constraints.

Attaching a patch for review from a client.

rszrama → created an issue.

The MR works for me; basically, instead of generating the order number early, this approach just updates the Affirm payment after the fact to use the order number for the Affirm Order ID. I'm going to create a follow-up issue to make this behavior optional as in other modules, so a site that diesn't use order numbers in the default way can retain the numeric order ID.

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

The issue for me is apparent even on a test product worth CA $199. (I'm basically just taking a clean Kickstart installation and converting the white armchair to Canadian.) The modal link never appears beneath the Add to Cart unless I have the enhanced analytics option turned on, because the code at present, as far as I understand it, just assumes the library is loaded without checking to see if it is.

On a quick test just now, note that I did have to clear all caches after toggling enhanced analytics off in order for the modal link to go away.

Worked for me! I tested the feature in a Kickstart 5.x install with Layout Builder first, then I disabled LB and saw it non-functioning, and then I applied the patch and it worked again.

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

Tested first with failure conditions, worked as (poorly) as expected. After the patch, works great!

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

rszrama → created an issue.

In our specific context, the issue was the inclusion of a customer email address in the signature parameters. That needed to be removed. However, it's now clear that each merchant may have their own configuration for which data points should be incorporated into a signature. As such, this module may be seen more as a starting point that should be customized per merchant.

rszrama → created an issue.

No clue how this got reopened; closing again.

rszrama → created an issue.

No clue how to delete the issue fork. I ended up just committing via a patch I finalized locally.

rszrama → changed the visibility of the branch 3517255-open-affirm-modal-review-step to hidden.

I refined this a little bit to only directly place the order on return when the modal is being used; redirected customers will process through normal checkout flow completion. Committed!

rszrama → created an issue.

Full patch attached with changes to RedirectForm to copy what we do in StripePaymentElement.

This still appears to be missing return processing that would actually place the order.

rszrama → created an issue.

Committed.

This looks great! Tested with and without the label and different SVGs. Smokin!

I had to reroll this to accommodate changes from other merges (attached). When testing, I noticed I'd end up on the payment redirect step (e.g., /checkout/8/payment) instead of the simulated successful return from a redirect to advance my order to the checkout completion page. The payment authorization went through fine and the Payment entity was created, I just didn't end up on checkout complete.