Perhaps the issue was implicitly resolved by #3099968: Layout builder incorrectly resolves global contexts values when viewing layouts → or another intermediate issue.
I can see node context is resolving on /layout pages, I dont recall if there was a particular way/order-of-operations that made this not work.
I'll close this one out.
Anyone can feel free to add better steps to reproduce in case there are more awkward ways of replicating the bug.
Should be something like
parameters:
symfony_messenger_doctrine.transports:
# Automatically create a Doctrine transport named 'doctrine'.
# Override this to disable or add more.
doctrine:
dsn: 'doctrine://default?auto_setup=true'
options: []
failed:
dsn: 'doctrine://default?auto_setup=true&queue_name=failed'
options: []
Or more verbose:
parameters:
symfony_messenger_doctrine.transports:
# Automatically create a Doctrine transport named 'doctrine'.
# Override this to disable or add more.
doctrine:
dsn: 'doctrine://default?auto_setup=true'
options: []
failure_transport: doctrine_failed
doctrine_failed:
dsn: 'doctrine://default?auto_setup=true&queue_name=failed'
options: []
@sandboxpl
I am also looking at usign different transport provided by sm_transport_doctrine module but this does not seem to work, and I can't find a way to configure it, is it correct that doctrine module won't support this functionality atm?
Transports shouldnt have to do anything to support failed functionality.
You just need to make sure the failed transport itself is pointing to the one you want.
SM has
sm.transports
failed:
dsn: 'drupal-sql://default?queue_name=failed'
So you just need to make sure your transports are re-defined it in your custom services.yml file, where the DSN is a doctrine:// protocol, not the new drupal-sql:// protocol.
Please keep us updated with your progress, it helps a lot.
To me, it seems the log simply didn't receive a lot of scrutiny (none) when added in #3191392: Make subdirectory path optional when using @namespace/template syntax → .
Perhaps instead, we should just remove the log.
All this configuration seems a bit much.
Merged, thanks @lussoluca
Dependent code is merged to 1.x, unblocking this work.
The docs also mention `sm messenger:failed:retry` command, which has not been implemented. In a MR or not.
Set to review to gather feedback on the MR in the meantime.
Merged, thanks @lussoluca!
Since this mentions the retry/failed commands, it should be merged after https://git.drupalcode.org/project/sm/-/merge_requests/35
I realise the MR didnt get any README attention, so adding docs in 📌 Documentation for Retries & Failures Active
Merged! Thanks @lussoluca
I see https://github.com/symfony/symfony/pull/54141 made it into messenger 7.3
Per https://symfony.com/blog/new-in-symfony-7-3-messenger-improvements#dedup...
We can un-postpone this feature if you have bandwidth.
--
For release after 📌 Drupal SQL/rate limiter/retry-failed/command iteration Active iteration.
We'll need to increase minimum messenger constraints.
Added test coverage to MR.
Perhaps we should promote this as an alternative to https://www.drupal.org/project/dead_letter_queue → ?
With outreach to the maintainers, or documentation of some kind.
The error is present on https://git.drupalcode.org/project/pinto_theme/-/blob/1.x/src/EventListe...
Marking this as a duplicate of 💬 The following module is installed, but it is incompatible with PHP 8.3.19 Active .
If PHP is interpreting the PHP minor minimum as a float in some way, then that sounds like a Drupal core issue of some kind.
Could you look into the precision settings and report there?
Marking a similar issue in Scheduled Transitions as duplicate.
User reports issues where the patch PHP version they have should match the minimum patch version.
--
My initial thought on this from the ST issue is it could be a YAML implementation issue; a difference between PHP YAML or YAML package.
Could also be some kind of strict typing thing where the `php` value of info.yml should be stringed after read.
But precision sounds like more of a clue.
I've decided !23 is too unwieldy. Too difficult to review and merge unrelated changes. The features need to be made more atomic, for review and code forsensic purposes. I've tried to do this work, breaking the code into stacked MRs.
I've taken the code from !23 removed the unrelated CS/Lint changes and unused/untested middlewares, and created PRs comprising what is currently found in !23 before its closure.
A MR each for:
- Drupal SQL transport
- Retry/failed (built on above Drupal SQL MR)
- Rate limiter (built on above Retry/Failed MR)
And extra MRs for
- Retry failed command (built on above Retry/Failed MR)
- The remnants of SQL middlewares. (built on above Drupal SQL MR)
The MRs are Stacked MRs, relying on the parent MR/branch to be merged.
---
So far I've added review feedback to the Drupal SQL transport, which needs to be addressed.
I haven't completed a full review of retry/failed yet.
Rate limiter is something that sneaked into !23, without discussion. I've created a new issue and MR containing this code. I don't mind including the rate limiter integration, but it should be done in a proper proposal.
The work from !23 has been split out into separate MRs for the relevant tickets, in order to keep things atomic: testable/documenable/reviewable etc.
dpi → created an issue.
Feel free to pick up.
Merge is postponed on ✨ Implement retry Active .
But the MR here still need a little bit of work, including a basic integration test.
Command tied to the work here created at ✨ Add messenger:failed:show command Active . WIP MR provided.
The new work is found at https://git.drupalcode.org/project/sm/-/merge_requests/33
The new work is found at https://git.drupalcode.org/project/sm/-/merge_requests/29 There is addressable feedback there. It's almost ready.
This MR precedes the work for ✨ Implement retry Active .
I don't see why they should be combined, so I've taken time to break them apart.
Blocked on ✨ Implement retry Active
MR found at https://git.drupalcode.org/project/sm/-/merge_requests/35
I created a stacked MR on top of Retry/Failed ( ✨ Implement retry Active ) MR, which is stacked on DrupalSQL ( 📌 Provide a Drupal SQL transport Active ) MR.
dpi → created an issue.
We need to postpone this on getting the SQL side in first.
The code right now is a spaghetti mess.
I'm not sure about the premise of this issue. To date, I've recommended using the limit=Xseconds command combined with traditional cron if the platform is intolerant of long running commands. Why is that not a viable option for Platform.sh?
Secondly, if a platform doesnt support async, then perhaps they should just consider using synchronous sending?
Lateruntime/kernel-termination processing of async messages, or "poor-mans-async" is a bit questionable. I'm wondering if that should just be a different contrib, since its not something we should ever be endorsing.
I do think I need to rethink some of this now that PHP is extending support lifetime, and having a period of time where two major Drupal versions are supported for longer makes the support matrix larger than I want.
Traditionally, I've moved things that are no longer supported to the Historical support → page, and there has been no crossover.
Given the "supported" concept in drupal.org doesnt actually translate to anything in Composer, I think what makes sense is I only include what branches are receiving new features. While other versions are relegated to historical support. A version under historical support does not necessarily mean it will never receive any more bug or security updates, that will be at my discretion.
The update page makes it look like Drupal is complaining or showing errors about the version you have vs what is available.
This is certainly a UX issue.
The version combination you have, of ST and core are not an issue.
What I read the Drupal report as is similar to `composer outdated --direct`, in which it it spells out you are not on the latest. But it should be smarter about communicating that.
Perhaps the problem lies with core's interpretation that a major version of a contrib is whether new core compat comes in. Whereas ST, and many other projects outside of the Drupal ecosystem, will include new major dependency changes in a minor version.
In any case this is not a problem with ST itself, and I have no intention of changing my release model to a new major for each Drupal core major.
Closing.
Please feel free to reopen if this is a problem with ST.
Or if you find its a problem elsewhere, please help others by providing links to other issues.
There is no problem or error here.
Update to v1.2 when your set of dependencies allow it, i.e. when you upgrade to D11. Until then, run any version less.
As of writing, if you are on D11, you already are required to use PHP 8.3. These two constraints are reflected exactly in ST 2.8.x series.
If you are on D10, you can use one of 2.5.x-2.7x.
You shouldnt ever need to think about any of this, since Composer manages it for you automatically, so long as you require any of v2.
This project, and most of my projects, have always had a proactive approach to PHP, where multiple minors of the project are maintained with their own Drupal core + PHP compatibility minimums.
Scheduled Transitions itself required PHP 7.1 at inception, while core required 5.5.
I have no intention to follow core's PHP/dependency graph. I do aim, and communicate as such clearly, the end-of-life schedule of each version on the project page, and associated version archive page.
Whether or not I actually make use of a PHP versions new features when a minor is introduced isnt relevant, in the least it allows for the possibility to make use of version specific features during its lifetime.
You are more than welcome to continue to use an older supported version of Scheduled Transitions, and update at your own leisure.
the actual checking of the array shapes' contents will start from PHPStan level 5 above; we are currently struggling with level 1...
- I don't think we can start doing this without us being on level 5 as well,
Level 2, not 5, by the looks: https://phpstan.org/r/63bdb4f4-ca81-4423-8828-9da62ef4a5c9
According to that try page, perhaps it is enforcable with just enabling reportPossiblyNonexistentGeneralArrayOffset, and we could baseline on that.
Stan and typing doesnt start and end with core. This change benefits, and is driven by, contrib and custom code.
As a project using PHPStan Lvl 9 + Strict I can immediately benefit from this.
Whether we enforce this in core now or later shouldn't matter too much, if non-core is consuming it, then it can also be responsible for reporting back to core. Just like we have to right now with the multitude of unenforced defective PHPDocs. When it comes time to core enforcing it, the amount of code is drastically reduced to simple bulk fixups, just like we would with existing PHPStan bulk changes.
How do we make sure we keep the type compatible with the actual array structure, can phpstan help us with this or is that something we'd need to define in a test?
Should we not bother?
When our internal usage changes, either regular runtime or tests, PHPStan will display errors.
For example if we add a new array key to the structure, and a test makes use of that new key, PHPStan will complain the key does not exist.
The same applies to removals, type changes, restructures, etc.
Thanks..
Probably not out of the box.
If I was looking at to this myself, I would start by using some views query alters, with some math convert to unix timestamp, round to 900 and divide by 900 then round and divide your time point to 900.
Something like that, anyway. Outside of this project itself. Consult your local DBA or views expert ;)
Can we adapt existing tests for these types of fields?
The tests don't need to be comprehensive, just cover this variant.
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