No idea sorry. No versions of ST to-date require 8.4 or newer.
ST uses the access control made available in https://git.drupalcode.org/project/scheduled_transitions/-/blob/2.8.x/sr...
If you find a bug with this logic, please provide further details.
Issue will be closed if no response in the near future.
Thanks @nicxvan for the first pass review, left a note there.
MR!12220 posted with my suggested solution: deprecating the juggling of a parameter.
Created a changelog @ https://www.drupal.org/node/3526421 →
A couple q's for @larowlan. Remaining open threads I dont see as actionable/my implementation notes.
dpi → created an issue.
Block plugins, along with manager, annotation, attribute, etc, are provided by core.
Thanks for the unsolicited interest.
This project doesn't normally receive such a deluge of activity in such a short time.
Like I said in another issue at #3493337-6: Not scaffolding files for drupal/auditfiles → , which you havn't acknowledged yet. I'll be able to assist when time allows.
Considering nothing is an urgent site breaking bug, the response remains the same.
The project doesn't need more maintainers at this time.
As a general piece of feedback for all issues, please consider the following:
- A issue may be rejected. Please expand on justification.
- Ensure tests are passing, and new tests are provided. You do not need to fix currently failing tests or linting, leave them.
- Converting a patch to an MR with minimal effort does not qualify for re-review.
Thanks, some actionables there for me.
Thanks @mkalkbrenner, I suppose so long as it stays up to data with the current stable version that makes me happy, and should reduce your workload.
If a version support was explicitly removed, then adding it to a release's release notes is sufficient.
Clarifying title: its block module (not block config or theme_hook:block).
I think though we're not "removing" a dependency, but rather block or block content are conditionally enhancing the other module depending on install state.
-
Updated issue summary a bit.
Just a small and late nitpick that I think this should have specific block "templates". Instead of potentially being mixed up with block "config", as in #2666578-32: Block Content entities have no Contextual links when rendered outside of Block config entity → .
Block Content entities have no Contextual links when rendered outside of Block config entity
around block_content being tied to block config I think this is a won't fix.
I don't think a dependency on Block Config is currently true, and may not ever have been.
Yes, Block Content are expected to be rendered inside the block theme hook/template. But not block config. For example Layout Builder renders reusable Block Content with contextual links on the view entity route just fine.
Perhaps
✨
Ability to display block_content entities independently, also outside of Blocks
Postponed: needs info
will help with putting all Block Content within a block theme hook/template universally, without needing manual wrappers as the #theme=block in \Drupal\layout_builder\EventSubscriber\BlockComponentRenderArray::onBuildRender.
Created a new MR @ https://git.drupalcode.org/project/drupal/-/merge_requests/12156
- Generally a lot of reduction and dependency on Block between the two modules, and especially in tests.
- A new small service for usage.
- A new core test module to test navigation blocks, without needing block.module. See also 📌 Reduce dependency on block.module in tests Active .
Implementation notes
BlockContentBrokenTest in block content is testing against \Drupal\Core\Block\Plugin\Block\Broken (in core), this core class relies on administer blocks permission defined by block. So I've created administer_blocks_permission_test test module to also define a administer blocks permission so we can test without block.module. I've created
🐛
Dependency from core block plugin to block module
Active
to address this problem where core is depending on a block.module permission, as of writing currently postponed on
✨
Add a Production/Development Toggle To Core
Needs work
BlockBlockContentHooks) which is conditionally enabled depending on whether block_content.module is enabled. This hooks class has form alter and button callbacks, which functionally was formally located in Block Contents' \Drupal\block_content\BlockContentForm. Associated tests have been moved to Block.module's BlockContentSaveFormRedirectTest.\Drupal\block_content\Entity\BlockContent::getInstances and \Drupal\block_content\Form\BlockContentDeleteForm moved to the three classes adjacent to \Drupal\block_content\BlockUsage\BlockContentBlockUsage
- I considered making this a feature of block.module, but decided that BlockContent should still be responsible for these messages, but split out of the entity class.
- A service was created, accessible at
Drupal\block_content\BlockUsage\BlockContentBlockUsageInterface - The service is conditionally available, depending on whether block.module is also installed (
BlockContentCompilerPass). See new test coverage atBlockContentBlockUsageTest - A mulipurpose iterable/countable class provides the necessary block config producing and efficient counting functionality that
BlockContentDeleteFormandBlockContent::preDeleterequire. BlockContentDeleteFormandBlockContent::preDeleteare still retain responsibility for calling out to this service.- Ive made
BlockContentUsageListfinal for now. It should be un-finalised only if an interface is extracted, in the future. \Drupal\block_content\Entity\BlockContent::getInstancesdeprecated, change notice at https://www.drupal.org/node/3523951 →
BlockContentBlockHooks) which is conditionally enabled depending on whether block.module is enabled. This hooks class provides the existing hooks to modify the block hook_theme/template and modify block config entity operations. Hopefully the block preprocess hook can be moved out when
📌
Remove block.module dependency from Layout Builder
Needs work
finishes decoupling block hook_theme/template from block.module to core/system.module. #[Hook('help')] modified with various decouplings and variable-izing permission names. One pieces of help text used to reference administer blocks, but was incorrect since permission changes in
✨
Add more granular block content permissions
Fixed
.BlockContentBlock::build to no longer rely on administer blocks, but on Url->access(). Looks like the permission was no longer correct anyway, since permission changes in
✨
Add more granular block content permissions
Fixed
.\Drupal\Tests\block_content\Functional\BlockContentTestBase.
- I re-evaluated all Kernel/Functional tests in Block Content, and updated them the explicit permissions the test user needs, instead of blanket permissions from
BlockContentTestBase. This includesadminister blocksspecifically. - There were many uses of
placeBlockto place title/actions/tasks on pages, which tests used to click links. However this method requires block.module, which we're trying to prevent unnecessary dependencies on. So I've introduced thenavigation_variant_testtest module which can render navigation blocks without block.module. I've created 📌 Reduce dependency on block.module in tests Active to expand usage of this method so other parts of core can reduce need of block.module viaplaceBlock.
Postponed 🐛 Dependency from core block plugin to block module Active on this issue.
Should this be under Base/Plugin systems instead of "Block.module" component?
@catch that issue looks like it could provide what we need.
Marking as postponed but anyone feel free to add other opinions.
Offering a fly by opinion as it was linked in #3524897-6: Dependency from core block plugin to block module → .
A "development" yes/no toggle sounds great.
Re the current issue title: "Add a Production/Development toggle". In some clients or projects there you might find terminology and a setup describing development, non-production, production.
A toggle as provided here I imagine would refer to mainly local-development related activities. So I think to avoid the binary labels being confused with non-prod vs production, this should simply be referred to as "Development mode".
I personally would suggest against a new permission, especially with such a limited purpose.
Its bloaty and too theoretical at this point.
Another approach could be just like we have with twig debug service parameter. Then pass along that parameter as a part of plugin creation. That way it isnt tied to a specific user, and the broken message can never be displayed in a non development environment, and inversely is always present on a development environment.
Contrib or future changes could easily tie multiple parameters together under a "development parameter", which turns on the above proposed parameter plus twig parameter plus others...
As found in 1.8.1 / 1.7.1 / 1.6.1
All good, normally I'd cherry pick across branches I deem supported.
I've made minor changes to satisfy linting to one branch. So I'll do a quick cherry pick in this case.
Head is passing w D11
How is this possible though? What is the definition and table?
Are you sure something isnt malformed or altered elsewhere?
Issue template needs to be fixed.
Working on this...
I think the coverage at \Drupal\Tests\Core\Controller\TitleResolverTest is sufficient.
Other than a tonne of implicit coverage.
PageVariantInterface is just passing valid values down from TitleResolver::getTitle
Can we look into and potentially close 📌 Add a test for re-parenting of menu links. Postponed: needs info if there is sufficient overlap.
Created a autoloading-related issue for ensuring classes are added to the autoloader before container cache is unserialized @ 🐛 Using instances of classes defined by modules in service container parameters causes fatal errors Active
I did find
📌
Extension System, Part IV: Properly register all installed extensions/namespaces during container generation
Needs work
, which is more about comprehensive/unified namespaces for everything. E.g themes are added to autoloader in a crazy place (\Drupal\Core\Extension\ThemeHandler::addTheme). Distinctly different to this issue.
Time has passed, new perspectives and such.
Following on a suggestion and discussion @ #3522410-11: Using instances of classes defined by modules in service container parameters causes fatal errors →
What if we did the inverse to what is currently proposed here, back to the original intent of the ticket before direction change in #2536610-6: Ensure all modules are autoloaded with PSR-4 only if enabled →
Always have all modules available in autoloader.
Then, the concept of "installed" means whether a modules hooks/services/discovery/plugins etc etc is loaded.
Converting what should be a Kernel test to a Functional test in order to get different PHP process is working out! 🎉 Still open to ideas if anyone else knows better.
Coverage
Critically, an error is shown when the new classLoaderAddMultiplePsr4 call in !12038 is removed:
Exception: Warning: unserialize(): Class 'Drupal\autoloader_test\AutoloaderTestEnum' not found
Drupal\Component\Serialization\PhpSerialize::decode()() (Line: 21)
When restored, the test pass once again.
NR
Thanks for the suggestion @godotislate, however I don't think thats works.
Problem is I need to add an enum or class reference to the container then get a new kernel either by a new PHP process or programmatically _rebooting_ it, so DrupalKernel uses the warm container cache. However since the test is all the same process, PHP already knows the class exists and caches it (AFAICT), so modifying the autoloader after the fact wont work.
Perhaps I will need a web test since I can rely on fresh PHP processes...
Pushed !12038
---
I've pushed up a PR which seems to fix the problem.
I'm at a loss for how to test this, as core/tests/bootstrap.php always adds all modules to the autoloader. I've tried a bunch of workarounds but I cant seem to get far. I'd appreciate if anyone can give that a go...
The flow of DrupalKernel has been moved around a bit, and the nearby type handling has been improved to satisfy PHPStan better, for even PHPStan levels core doesn't enforce yet. Especially with how core is passing around this semi-endorsed "container definition array" to various subsystems, for which I've specifically added a @phpstan-type for.
The way the PR currently works is there is a new cache entry in cache.container cache bin with a cache key with a different prefix to the entry with the entire container definition array.
A new ContainerDiscovery class is implemented to hold onto discovery related data. Objects beat arbitrary arrays. And we can extend it later if we need other discovery related things added to it.
The flow of where classLoaderAddMultiplePsr4</code. is called has been changed up. It isnt evident to me why <code>classLoaderAddMultiplePsr4 is currently in attachSynthetic, and it doesnt look like its relevant to that method as state and namespaces are not modified there. Perhaps it was more relevant many refactors ago. So it has been moved out of attachSynthetic, up into initializeContainer, so I can track and prevent doubling up of calls to classLoaderAddMultiplePsr4
Some parts of DrupalKernel have been changed to be more concise especially when a "discovery" edition of a code block is introduced alongside an existing "container definition" block.
fjgarlin → credited dpi → .
Merged this simple workaround.
I believe there is an issues somewhere..?
It would be nice to have code always-available, and simplify what "install" means. As in an "enabled" module (etc) adds it do discovery and namespaces, and which hooks are executed.
@astonvictor irt your activity on other issues, Ill take a look when I'm back from vacation. For now Im sighting things piecemeal.
This is necessary to be able to run linting (make lint) where auditfiles is the root project itself.
I'm not sure what issue youre reporting? What problem is this causing?
Notably I could tear apart the hook theme regeneration stuff in compiler pass and move it to a service, but Id still need to construct hook_theme default variables by looking thru definitions, then doing reflection stuff. Which is a PITA.
...or nullify all default theme variables
This issue might be suitable to also update \Drupal\Component\DependencyInjection\Container::getParameter to allow enum returns.
Method signatures
\Symfony\Component\DependencyInjection\ContainerInterface::getParameter
public function getParameter(string $name): array|bool|string|int|float|\UnitEnum|null;
\Drupal\Component\DependencyInjection\Container::getParameter
public function getParameter($name): array|bool|string|int|float|NULL {
--
And somewhat related, looking to address use of enums in parameters causing unserialisation on warm loads blowing up since the autoloader isnt fully populated, at @ 🐛 Using instances of classes defined by modules in service container parameters causes fatal errors Active
The result is similarly catastrophic for _simpler_ data types stored in the container, like enums.
Enums are a valid return type of getParameter() in Symfony, but Drupal has not been updated to support them.
New minor as its a soft break to internal functionality.
Notably, our private "common" library uses it via setPintoHookTheme, so will need 0.5.1, and updates to code:
- param type array -> string
- add unserialize before passing to array_keys
There shouldn't be any runtime performance penalty, as the result of \pinto_theme is subsequently cached elsewhere on first-use.
Any custom code using pinto.internal.hook_theme must also follow our lead, by unserialising. 🤮
Critical points:
DrupalKernel
if (empty($this->moduleList) && !$this->containerNeedsRebuild) {
$container_definition = $this->getCachedContainerDefinition();
}
and further down, where module PSR4 namespaces are added:
$this->attachSynthetic($container);
dpi → created an issue.
Since theres no direct answer to #7, closing this so there is no wasted effort in this issue, compared to the long-lived/discussed/followed issue at ✨ Allow components to define asset libraries Needs work .
Please open if differences can be justified.
Coverage pushed.
dpi → created an issue. See original summary → .
Please do not spam tags.
To prevent notifications getting routed to catch-all channels, like Chat, this should by default error out (exception) if a notification type is not covered by a configured interface -> channel/transport mapping
This should help with scenarios like:
Notification is routing to a Slack recipient.
The mercure transport is enabled (is a chat channel transport)
The mercure transport will accept the notification with slack recipient by default.
We add something that can limit unintended transports.
\Drupal\notifier_chat_channel\Transport\Slack\Recipient\SlackRecipientInterface: chat/slack
IF a message is routed which dosent match a class/iface/abstract etc combo, then throw exception.
This should reduce the number of custom channelrouter services needed, and also help to prevent unintended message routing if for example more transports within a channel are added progressively.
The first implementation has been added, for Slack.
Looks like we'll need to do this for each provider, and adapt concepts from each.
Now that we know the required effort, we can do that in separate issues per service.
Incl notification and channel router, per README
dpi → created an issue. See original summary → .
With big codebases, we're doing php-driven autoregistration without services.yml (as core decoupled from sf and is still bikeshedding about that).
Quite some dev dudes i know with big code bases do it like this.
Thanks @geek-merlin, this makes sense, and allows me to approve this issue. I also prefer everything in `src` as a service, relying on private/service removal if unused, per Symfony.
Perhaps you'd be interested in taking a look at a core ticket I created a year ago: ✨ Directory based automatic service creation Needs work to help de-bikeshedding core ;)
Something needs to be done about the HuxCompilerPass2 name, and I'll do a deeper review. This will happen when I can get to it next, but isnt a high priority. Tests, docs, updates also
In the case where multiple chat channels are configured, greedy channels like Mercure (supports() has minimum requirements), ensure filters filter out for example where a message needs to go to Slack and never mercure.
The new CanonicalProduct and Slots attr via php extends inheritance should fulfill the requirements here.
With these systems, you can sub in your own twig/js/css, but inherit from slots and a parent object.
Futher, Slots are the recommended way of working, and they dont have a much surface thats hookable.
If these are not a good solution, let me know!