Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
The 1.x branch should get a code style cleanup, schema check (if schema existing for this module) and get basic tests. In the main module as well as in submodules (if existing).
As tests will be the most time-consuming and hardest part, we should start with the most basic tests, which might for example be:
- Configuration page access respecting the permissions set
- Configuration page functionality (save)
- Selected layouts removed from the layouts returned from the API function
- Advanced: Selected layouts removed from the UI, for example in layout_builder
- ...?
To be able to implement them in a useful way, first check, which functionalities the module provides and get a demonstration.
Afterwards, we could have a look at similar modules / core, what and how they test.
Tests should be implemented from quick and easy to complicated, as it will take a lot of time to have a good test coverage.
Issue fork layout_disable-3263064
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
anybodyComment #4
grevil commentedI fixed the coding style issues, added tests, added schema definitions and fixed the flawed config usage.
Comment #5
grevil commentedComment #7
anybodyGREAT work @Grevil! You identified a lot of flaws etc. :)
Just one (but important) point left, the change has the potential to break all existing sites, which should never happen. After that's fixed, this is all fine, I think :)
Comment #8
grevil commented@Anybody, I reran the Tests with A version of MySQL >= 5.7.8, the tests should work properly now.
Comment #9
anybody@Grevil: Can be changed here, I did that: https://www.drupal.org/node/2982948/qa
Comment #10
anybodystill scares me a bit, so I'll leave this open for RTBC and commit until I have the time to immediately test it in production.
Comment #11
anybodyComment #12
grevil commented@Anybody, why though? Before you were getting the "layout_disable.settings" and then in these settings you were looking for the "layout_disable.settings.disabled_layouts" nested keys. So because there was no manually created schema, Drupal automatically created a schema like this (at least logic wise):
Since the get() function for the config, will think you are looking for nested keys, because you are using the "." operator in the get function. See https://www.drupal.org/docs/drupal-apis/configuration-api/simple-configu....
How it is currently implemented is correct.
We have a schema like this:
In code, we are getting the module's config like this:
Afterwards, we are looking for the disabled_layouts key in the config like this:
$config->get('disabled_layouts');(For some reason we are getting editable configs, which is actually unnecessary in this case)
Comment #13
anybodyAs just discussed, this needs an update hook for the wrong old settings structure.
Comment #14
anybodyComment #15
grevil commentedOk, I just tested the update hook, it works as expected!
Comment #16
anybodyManual review looks good now, but two failing tests.
Comment #17
anybodyFailing tests were due to an upstream PHP 8.1 compatibility issue in core (->drupalGet()). RTBC! :)
Comment #18
anybodyVery well done @Grevil, thanks!