I also wonder if we could do some route lookups and list paths the permission would grant you access to
Some questions on the MR, will keep an eye out for when this is addressed, as getting this into 11.2 early is a priority
Committed to 11.x and backported to 10.5.x, 11.1.x and 10.4.x
Nice work all
Committed to 11.x - great work folks
Published the CR
kristen pol → credited larowlan → .
larowlan → created an issue.
larowlan → created an issue.
wim leers → credited larowlan → .
wim leers → credited larowlan → .
Committed to 11.x - thanks!
Committed to 11.x, tagging needs followup for the work to get other modules to the point where they can flip this parameter on.
Thanks for the fast turnaround @nicxvan @mstrelan
This is ready for another pass from Wim
larowlan → created an issue.
Committed to 11.x - thanks!
Committed to 11.x - fixed
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Trivial changes, back to RTBC
Committed to 11.x - thanks!
Committed to 11.x - thanks
I don't know that there's an automated rule we can us for this, but it would be good to add a follow up to investigate
Whack a mole, needs another reroll 😭
Committed to 11.x - thanks!
Committed to 11.x and backported to 10.4.x, 10.5.x and 11.1.x
10.3.x and 11.0.x are security only.
Issue credits from the private issue
larowlan → made their first commit to this issue’s fork.
Couple of minor issues in the MR, also needs a reroll
Thanks for working on this.
Left some comments on the MR
This also needs re-roll - thanks for working on this!
Committed to 11.x - thanks!
Committed this to 11.x - nice and early in the 11.2 cycle so we can shake out any issues (should they exist).
Huge effort folks, congratulations.
Published the change record.
Committed to 11.x - thanks!
Committed to 11.x - thanks!
One comment on the MR, fine to self RTBC
larowlan → changed the visibility of the branch 10.3.x to hidden.
Committed to 11.x - thanks!
Left some questions on the MR
----------------------------------------------------------------------------------------------------
Running PHPStan on changed files.
------ ------------------------------------------------------------------------------------
Line modules/filter/src/Plugin/Filter/FilterHtml.php
------ ------------------------------------------------------------------------------------
270 Method
class@anonymous/modules/filter/src/Plugin/Filter/FilterHtml.php:265::setTextMode()
has no return type specified.
------ ------------------------------------------------------------------------------------
[ERROR] Found 1 error
PHPStan: failed
This failed on commit check - but that's a filter plugin, not a views filter plugin so is out of scope, reverted that changed and it passed
Committed to 11.x - thanks!
More phpcs rules - nice!
Needs a reroll, fine to self RTBC
Committed to 11.x - thanks!
Needs a reroll, fine to self RTBC
Committed to 11.x - thanks!
There are three new fails in HEAD
FILE: ...core/drupal/core/tests/Drupal/Tests/Core/Theme/Icon/IconDefinitionTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
225 | WARNING | Unused variable $icon.
| | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
--------------------------------------------------------------------------------
FILE: ...rupal/core/tests/Drupal/Tests/Core/Theme/Icon/Plugin/PathExtractorTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
128 | WARNING | Unused variable $index.
| | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
--------------------------------------------------------------------------------
FILE: ...drupal/core/tests/Drupal/Tests/Core/Theme/Icon/Plugin/SvgExtractorTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
132 | WARNING | Unused variable $index.
| | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
--------------------------------------------------------------------------------
Time: 2 mins, 25.99 secs; Memory: 12MB
Fine to self RTBC here
Committed to 11.x - thanks!
Nice to see another rule enabled
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Asked other committers about this one
Found a module typo in the MR.
Now we're starting to convert things over (ie. moving things out of modules like 📌 Move helpers in field_ui_test.module and delete it Active I have concerns about the scope of this issue.
Should re repurpose it to mark the modules as converted that are actually converted and handle the ones that can't be in their own issue?
I think we got to the bottom of the memory leak and now we have stopped the bleeding we can do this in a much more controlled fashion right?
Committed to 11.x
No backport because this is a string change and will impact translations. Tagging as such.
Thanks folks. It was nice to meet you in Singapore @anmolgoyal74!
Committed to 11.x - thanks!
The MR is still marked as a draft
Committed to 11.x and backported to 10.4, 10.5 and 11.1
No backport to 11.0 and 10.3 as they're now security only.
You can see 17 database queries and 31 cache gets and one cache tag checksum fetch less https://git.drupalcode.org/project/drupal/-/merge_requests/10531/diffs?c...
🔥 love it
Committed to 11.x - awesome work
Committed to 11.x and backported to 10.4.x, 10.5.x and 11.1.x
10.3.x and 11.0.x are now security only, so no backport to there.
Just one minor question about a empty hook that's left behind. Fine to self RTBC after linking/creating a followup
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Committed to 11.x - thanks!
Can we get a follow-up created to use \Symfony\Component\ErrorHandler\BufferingLogger
and remove the use of dblog/database in this test?
Committed to 11.x - thanks!
Left some comments on the MR, we can simplify the date formatter test bit more now - there's some dead code
Committed to 11.x - thanks folks
Credits
Rerolled after 📌 Fix PHPStan L1 errors "Call to method getDefinitions()/getSortedDefinitions() on an unknown class Drupal\Core\Plugin\CategorizingPluginManagerTrait." Needs work , only removed the baseline changes, if it comes back green will commit
larowlan → changed the visibility of the branch 3474533-componentpluginmanager-must-implement to hidden.
Committed to 11.x thanks!
Going to resurrect bits of this as there are a lot of design considerations not yet addressed.
Will start with a rebase
larowlan → created an issue.
larowlan → created an issue.
Crosspost
Crediting Nathan who pointed me at cli viewport size as a possible cause of the failing e2e test when only in headless mode
Yes, client side - we post a model to the preview and form endpoints and there's some hard-coded stuff happening with e.g. images to flip them back from a file URL to a target ID. Look at \Drupal\experience_builder\Controller\ClientServerConversionTrait::findTargetForProps
for example - that can only go away if the client-side model stores a reference to
[
'alt' => 'This is a random image.',
'width' => 100,
'height' => 100,
'target_id' => (int) $this->mediaEntity->id(),
]
As well as
[
'src' => '/path/to/some/image.webp',
'alt' => 'This is a random image.',
'width' => 100,
'height' => 100,
],
In terms of the form state, that is only a global place to store the current form values before we commit them to the model. Previously it was done via a context provider but that didn't work for ajax form additions - the global state does
ha!
larowlan → created an issue.
Looking into fails
Credits
I've opened https://git.drupalcode.org/project/experience_builder/-/merge_requests/487 as another attempt
@wim leers ApiContentUpdateForDemoControllerTest
is broken in HEAD but it doesn't fail in CI.
📌 Change ApiContentUpdateForDemoController to save from auto-save instead of request data Active changed the endpoint to no longer require a body (it uses the autosave store instead) but the openapi.yml wasn't updated.
Without the change in https://git.drupalcode.org/project/experience_builder/-/merge_requests/479 this causes a fail when run locally
1) Drupal\Tests\experience_builder\Functional\ApiContentUpdateForDemoControllerTest::testSave
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
Array &0 [
- 'message' => 'Saved successfully.',
+ 'message' => 'Missing required header "Content-Type" for Request [patch /xb/api/content-update/{entityTypeId}/{entityId}]',
Updated the issue summary, which I should have done yesterday, but I quickly put this up before I finished for the day
larowlan → changed the visibility of the branch 3494388-see-why-openapi to hidden.
wim leers → credited larowlan → .
larowlan → made their first commit to this issue’s fork.
larowlan → created an issue.
I think this controller is temporary code so this unblocks more work
This looks like a nice cleanup to me
larowlan → made their first commit to this issue’s fork.
larowlan → created an issue.
MR is back to green
larowlan → created an issue.
Pushed some work to start building the regions based on the page template. It's failing openapi spec validation at the moment
Will continue tomorrow.
Another related issue. Going for a rebase here as 📌 Implement auto-save of the page template config entity Active will need the changes here
The issue with #17 was because we skipped settings, added a test and fix for that - and removed the @todo
This also results in duplicate blocks on page render, so we likely need more changes. Looking into them