Committers, please backport this to 11.2.x. It is a show-stopping bug for the new direct-write mode, and is an internal change in an experimental module that does not affect the data layer in any way.
phenaproxima → created an issue.
That seems extremely shippable, and definitely blocks a 2.1.0 release since that's the one with Drupal 11.2 compatibility.
This is a duplicate of 🐛 RecipeActivator needs to handle unpacked recipes that aren't Composer-managed Active , which is a bit further along and more robust.
Not postponed!
phenaproxima → created an issue.
No tests yet, but this is ready for review, I think.
phenaproxima → created an issue.
We already have a contact form as part of the drupal_cms_forms recipe, so the site template will simply reuse that as a dependency.
Merged into 4.x.
Drupal 11.2 is now in beta, so this is good to go, I think.
As for the missing features, those are are not in the defined (settled) scope, so they could maybe be added later, but won't be in the initial version of this template unless @pameeela or @ckrina asks for it. :)
- This is resusable content, create/edit in one place, placeable in XB, other?
- What entity type are we thinking? blocks might be clunky, there is micro-content which I have not tried
- Blocks come with other Drupal things that might be confusing for editors
- Nodes are really pages, again not a perfect fit
- an FAQ is just a Q and A single item? Is it some type of accordion?
After some discussion with @pameeela in Slack, I think nodes are actually a fine fit for these, in combination with www.drupal.org/project/entity_display_route, which is slated to be merged into core but hasn't quite made it over the finish line for 11.2.
One of the major advantages of having nodes for all this is, I think, that editors can find all of their content, including the "micro-content", at /admin/content. Rather than having to search for it in different places of the admin UI.
Beyond the need for a shim module to suppress the page display for these content types, I can't see much advantage offered by blocks (or something else) over nodes.
Merged into 2.x and cherry-picked to 1.2.x.
Merged and backported - thanks!
@pameeela, there's not a lot we can do from here - the error comes from core.
Crediting @chike for reporting 🐛 Applying the Events recipe failed with fatal error because field_date already existed Active which, although legitimately was closed because it works as designed, led us to notice this bug.
Looks right to me, go for it if tests pass. This should go into 2.x and be backported to both 1.2.x and 1.1.x.
I agree with both Jim and Pam here; fields determine the structure of the database and conflicts/differences can cause data loss, so it is completely reasonable for the recipe to require a pre-existing field_date to be a strict match.
Closing because this works as intended.
Implemented #6 in an MR, but now that I think about it, I'm not entirely sure we want to do it this way, precisely because it requires changes in the top-level composer.json. That means the drupal_cms_image recipe, which is a foundational recipe, is no longer compatible with anything except drupal/cms projects, or projects that have specially been configured to handle it.
So that brings yet another idea to mind: what if we made the recipe depend on drupal_cms_olivero, and moved the breakpoints into that?
I understand that this doesn't seem to solve the problem on its face, but hear me out. Drupal CMS Olivero, thus far, only exists as a shim until we have Experience Builder and site templates. We don't intend to support it forever...at least, not in its current form.
But what if we moved the breakpoints into that, and made drupal_cms_image depend on it...and then, rather than completely drop Drupal CMS Olivero when we have XB, we instead just made a 2.x branch of it that only contains the breakpoints? The tricky thing here is that it would retain the name "olivero", but if it's a minimal, hidden theme that only contains breakpoints, is that really a big problem?
This is effectively the same solution as moving the breakpoints into their own actual contrib project, except we'd just reuse an existing contrib project that Drupal CMS projects are already using, which saves us some overhead (at the cost of a confusing legacy naming scheme, but maybe there are ways to work around that too).
Thoughts?
That's fixed now, I had just forgotten to update the lock file.
How to manually test this:
composer create-project drupal/cms:dev-recipe-unpack -s dev --repository='{"type":"vcs","url":"https://git.drupalcode.org/issue/drupal_cms-3524255.git"}' DIRNAME
Everything should install as normal, and you should be able to install Drupal CMS, but you should also see many modules added to the top-level composer.json, and few (if any) recipes.
Patched Drupal CMS with this and the random failures appear to be squashed dead. (They were very frequent before.)
Ship it.
Merged into 2.x and cherry-picked to 1.2.x. Thanks!
Actually, I have an idea that might make us all happy: why don't we postpone this until Mediacurrent delivers the Drupal CMS design system which we're going to build our site templates on? As far as I know, the design system will be packaged as a theme, and we could either add the breakpoints to that, or adapt drupal_cms_image to use the breakpoints it defines (if any). Then we don't have to depend on Olivero, but we can instead depend on a theme that is squarely in our ecosystem, and we don't have to do Composer shenanigans.
Wouldn't it be more robust to just use a theme project?
It would leave a little bit less to chance, but it's more stuff we have to maintain separately, for a very limited/specific use case. And it means that drupal_cms_image is a recipe that requires separate glue code, which isn't ideal. Keeping it all in one place is preferable.
Could this be an actual contrib module that themes can depend on?
We could do that, but I'm not sure the use case would justify the added overhead and infrastructure. Since the theme would contain zero runtime code, and would be updatable (via the scaffold plugin), I'm not entirely certain what the benefit of spinning up a module would be.
Re-targeted against 2.x. I think this is also okay for a minor version, but I'm going to stop short of backporting to 1.1.x.
Escalating since this is a random failure introduced in 11.2, almost certainly by 📌 Replace Contextual Links BackboneJS usage with VanillaJS equivalent Needs work .
phenaproxima → created an issue.
📌 Update to Drupal core 11.2 beta Active is committed, so this is unblocked.
Merged into 2.x and cherry-picked to 1.2.x.
Drupal CMS 1.1.1 was released on May 7th, this can be closed.
This is addressed in 📌 Update to Drupal core 11.2 beta Active .
Does this mean actually re-use drupal_cms_blog or copy it?
This means reusing it as a dependency. I think it's a good practice to depend upon existing recipes as much as possible, and build on them.
Clarifying that we may or may not need integration with all (or any) of Mailchimp, Hubspot, or Zapier. I guess we'll find out as we go along.
Thanks for the review @thejimbirch. Merged into 2.x.
Confirmed that this bug fix works on 11.2 but not 11.1.
phenaproxima → created an issue.
phenaproxima → created an issue.
IMHO this should target 1.2.x as well.
Not in a release version, @pameeela. I think we should switch to the beta now, though, in order to suss out any early problems.
phenaproxima → created an issue.
I thought of a way to do this. It's slightly nuts.
We do need to provide some glue code, but why not package it up in the recipe itself, and use the Composer scaffolding plugin to move it to the right place where it will be discoverable?
In other words, we should have the following files:
recipes/drupal_cms_image/assets/drupal_cms_image.info.yml
recipes/drupal_cms_image/assets/drupal_cms_image.breakpoint.yml
Those two files comprise a tiny, minimalistic, yet complete theme, called drupal_cms_image.
Meanwhile, drupal_cms_image's composer.json tells the core scaffold plugin to move those files into themes/drupal_cms_image. The top-level composer.json is changed to allow drupal_cms_image to scaffold files. Then the recipe enables the (tiny, minimal!) theme, and all the image styles depend it, and...voilà. No more dependency on drupal_cms_olivero.
phenaproxima → created an issue.
All good. Merged into 8.x-2.x. Thanks!
FWIW, I (and Leslie) do NOT support surfacing this setting in the UI.
Direct-write is not a choice that can be surfaced in the UI. That would imply it is a configuration flag, which it is not. It has to be enabled by a developer -- someone with access to the code base, who can alter settings.php (or, better yet, settings.local.php physically). That's all by design.
This security-related bug fix should have test coverage, I think.
Closing as a duplicate of 📌 Automated Drupal 11 compatibility fixes for openapi_rest Active , which fixed this.
For the record -- the cspell fix probably just involves adding the word to the project's internal dictionary, rather than actually changing occurrences of the word.
Closing as a duplicate of 📌 Automated Drupal 11 compatibility fixes for openapi_rest Active , which added the GitLab CI configuration.
Merged into 8.x-2.x. Thanks!
All good here. Test coverage is quite broad; cspell and PHPCS jobs are failing but we've opened 🐛 Fix issues with pipeline Active to deal with those. The "max PHP version" test job is failing but that's due to an issue upstream in OpenAPI, not in this module. Ship it.
Can this be converted to a merge request?
If the other OpenAPI modules are inconsistent, I think it makes sense to move them to Web services (note the capitalization), not OpenAPI.
Problem 1
- drupal/openapi_rest[2.0.0-rc1, ..., 2.0.0-rc3] require drupal/schemata_json_schema * -> found drupal/schemata_json_schema[dev-1.x, 1.0.0-alpha1, ..., 1.x-dev (alias of dev-1.x)] but it does not match your minimum-stability.
- Root composer.json requires drupal/openapi_rest ^2.0@RC -> satisfiable by drupal/openapi_rest[2.0.0-rc1, 2.0.0-rc2, 2.0.0-rc3].
The solution to this is not to change the info file; the problem here is that you probably have minimum-stability: stable in your project's composer.json, but Schemata has no stable release. (You could likely get around this by running composer require drupal/schemata_json_schema:@alpha.)
I didn't write this module and don't know much about how it works, so I'm not really comfortable removing dependencies without knowing why they're there.
Merged into 8.x-2.x. Thanks!
One real question -- if Drupal 10 is still supported, surely we should also test against that?
phenaproxima → created an issue.
@catch, if core is willing to conflict with older versions of Project Browser (and, for that matter, Automatic Updates), then I'd gladly file that follow-up.
Let's backport this to 2.0.x. The "read composer.json" way should still work on Drupal 11.1 and older, and honestly it's probably more reliable (in certain edge cases) than the old way.
The problem is that it removes the recipe from Composer.json.
It is supposed to do this. That's the point of unpacking. You unpack so that the recipe is no longer part of the Composer-managed dependency tree. :)
I think what we'll need to do here is adjust the recipe finder so that it simply scans composer.json to find out where recipes would be installed, and look there.
Re-targeting for 2.1.x; I'm not sure this will need to be backported.
Probably safer to commit this to the next minor version of Drupal CMS, rather than 1.1.x. It will need a change record to explain how to make this adjustment to existing sites manually.
Changed it a bit more. Now, if auto-detect fails, the field that couldn't be auto-detected is marked as required, so that you as a user don't get the impression that everything is copacetic. Status or warning messages are set for each field, so you know exactly whether or not it worked.
Thanks for the feedback - I agree it could be clearer. I changed it so that, if the path is auto-detected, it's made into the placeholder (as a sample value), and a status message tells the user that the path was auto-detected.
There is absolutely a test: https://git.drupalcode.org/project/drupal/-/blob/3dfed6a33787da29769280d...
I would like to mention one thing about ComponentSource sounding roughly analogous to MediaSource -- the reason MediaType plugins are not a thing is because MediaType is a bundle config entity -- same idea as NodeType. I think that, if the media system didn't have bundle entities, MediaSource plugins would be named MediaType plugins.
XB, however, has nothing analogous to MediaType entities. So it should not necessarily be echoing Media's naming decisions here.
phenaproxima → created an issue. See original summary → .
Created an initial version of the plugin which does a very simple dump of installer operations before they happen, into either JSON or PHP format (PHP is the default since it's probably faster to parse).
This would probably provide Package Manager with enough information to operate on.
Been thinking about this more, and the sense I'm getting is that what we want here is validation on multiple fronts. In other words:
- It shouldn't be possible to save a Component entity if its source plugin defines any invalid slots. This is what I've implemented so far in the MR.
- Nor should it be possible for components laid out in a tree to be sitting in an invalid slot (regardless of where and how that tree is stored).
I added some light test coverage here, but I have a question about where this validation belongs. The MR feedback seems to conflict with #9, and I'm not sure whether to believe now-Wim or past-Wim. :)
I think so, yes.
For the record, I generally land closer to @alexpott's position here. The merge plugin is ridiculously complicated and adds enormous complexity, and is likely to significantly increase the number of bugs and edge cases. I'd prefer to conflict with it, at least initially, and then later we can figure out how and if to support it.
I guess this is tentatively RTBC? The failures in the max PHP variant don't seem to be related to Project Browser itself. From the logs:
ERROR: PHPUnit testing framework version 11 or greater is required when running on PHP 8.4 or greater. Run the command 'composer run-script drupal-phpunit-upgrade' in order to fix this.
Addressed #25 and #26, except for:
The form saves on Enter. Is that what is wanted? Some pages do and some don't and that is annoying to me.
I very much doubt that this has anything to do with Package Manager itself, it just uses ConfigFormBase like many other settings forms in core do. But maybe this merits a follow-up (in Claro)?
To clarify something: just because Project Browser opts into direct-write support does NOT mean it is automatically enabled!
Direct-write has a two-part opt-in process. From the change record → :
To operate directly on the live site, two conditions must be met.
- A new setting, package_manager_allow_direct_write, must be set to TRUE.
- Individual subclasses of \Drupal\package_manager\SandboxManagerBase must have the \Drupal\package_manager\Attribute\AllowDirectWrite attribute applied to them.
Project Browser will not, by supporting direct-write, be assuming that all environments are local. The site owner needs to actually allow direct-write to be used at all by altering settings.php.
The best practice here would be to enable it in settings.local.php only, which should not normally be deployed to a production environment. Therefore, done correctly, local environments will be able to use direct-write (which is what it was intended for), and production environments should not.
Of course, you as a site owner could enable it on a production environment too, which is something we explicitly warn against. If you do that anyway, against our warnings, you're on your own. Drupal takes this secure-by-default-but-you-can-break-it-if-you-try approach in many places.
Letting a public web server write to code directories is a huge security risk. It's a hacker's dream. On local environments it's not an issue.
That's why direct-write mode needs to be enabled per environment (in a setting), and it explicitly warns about being risky and not recommended on production.
Project Browser should opt into it specifically to support local development.
To clarify one thing about the proposed event...
Here's what Composer's documentation has to say about it:
pre-operations-exec: occurs before the install/upgrade/.. operations are executed when installing a lock file. Plugins that need to hook into this event will need to be installed globally to be usable, as otherwise they would not be loaded yet when a fresh install of a project happens
In my initial exploration, that thing about the plugin being globally installed is not strictly true. I think it would be true if you wanted to affect projects that haven't been created yet...but for Package Manager's purposes, we don't care about that. Package Manager only operates on and within existing projects with a lock file. As long as the plugin is locally installed in the project, this event should suffice for our purposes.
phenaproxima → created an issue.
I refactored the test for clarity, but didn't really change what it does. Restoring RTBC here, since we'd really like this in 11.2. Feel free to send it back to NR for a separate set of eyes if that is unpalatable :)