Merged into 1.x and cherry-picked to 1.0.x. Thanks!
To do this, let's alter .gitlab-ci,yml such that there are two environment variables that make up the "canary" configuration:
CANARY_CORE_RECOMMENDED: The constraint used fordrupal/core-recommended.CANARY_PREFER_STABLE: The value of Composer'sprefer-stableflag:trueorfalse.
Then we can configure multiple overnight canary jobs that use different values for these two variables.
Tentatively closing, but let's reopen it if this gets reported again.
Let's postpone this on ✨ Add a Composer script handler which automatically uninstalls Experience Builder whenever it is updated Active .
phenaproxima → created an issue.
Dang - the fix I created here won't actually work, which is why the pipeline failed.
Yeah - alpha7 removed the /admin/modules/browse route; you always need to pass a source plugin ID as well, for example /admin/modules/browse/drupalorg_jsonapi. But we did an incomplete job when we made that change, and therefore it is still possible to visit these defunct routes, which will cause the exception you got.
This will be fixed outright by 🐛 WSOD when using settings to limit sources to recipes Active , so tentatively closing this issue as a duplicate of that one.
This issue, and MR, are bullshit. The proposed code fix would be a fatal error because you can't use $this in a static method.
Project Browser is D11-ready and everyone using Drupal CMS knows that. :)
A few minor points found in review. My main complaint is the heavy use of !important, which points to a faulty CSS cascade.
Moved 🐛 Recipes are sorted randomly on Recipes tab Active to the could-have list. It's a slight nuisance but not a blocker.
Seems like a reasonable approach but we will need stronger test coverage.
Closing as a duplicate of 🐛 WSOD when using settings to limit sources to recipes Active , which has a more complete and robust fix. It should not be possible to access /admin/modules/browse without a source plugin ID.
This problem is fixed by 🐛 WSOD when using settings to limit sources to recipes Active , so let's focus our efforts there.
phenaproxima → created an issue.
Thanks @ckrina. Since this has now been confirmed as an intentional design decision, closing this issue out. But feel free to reopen if there's more discussion to be had here.
Thanks for reviewing, @penyaskito. Merged into 1.x and cherry-picked to 1.0.x.
Yeah, that's caused by 🐛 Cannot download and install new modules using Project Browser (developer experience) Active , which I intend to fix shortly.
Sites will eventually have the stable XB module enabled and then the plugin could continue to affect those, go out of date with the composer plugin API
To clarify one part of this: the script specifically does nothing if you are updating XB from any version newer than 1.0.0-beta1 (the point at which a stable update path begins): https://git.drupalcode.org/project/drupal_cms/-/blob/1.x/project_templat...
Also, it is merely a script, not a Composer plugin. So it's only using Composer's actual API. That said, Composer does play a little faster and looser with its API than core does, so the point about it being non-upgradable stands.
It's possible that "skip this step" was only added to ensure it's understood that you don't have to choose anything here, even though they're functionally identical.
Assigning to @ckrina for input.
With this fix, I was able to apply a recipe in the generated Tugboat environment.
Assigning to @penyaskito for sign-off.
I lean towards preprocess because it means no additional dependencies, and is fine for this subtheme, which is a kludge anyway and not something we will maintain long-term. If the preprocess method is somehow insufficient, ECA would be my second choice.
Merged into 1.x and cherry-picked to 1.0.x. Thanks!
phenaproxima → made their first commit to this issue’s fork.
I think this is mergeable as-is, because it fixes the immediate bug reported by @chrisfromredfin, and doesn't help or harm the status quo.
Ideally, we would not need this script in the Drupal CMS project template -- fully agreed there. If we can move it somewhere less intrusive, I'm all for that. But that wouldn't need to be done in this issue; it would be a separate (likely minor release blocking) issue.
I think I actually prefer ECA in this situation, because it's a one-off in the Olivero subtheme. While I agree that Block Class is a good module and would be a safe inclusion in Drupal CMS, it's unclear how it would work in the context of Experience Builder, and we should be generally leaning towards being XB-ready.
Merged into 1.x and ported to 1.0.x. :) Thanks for finding this and fixing it, @penyaskito!
can you confirm whether this code is ever going to be upgradeable/removable when via automatic updates/normal composer commands without any manual intervention
When you say "this code", are you referring to the ExperienceBuilderDemo.php script?
Okay - in that case, I guess ECA needs to be updated to react to RecipeAppliedEvent. :)
I can't imagine that we want to trigger the cron run on Ajax requests, or during installation, so if we can adjust the ECA model to account for those situations, it seems like a worthwhile bug fix to me.
Merged into 1.x and cherry-picked to 1.0.x. Thanks!
Assigning to the privacy track lead to arrive at a decision and/or implementation.
Tagging as an issue of great interest to Drupal CMS, since Search API Autocomplete lies at the core (no pun intended) of our search offering.
Merged into 1.x and cherry-picked to 1.0.x.
Merged into 1.x and cherry-picked to 1.0.x. Thanks!
Okay, just needs the release note then. I'm merging this anyway, just to get it done; go ahead and mark this Fixed when the release note is there.
The "Update Extensions" page is actually not in core; it's part of the contributed Automatic Updates module. (Package Manager has no UI at all.) Moving to the appropriate issue queue.
Code looks fine, but would like front-ender sign off...
This will also need a release note explaining how to update the old configuration.
No code objections, but can we get some before/after screenshots?
Agreed; this should be a pretty easy fix. I think we should just remove the prompt from the message that gets output by Composer.
Drupal CMS has the same system requirements as Drupal core, because we don't impose any additional system requirements beyond what core has.
Therefore, this is not a Drupal CMS-specific issue, and there's nothing we can really do about it. But, moving to core's issue queue for further discussion.
Thanks @dunx. Converting to a meta.
The problem with this idea is that the "wizard" is actually a generic part of Project Browser and the core recipe system. It's not specific to the AI module.
By design, all recipes that require inputs, require all their inputs to have a value. But also by design, all recipe inputs need to have a default value associated with them.
So this would probably be a feature request in Project Browser -- allowing the recipe input step to be skipped, and accept the default values implicitly. Having said that, this would not be a good idea for all recipes -- things like AI or Google Analytics need an API key in order to work properly, so if they are applied and not configured, weird errors will happen.
Nonetheless, moving to Project Browser's issue queue for further discussion.
I think we probably need to update the test coverage for this to ensure it doesn't regress later.
This works as designed; recipes cannot make conditional decisions of any kind, which is an intentional core design decision to ensure recipes are predictable (which allows them to be composable). It does suck that it has to turn on a module it doesn't need, but that's not something we'll be able to fix in a recipe unless we create a custom config action which can uninstall unneeded AI module(s) -- and that would have to be done in the AI module itself.
Closing for now, but feel free to reopen if you feel this bears further discussion.
Not only that, but please link them all here as related issues, so that we can turn this one into a meta. Thanks to whoever is willing to take that on!
It seems pretty straightforward to at least add Unlighthouse to our DDEV setup. I tried it and was able to get a report generated. But I am not an expert; anyone else want to take this on?
If there's a DDEV add-on for Unlighthouse, that's probably the more preferable way to go.
Tagging as an issue of great interest to Drupal CMS.
I suspect this is an issue in Gin; moving to that queue until it's shown that this is being caused by something specifically in Drupal CMS (although I don't see how, to be honest).
That's a great idea. Setting back to NW for that.
Merged into 1.x and cherry-picked to 1.0.x as a minor quality of life improvement that we were aiming for in 1.0.0 anyway, but it missed the deadline by a few hours. We won't make too many exceptions like this, but I think in these circumstances it's probably fine.
I think I got it. Could use a review. Setting explicit versions in every component's composer.json is probably the way to go; it keeps things nice and consistent, so thanks for tracing that, @penyaskito!
For clarity's sake I also moved the canary-specific logic to our GitLab CI configuration, since that's the only place where the canary build happens.
Since this only affects internal infrastructure, and will help with the review process for all MRs, I think this is eligible to be backported to 1.0.x.
Merged into 1.x and ported to 1.0.x due to a merge conflict in recipes/drupal_cms_person/composer.json which shouldn't have happened at all, but better to discover it now than later. Thanks!
Code looks good to me and has my sign-off; the test failures were the usual random MySQL-related nonsense. Leaving the RTBC for someone else. :)
Let's not leak further Tugboat stuff into the composer.json generation script; I'd prefer taking a more GitLab CI-like approach, where we use branch aliasing in the Tugboat configuration script. Can we give that a shot?
If we're conditionally adding classes, I'm thinking this is probably something we need to do in our Olivero subtheme.
Merged into 1.x and cherry-picked to 1.0.x.
phenaproxima → made their first commit to this issue’s fork.
Merged into 1.x and cherry-picked to 1.0.x. Thanks!
This is clearly a bug and is therefore a candidate for backport to 1.0.x.
Maybe we could add the H1 to the regular page content (the body text) and assign the visually-hidden class to it?
With Drupal CMS having launched, I think it will be very beneficial to development to have local performance testing from here on out. So, merged into 1.x. Since this only affects internal development infrastructure and is never distributed to end users, cherry-picked to 1.0.x as well. Thanks!
For some reason, I'm very annoyed by the existence of this bug. So, now that we're out of commit freeze, and just to appease my own neuroses, merged into 1.x and cherry-picked to 1.0.x. Thanks!
We really need this in Drupal CMS as soon as possible.
What's the point of a required checkbox? It's not consent if you have to consent. All it's gonna do is add confusion and cognitive load.
Also, the Update module is not collecting any kind of identifying information whatsoever, as far as I know.
I would be against this change, personally, unless there's a legal requirement around it.
I really like this idea. If we created an include file for other modules' GitLab CI (similar to what the DA itself provides for contrib modules), it wouldn't necessarily even be that hard to do.
Won't move the issue for the moment, but this probably something that needs to be fixed in core, since Navigation is a core module and I'm not sure how much we're able to affect it in Drupal CMS.