alexpott → created an issue. See original summary → .
Given we're sill in alpha for the 3.x I think it is okay to change these constants. Also I can't imagine why custom code would be using them. Also we're not changing the value in configuration.
alexpott → created an issue.
The redirect has been removed as a result of 🐛 Check if entities are locked on the delete confirm form Active so this is no longer an issue.
The redirect has been removed as a result of 🐛 Check if entities are locked on the delete confirm form Active so this is no longer an issue.
I've added a test to prove the bug and prove the MR fixes the error.
I'm still think about whether the fix is correct. I think it might be. Will discuss with other maintainers.
This is still an issue - I've improved the steps to reproduce in the issue summary.
@idebr I think this fix is ready. It would be great to get your feedback on this before I merge.
Marking this as RTBC - we've got test coverage - I've not made any functional changes to this patch and it has been running on a production site I'm involved with a for a year - since #22.
This might not actually be an issue because \Drupal\menu_link_content\Entity\MenuLinkContent::postSave() will result in clearing the cache tags of the affect menus.
See \Drupal\Core\Menu\MenuTreeStorage::save()
$cache_tags = Cache::buildTags('config:system.menu', $affected_menus, '.');
$this->cacheTagsInvalidator->invalidateTags($cache_tags);
Going to close this one because adding the cache tags for each entity feels like noise.
alexpott → created an issue.
Been using this on a production site for a while. Used AI (Claude) to add a test around menu link content translations so we've got some more explicit functionality testing in place.
alexpott → changed the visibility of the branch feature/3192576-langcode to hidden.
alexpott → created an issue.
Added 📌 Allow users to configure the locale chunk size in settings.php Active for @penyaskito's idead about making the chunk size configurable. I think @andypost's point about service injection is a much larger scope that does not really fit into a single issue and certainly should not hold this simple change up.
alexpott → created an issue.
Turns there are two problems here:
1. The route access check for the content lock JS lock route is not correct for translations
2. The libraries for the content_lock.js are not correct.
I've fixed 1 by adding a hash check... so if you have access to trigger the form alter that adds the js then you'll have access to check/lock the entity if possible, I think this is reasonable but more heads thinking about this would be great.
Fixing 2 is easy,.
@prem suthar the PHPCS fixes are unrelated and would belong in a separate issue. They have been fixed in a separate issue.
alexpott → changed the visibility of the branch 3478163-typeerror-argument-2 to hidden.
Let's fix this on 3.x.
Fixed the error reported in #5 - I think this is a bug in 3.x but it is good to catch it and fix it here because it is caused by deleting entities with form op locking enabled.
@idebr awesome thanks! Now we can fix it correctly!
I don't understand why this is using a configuration object. I think the issue summary has the correct code. In fact I've got a helm job that does
command: ["sh", "-c", "cd /app/web && drush ev \"if (\\Drupal::state()->get('install_time') === NULL) throw new \\Exception('Not installed');\" && ../vendor/bin/drush -v cron"]
I think it'd be a create idea if was done by core.
Just found another reason why this is a good idea. During install we limit theme hooks to only those from system module - see \Drupal\Core\Theme\Registry::build() so any cron hooks that doing any theming are going to struggle.
I think separating cron from the installer is the best way to go here - rather than trying to make the full theme system available once all the modules and themes are installed.
Very cool that 📌 Add blacklist and whitelist to the list of flagwords Active has landed - imo that means this is fixed.
This is going to save time during config import because the config import will delete all the blocks created by block theme initialize. This is why we've not noticed it till recipes because that's using the cofig sync flag to control what the module installer is doing.
Updating issue credit.
I've added test coverage of both a user without editing permission and the user with editing permission creating a translation and I'm unable to reproduce the error. There's no message about something going wrong.
What am I missing?
This fix is not quite right. I'm work on comprehensive test coverage atm.
Firstly the module only supports locking on the translation level if the conflict module is enabled otherwise it's on the entity level and you should not be allowed to add or edit translations of a locked entity. So the logic needs to account for that.
Secondly we'd stopped testing with the conflict module because it was not D11 compatible - it is now - so I've fixed that in 📌 Re-enable testing with the conflict module Active
alexpott → created an issue.
+1 I've not worked with Emma much but we need core leadership in this area and recognising the Emma's role in Drupal CMS by making her the UX Manager for core is great and part of the plan of how Drupal CMS would bring new voices and leaders into Drupal.
Ugh... the merge plugin is super complex. I think we should do the quick thing first and add a conflict to the composer.json. That will stop existing projects from breaking if they require this. Then we can open up an issue to try to deal with the root cause. I do not think it will be pleasant because the merge plugin is really icky. Why are you using it? Hopefully it is just a legacy of what Drupal used to do.
This is a duplicate of 🐛 TypeError: Drupal\content_lock\ContentLock\ContentLock::release(): Argument #1 ($entity) must be of type Drupal\Core\Entity\EntityInterface, null given Active which has been fixed.
Let's do a similarly hacky test for modules_installed... it's all pretty side-effecty anyway.
alexpott → created an issue.
alexpott → created an issue.
alexpott → created an issue.
This seems correct. It's all a bit messy because of the way that StackedSessionHandlerPass works. It might be nice at some point to abstract the session counting logic so a user could supply an implementation for a different backend.
We could add a follow-up to add some test coverage of this case.
Nice work @scott_earnest
Sure this makes sense.
Dupe of 📌 Nullable types must be explicit Active
Creditting people from 📌 Nullable types must be explicit Active
alexpott → made their first commit to this issue’s fork.
Committed and pushed 084ce88e263 to 11.x and c493c1fcc4d to 11.2.x and d467e8de288 to 11.1.x. Thanks!
Backported to 11.1.x as a bugfix.
Worked out what is going on... and a pretty decent workaround.
I think the problem is Drupal\Core\Cache\BackendChain. During installation we swap all caches out to be memory caches - see \Drupal\Core\Installer\NormalInstallerServiceProvider - so this is wrong. I've not worked out why incorrect stuff gets written to the database yet.
alexpott → created an issue.
FWIW it has been wrong from the moment we introduced the FetchAs enum - see 📌 Replace \PDO::FETCH_* constants to indicate fetch mode with an enumeration Active
Found 🐛 Fix Drupal\Core\Database\Fetch* types as they are wrong Active while reviewing this one.
alexpott → created an issue.
alexpott → made their first commit to this issue’s fork.
My votes are for:
- Joris Vercammen (borisson_)
- Marine Gandy (Mupsi)
Second time lucky. Backported to 11.1.x as this is a test-only fix.
Committed and pushed 2d31d3e8f14 to 11.x and 1c881a1efb5 to 11.2.x and 60d98a67e99 to 11.1.x. Thanks!
Crediting @larowlan as this came from a comment by @larowlan.
I think we should retitle this issue. It's not really about test classes anymore. Plus I left quite a bit of feedback on the MR.
alexpott → changed the visibility of the branch 11.x to active.
This has been solved in Drupal 11 - see ✨ Allow explicit COMMIT in Transaction objects Active - contributors to this issue were credited on that issue.
@jose reyero I created the merge request for you - https://git.drupalcode.org/project/drupal/-/merge_requests/12121
FWIW does this actually happen on 10.5.x? I think state cache collecting only was turned on by default in 11.x. - anyhows no harm in the tests being aligned.
Committed 7aa2683 and pushed to 10.5.x. Thanks!
Getting this into 11.x early in the 11.3.x cycle to wrangle out any issues and work on the follow-ups.
Committed 4815dcc and pushed to 11.x. Thanks!
Crediting people who contributed to 📌 Transactions should be allowed to be committed explicitly Needs work - ie. c960657 and anybody.
Please all the peeps from this issue.
The revision log message formatter should allow the same list of tags are we do in \Drupal\node\Controller\NodeController::revisionOverview()
$column = [
'data' => [
'#type' => 'inline_template',
'#template' => '{% trans %}{{ date }} by {{ username }}{% endtrans %}{% if message %}<p class="revision-log">{{ message }}</p>{% endif %}',
'#context' => [
'date' => $link,
'username' => $this->renderer->renderInIsolation($username),
'message' => ['#markup' => $revision->revision_log->value, '#allowed_tags' => Xss::getHtmlTagList()],
],
],
];
In this case we need to change the formatter for the revision log message to be able to use basic html. Then we can use it in views. If you look at the revision page after doing a revert the log message is perfectly rendered em tags and all.
I think we should fix the table to allow html. It means all the existing messages will work.
As a minor feature request committed to 11.2.x so it will be part of 11.2.0 even though we are in alpha. This is very low disruption.
Committed and pushed ed28d46cb09 to 11.x and 176346e770b to 11.2.x. Thanks!
Backported to 11.1.x as a test-only change.
alexpott → created an issue.
Backported to 11.1.x as a bug fix.
Committed and pushed 420f7b2ff2d to 11.x and 180597712ad to 11.2.x and a94e3c35cb7 to 11.1.x. Thanks!
Backported to 11.1.x as a bugfix.
Committed and pushed 42fb139e294 to 11.x and bd08e821d86 to 11.2.x and 335bc707498 to 11.1.x. Thanks!