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.
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. 😊
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?)
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. : )
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.
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.)
Revised.
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.
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.
- 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.
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:
- 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.
- 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.
- 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?
- 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?
- 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.
- 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?
- How can we conduct a performance test in the abstract w/o a production server environment?
- 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:
- 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%.
- 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%.
- 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.
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.
Tested first with failure conditions, worked as (poorly) as expected. After the patch, works great!
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.
No clue how this got reopened; closing again.
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!
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.
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.
All committed.
Merging the MR as is, but in a follow-up, I'm going to rename the helper function to getDashboardBaseUrl(). I'd prefer it to be a little more explicit in the absence of a requirement for more general usage.
Tested, everything works great. Nice refactoring. 👍🏻
If we go this direction, I think we may need a couple of different approaches to messaging.
For on-site usage (i.e., a cart link replacing an Add to Cart form in a catalog page), the default messaging is probably fine, though I think taling about "quantity" where there is no quantity input is awkward at best.
For off-site usage (i.e., a link template used in a Google Ad to "buy it now"), I don't think it makes as much sense to talk about a product generically not being available. I envision something more related to a link, promotion, etc. no longer being active, though even there, I'm not sure we can assume. Other ideas?
I'm not sure what the correct course of action should be here, to be honest. On the one hand, availability checkers should be respected, but on the other hand, how they are respected can be context dependent.
I believe this is why, for example, the cart manager itself does not check availability before creating an order item. Additionally, I'm pretty sure Commerce only checks availability after the add to cart has occurred via the order processor, but I could be wrong.
In this case, I'd almost suggest we leave the default behavior as is but optionally execute availability checking first, rendering the message upon failure. (The problem is, that message is pretty non-specific ... but it's the best we have atm.)
Generally speaking, the availability system needs work. Will link to this from our internal Commerce channel for discussion in a future breaking release.
We're switching to a new pattern for release naming; we'll settle this up ASAP.
Ah, good find re: the monthly estimate not appearing. I was actually wondering about that during my testing but forgot to note it. : )
Attaching a reroll of your patch with no changes, as the underlying code had changed a bit. That said, I'm still not sure it's a great UX. The modal can be closed with escape, but there's no X a user could click, nor does clicking outside the modal dismiss it. There's a bug in the behavior, too, in that you cannot then resubmit the form if you dismiss the modal, because the JS disables direct form submission.
This is the patched appearance:
I don't mind just improving what we have, but I'm also open to rethinking the UX based on what I've seen elsewhere. This from Miva is really interesting, for example:
You can easily dismiss theirs, and the side by side comparison / selection of the address is nice. Their button is weird, though, in that using what you entered shouldn't be construed as an update ... I'd likely just generalize that to a "Continue" button. That said, I'm not sure how much we'd have to rework to make the method based on a radio button selection, so my approach to start will be to just make the modal work with dismissal and checkout form resubmission.
Retaining this for now in case there's an improvement for us to make in 2.x.
Revising this ticket to point to the payment integrations section of https://docs.drupalcommerce.org/v2/user-guide/ instead.
After a few passes, all done in the 3517257-jslint branch.
Turns out the library itself being included multiple times per page was the issue. I went ahead and converted all three JS libraries to remove jQuery and add Drupal's once to prevent multiple application of the behaviors. All works great now!
I hoped this would be a simple revision of the JavaScript behavior making use of Drupal core's once library, but that doesn't seem to solve the issue. Now instead of always getting 5 iframes, sometimes I get none, and sometimes I get 5. Will keep digging.
Ultimately, it appears Affirm does not support mixed accounts. We finally got a separate account for Canadian transactions and have no ability to intermix the two.
Revised the message pattern and added log messages for all operations.
Committed.
Validated against and merged into the 2.x branch.