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.
zaporylie → created an issue.
The scope here is slightly bigger than what's described in the issue title and summary, as suggested in #9. In short, commerce condition plugins are not correctly handled due to form decoration at the eca deriver level. I am bumping the severity to Major as reusing commerce Condition Plugins is one of the biggest potential value of using ECA with Commerce.
Backstory: I am working on recipes and want to react to the add-to-cart event (that part works fine) but only if a certain condition is met - order type is X (doesn't work). The issue is that bundles are stored as an array, but the eca_commerce_commerce plugin expects them to be string
zaporylie → created an issue.
Default content updated in https://git.drupalcode.org/project/commerce_kickstart_demo/-/commit/16b7... to feature uuids (this was done manually, not via default_content export script).
Proposal made in #2 above is now part of the assets module, commit ref https://git.drupalcode.org/project/commerce_kickstart_demo_assets/-/comm...
zaporylie → created an issue.
zaporylie → created an issue.
Putting this on hold as I need to verify if the lib won't change its location shortly. Will post an update when I know more.
zaporylie → created an issue.
Attaching POC diff I created at one point for 1.x branch
zaporylie → created an issue.