Deleted bundles do not have their language configuration deleted

Proposal

Everything in the configuration will be deleted, when a bundle is deleted, so this should be deleted to.

We already have a language_node_type_update in language, to detect changes in the bundle name, so it could be easily added a delete hook to. Attached a inital patch for this.

I wonder why a node_type_update is used and not a entity_bundle_rename? So only when node bundles are changed the bundle keeps the translation settings. In the VocabularyFormController is a special submit function to take care of that, which could be removed. Further comment bundles are currently not renamed at all. Didn't changed it yet, as I'm not sure of the impact.

Also I noticed while testing, when creating a new bundle a entry in language.settings.yml is added, with no bundle name. In the element process function is the old value and not the new one. Added TODO in attached patch.

Added a rename and delete entity_bundle function for translation_entity too, as there its completly missing. Here the problem is that after renaming is done, somewhere the translation data are set again, with the old bundle name. Didn't figured out yet, where this is happening.

The meat of the test is commented out, so not sure it proves anything (yet). Comments on the code:

+++ b/core/modules/language/language.moduleundefined
@@ -315,6 +315,8 @@ function language_configuration_element_process($element, &$form_state, &$form)
+  // @todo On bundle create $element['#entity_information']['bundle'] is empty.
+  //   http://drupal.org/node/2015615

I don't think we need to mention this here, we have an issue for this one.

+++ b/core/modules/language/language.moduleundefined
@@ -420,6 +438,13 @@ function language_node_type_update($info) {
 /**
+ * {@inheritdoc}
+ */
+function language_entity_bundle_delete($entity_type, $bundle) {

+++ b/core/modules/translation_entity/translation_entity.moduleundefined
@@ -1038,3 +1038,33 @@ function translation_entity_save_settings($settings) {
+/**
+ * {@inheritdoc}
+ */
+function translation_entity_entity_bundle_delete($entity_type, $bundle) {
...
+/**
+ * {@inheritdoc}
+ */
+function translation_entity_entity_bundle_rename($entity_type, $bundle_old, $bundle_new) {

How can procedural functions have inheritdoc? Where would they inherit from?!

+++ b/core/modules/language/lib/Drupal/language/Tests/LanguageConfigurationElementTest.phpundefined
@@ -137,4 +137,30 @@ public function testNodeTypeUpdate() {
+    // Delete the bundle.
+    // ... how?

As this entity type does not actually exist and you can't delete it for real, you need to invoke the hook that would be invoked if the bundle would be deleted directly:

entity_invoke_bundle_hook('delete', 'some_entity_type', 'some_bundle') should do the trick.

Rerolled.

rerolled

Im at DrupalCon Amsterdam 2014 and working on it.

Rerolled.

Finished the test which was started but didn't worked yet.

Issue summary "Proposed resolutions" needs update.

I think the scope is too small, this require CUD covered or at least issue for update needed

+++ b/core/modules/language/language.module
@@ -277,6 +293,8 @@ function language_node_type_update(NodeTypeInterface $type) {

+function language_entity_bundle_delete($entity_type, $bundle) {

so update supported only for node types?

Please see also #2355909: language.settings config is not scalable.

Postponing on #2355909: language.settings config is not scalable then. That should provide full CRUD but tests that it happens on entity bundle actions will still be useful unless already added there.

Did #2355909: language.settings config is not scalable resolve this entirely?

Yep, we covered this, but we miss test coverage for deletion (we have for addition and update).

We should reuse the test (and update the IS).

Retitled for what needs to be done. Updated issue summary.

I'm working on this one in context of the Global Sprint Weekend 2015.

Removed tag by mistake.

Mmmmm... interesting.

The test actually reveals that this is still a bug on HEAD. But I expected this to be solved by the configuration dependencies. What's wrong there? The proposed fix may not be the best way, as IMHO this should be handled at the config dependency level.

There is nothing automatic about deleting a config entity that is a dependency of another config entity and having that config entity deleted. The only place where something like this occurs in extension uninstall.

+++ b/core/modules/language/language.module
@@ -232,6 +232,15 @@ function language_entity_bundle_rename($entity_type_id, $bundle_old, $bundle_new
+  // Remove the content language settings associated with the bundle.
+  ContentLanguageSettings::loadByEntityTypeBundle($entity_type_id, $bundle)
+    ->delete();

This should only do the delete if it needs to. I've never really liked the fact that the load creates new object. Feels icky.

Well, not creating the object would mean that we would need 5 lines of code instead of 2 everywhere where we use it. But yes, should do an !isNew() check.

Retitled according to the current status. Also updated issue summary.

Checked that the entity existed before deleting.

I think #2416409: Delete dependent config entities that don't implement onDependencyRemoval() when a config entity is deleted should take care of this automatically, we're finally at a place to automatically act based on the dependency information that we have.

@Berdir: that's awesome! I missed that one.
However IMHO getting this in still is interesting because of the additional tests.

Wrong order of the patch uploads, oops.

I agree additional test coverage would be useful even if the other one fixes the issue. Only found English issues.

1. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +   * Tests the language setting are deleted on bundle delete.
   

   settings

2. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +    // Create are language configuration for the articles.
   

   Remove "are" IMHO, I think you meant "our" but its fine without it.

3. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +    // Check the language default configuration for the articles is present.
   

   Remove the second "the".

4. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +    // Delete the articles bundle.
   

   "article" is the bundle name.

5. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +    // Check that the language configuration have been deleted.
   

   has been?

   (given its one file).

6. +++ b/core/modules/language/src/Tests/LanguageConfigurationElementTest.php
   @@ -164,6 +164,41 @@ public function testNodeTypeUpdate() {
   +    $this->assertFalse($configuration, 'The language configuration were deleted after bundle were deleted.');
   

   was instead of were for both no?

Fixed grammar.

All righto, thanks!

Wait, patch includes unrelated things.

Oops, wrong patch. Interdiff was right.

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 6cfaed4 and pushed to 8.0.x. Thanks!

Yay, thanks!