- Issue created by @Wim Leers
- Assigned to Wim Leers
- Status changed to Active
4 months ago 11:15am 6 March 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Repurposing this to add validation constraints; the original issue summary no longer makes sense because the scope/approach of ๐ Configuration schema & required values: add test coverage for `nullable: true` validation support Fixed evolved.
- Status changed to Needs work
4 months ago 11:50am 6 March 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Well this is interestingโฆ AFAICT
block.block.*:provider
is dead/completely unused, butblock.block.*:settings.provider
is used. - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Ah, I finally found the explanation for why
status
,info
andview_mode
are part of the generictype: block_settings
(and hence apply to ALL block plugins!).The answer: it's simply an oversight that occurred in #2274175: Block plugin schema should be defined separately from the entity โ . ๐๐ (
\Drupal\block_content\Plugin\Block\BlockContentBlock
landed >1 year earlier, but the tight coupling only was removed ~6 months earlier, in #1927608: Remove the tight coupling between Block Plugins and Block Entities โ . Arguably at least there this should've been rectified, but that was at the height of the "we must get D8 done yesterday!" era, so it totally makes sense.)So this issue is definitely cleaning up some very old technical debt, that's caused confusion for every person who has ever looked at the exported YAML for blocks! ๐
- ๐ซ๐ทFrance andypost
Great find! Would be great to have CR for that change somehow as I bet some contrib already using the same pattern
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
#6: this will absolutely need a change record โ probably multiple!
Wow, with #5 fixed, plus one small fix to the
ExtensionExists
constraint, we went from 788 failures to 115 failures! Suddenly this became doable ๐ - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Yay, and after refining the constraints for the sole block (the search block) whose settings I've added validation constraints to: 115 โ 56 failures.
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Reduced scope by:
- Undeprecating the (unused and useless)
provider
exported property inBlock
config entities around dropped us to 52 failures - Undeprecating the
status
andinfo
settings for "content block" blocks dropped us to 51 failures
- Undeprecating the (unused and useless)
- Issue was unassigned.
- Status changed to Needs review
4 months ago 6:58pm 6 March 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
GREEN!
Update path tomorrow.
Very satisfied that I got this to green with just
18 files, +138, -49
in less than a day ๐ฅณ๐ With this in, we could seriously accelerate config validation adoption, and Recipes indirectly! - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
2 of the 3 follow-ups created.
Update path test pushed.
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
Update path pushed.
3rd follow-up created (with MR!).
Two blockers created:
- ๐ Add config validation for weights (blocks, filters, etc. all use weights) Fixed
- ๐ ExistsConstraintValidator should ignore NULL values and treat `core` as a valid module Needs review
โฆ both with a ready-to-review MR that's green. ๐
Ready for final review! But please help land the two (trivial!) blockers first ๐
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
๐ Add config validation for weights (blocks, filters, etc. all use weights) Fixed is in! Will update this MR in the morning.
Next up: ๐ ExistsConstraintValidator should ignore NULL values and treat `core` as a valid module Needs review .
- Status changed to Needs work
4 months ago 12:07am 8 March 2024 The Needs Review Queue Bot โ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- Status changed to Needs review
4 months ago 8:36am 8 March 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
MR now on top of ๐ Add config validation for weights (blocks, filters, etc. all use weights) Fixed ๐
- Assigned to Wim Leers
- Status changed to Needs work
4 months ago 2:05pm 8 March 2024 - ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
๐ ExistsConstraintValidator should ignore NULL values and treat `core` as a valid module Needs review is in too.
Will update this MR.
- Issue was unassigned.
- ๐ง๐ชBelgium Wim Leers Ghent ๐ง๐ช๐ช๐บ
I started added explicit validation logic for all of the properties on
Block
config entities. I just pushed the test coverage fortheme
.But then I noticed that
block.block.*:region
doesn't have a validation constraint just yet! So this definitely needs more work before it is ready for final review. Will continue this next week. - First commit to issue fork.
- Status changed to Needs review
about 2 months ago 12:47pm 2 May 2024 - Status changed to Needs work
30 days ago 12:44pm 29 May 2024 - ๐บ๐ธUnited States phenaproxima Massachusetts
This is looking good but still needs some changes...
- Status changed to Needs review
10 days ago 10:50am 18 June 2024 - Status changed to Needs work
10 days ago 12:49pm 18 June 2024 - ๐บ๐ธUnited States phenaproxima Massachusetts
A few questions but overall I think this looks pretty good.