Given this will use the Intent API please also consider adding SupportsZeroBalanceOrderInterface simultaneously. Thanks.
Re #13: I would expect for unavailable purchasable entities to be available to the wishlist, at least to some extent. If the item is not available, it means (at least in my head) it is not purchasable. But one might add it to the wish list to track whether it becomes available in the future.
Thanks for keeping pushing for the release and sorry for the delay in tagging one.
I just tagged 8.x-1.3 and published the release https://www.drupal.org/project/telephone_formatter/releases/8.x-1.3 →
I gave some more thought to the proposal in #7. While it makes sense within the scope of the current issue, I believe we should keep the implementation as it is and instead adjust the scope.
In my view, we're aiming to replicate the default behavior of the Add to Cart form—which, like with any other entity type in Drupal, runs validation before creating an order item.
A good example of why this matters: imagine a Commerce site with multiple product types, each tied to its own order item type. Some of those order item types have additional required fields. Full validation ensures we don’t end up creating invalid order items when the Add to Cart form would normally prompt for more data before proceeding.
LGTM
The MR adopts the same messaging approach as the standard Add to Cart form. It delegates responsibility to the checker to override the generic message with a more specific one that clearly explains why the item couldn’t be added to the cart.
For some reason, I didn't know about this module until recently. And.. yeah.. It's a great module!
I found this issue with 🐛 Synonyms autocomplete field does not fail if unknown value entered Fixed . The requirement I am facing is to be able to create custom content entities (not terms) and support synonyms at the same time. Or, in short, respect the setting of the entity reference handler.
We had an internal discussion that triggered a small but mighty change to the scope of this issue. The idea is to toggle availability checkers on a per-order-item-type basis.
The MR is now updated accordingly.
I’ve updated the PR to use the attribute approach. Generally, it’s working, but there’s a major flaw in the architecture.
Because of how the container works, a single class can represent multiple services. With an immutable Attribute, we lose the ability to vary metadata per service instance.
Given that, I’m considering moving back to the interface approach but using a Metadata value object to keep things cleaner and more structured.
I came to the same conclusion about utilizing a custom Metadata attribute. Additionally, I believe the metadata could also include an ID (separate from the service name), which would then be used in the config sequence to mark the checker as enabled or disabled.
I also think that filtering of checkers should happen in the constructor (as originally proposed) to avoid the performance overhead of limiting the list repeatedly during each call to AvailabilityManagerInterface::check.
The main advantage of using tagged services over plugins is better performance, as tagged services are collected during the container build phase. Introducing extra filtering steps at runtime for every pass would be suboptimal. This approach would only make sense if we were using plugins, which we are not.
I published the MR with the proof of concept. Below is a screenshot from UI with the exposed checker from ✨ Add an availability checker which checks if the entity is accessible or not Needs work .
This is a valid addition. However, with 3.x already released, we missed the opportunity to introduce it in a new major version—especially since it might not be backward compatible. That said, this is actually a good thing. We should ideally avoid introducing backward-incompatible changes even between major versions, when possible.
I believe this new checker can still be added, as long as it only affects new sites and existing sites retain their current behavior.
To make it easier for our core audience—ambitious site builders—to enable or disable availability rules and checkers as needed, I’ve proposed ✨ Expose Availability Checkers in the Admin UI Active .
zaporylie → created an issue. See original summary → .
- We not only use the availability manager in the context of the order processor but also in the context of validation constraints. That's what prevents non-available items from being added to the cart.
- We can replicate the context in the cart links module and use it to validate the order item, in fact if we call
validateon the order item, that would trigger the same code/behavior as the add to cart form.
I added MR with a proposal.
zaporylie → created an issue.
zaporylie → created an issue.
Chiming in as this is a blocker for my contrib module that aims to inject an additional link to be shown next to the installed module. Currently, the list of potential links is fixed to only 3 options - help, permissions, and configuration. IMHO, the module interface should make it possible to inject any link. Dropping support for icons could and moving onward to dropdown, operation-style links could make that much easier.
I am not sure if skipping default content support for modules was intentional, so I created this issue to also serve as documentation in case the issue status is changed to "won't fix".
zaporylie → created an issue.
MR needs to be rebased.
I don't feel comfortable about this behavior in the context of a promotion form, otherwise all should be good.
As part of my MR I only added support to selected entities:
- order
- product
- product_variation
- promotion
- coupon
- store
Some notes:
- Not all content entities support sidebars (ie. coupon), which makes the form look a bit weird (very subjective opinion)
- For config entities we are better off without gin content form support as these forms are rather simple, featuring only handful of fields
- Some content entities are not to be edited in the context of admin UI (ie. payment_method) hence no need to support it either
I removed support for D10; otherwise, LGTM. Tested and works precisely like Centarro Claro. I also checked that the theme is not installed nor available after the site is installed which is expected. Thanks
I appreciate the feedback in #10 and #11. Routing recipe application d.org update requests to the queue, with the update module acting as the queue consumer, sounds like a clean approach that respects global tracking settings.
I’m curious about one more thing — the issue summary mentioned the name as one of the properties featured in the request. I’d like to clarify two points:
- The recipe directory name—which is essentially what the recipe name comes down to—has only a loose connection with the project on d.org. I believe we should only send an application update if the recipe includes a composer.json file, so we can verify that it belongs to the drupal/ namespace as outlined in one of the conditions in the issue summary. This would filter out all custom recipes added at the project level (i.e., not available on d.org even if the recipe name matches a general project on d.org), as well as recipes under other vendor namespaces.
- This also means that all core recipes would never be tracked. Are we okay with that, or should we support core recipes too? If so, we’d need to collect version information from the drupal/core package.
The site name is set by the installer based on the provided input. I just tested it with and without the demo content, and my site was named with the name I provided.
I wonder about a scenario in which a recipe is applied multiple times because it might be a dependency for many other recipes. That would be a common case for starter/base recipes. Said recipes can be deduplicated, which is an approach the recipe installer kit is promoting, or simply applied multiple times. The main issue I see here is inconsistency, which results in slightly off telemetry data.
Re #5: while I agree that the proposed approach is clean, I wonder how this could be respected if the recipe is applied via installer (no option to opt-in/out) or the site is installed from the recipe (drush si ../recipes/drupal_cms_starter).
Re #7: Recipe Tracker (thanks for mentioning it here) is meant to track the application of recipes locally, within your Drupal instance, and never sends any data outside the Drupal instance context.
Thanks for the catch, and if you are interested in contributing, feel free to propose a patch/MR. Once again, thanks!
I discovered that most of the duplications I am seeing are coming from strict comparison when batch tasks are containing instances of Recipe object representing the same Recipe.
All blockers were addressed. Therefore, closing as fixed.
zaporylie → created an issue.
All issues were addressed. I will be tagging the stable release soon.
I tested this MR and happy to report that the horizontal scroll is now gone and no incorrect overlaying could be spotted anywhere. LGTM
zaporylie → created an issue.
That works, the validation is happy, I am happy. Thanks :)
I set it up locally and the MR seems to do the job. Local tasks stick to the right and are visible most of the time. Only when off canvas sidebar is opened do they get hidden but at least they are not in the way which is a major improvement. Thanks!
zaporylie → created an issue.
zaporylie → created an issue.
https://www.drupal.org/project/commerce_recipe_core → is now in place.
Fixed via 🌱 Consider using Recipe Installer Kit Active
zaporylie → created an issue.
This was fixed as part of 📌 Convert commerce_kickstart profile installation to recipe Active . We may want to expose the currency selector in the installer down the road but for now we decided not go this way for the initial release.
This will likely be fixed as part of 🌱 Consider using Recipe Installer Kit Active . Postponing for now.
zaporylie → created an issue.
I removed Commerce Product Tax from the recipe in https://git.drupalcode.org/project/commerce_kickstart_demo/-/commit/bdea...
Given Commerce Kickstart 5 will be based on Recipes we decided it makes sense to proceed with Recipe Installer Kit
MR for changes in Commerce Kickstart Base are in https://git.drupalcode.org/project/commerce_kickstart_base/-/merge_reque...
The recipe itself can be found in https://github.com/zaporylie/commerce_recipe_admin_ui
While I put the recipe in commerce_recipe namespace I don't believe now this is a good choice. This would not be a general commerce recipe but part of kickstart ecosystem hence it should belong to the commerce_kickstart namespace.
zaporylie → created an issue.
New Config Action was introduced in ✨ Define grantExistingPermissions config action Active and utilized in this recipe.
zaporylie → created an issue.
This still happens even if the theme is enabled via Recipe. Applying drupal/drupal_cms_admin_ui recipe on the existing site with customized block placement makes these blocks also appear in the Gin admin theme.
This has almost no impact on the current sites and change is minimal therefore, in order to avoid applying patches, lets merge it in.
zaporylie → created an issue.
zaporylie → created an issue.
zaporylie → created an issue.
The recipe can temporarily be found in https://github.com/zaporylie/commerce_recipe_core/ but will be pushed to drupal.org once the respective project is created.
The recipe is temporarily available in https://github.com/zaporylie/commerce_kickstart_base but will be pushed to drupal.org once the respective project is created.
zaporylie → created an issue.
zaporylie → created an issue.