Require `langcode: …` only for simple config that contains translatable values

Wow, it took me ~1 hour to figure out why/how the hell mergedConfigExport was pre-set to some value — turns out I am to blame, due to #2949021: Deprecate schema fallback in ConfigEntityType::getPropertiesToExport → ~6 years ago 😅 Untangling that is out of scope here, so just updating that existing test fixture.

What about things like third party settings which may contain translatable strings even if the original config entity definition does not?

Finally found why I was under the impression that all config automatically got a langcode upon saving: \Drupal\locale\LocaleConfigManager::updateDefaultConfigLangcodes() does that!

And locale_config_batch_set_config_langcodes() means that all default configuration gets updated to have a langcode.

Interestingly, \Drupal\locale\LocaleConfigManager::getTranslatableData() already has logic similar to what LangcodeRequiredIfTranslatableValuesConstraintValidator is adding!

1. CR created: https://www.drupal.org/node/3427629 →
2. Per #5, we now need an update path for all config on a site. All config on a site will have to be checked against its config schema (if it has it) to ensure that langcode is present if and only if it it contains translatable data. For config without a config schema, we should accept either. The update path test should start from a fixture containing one config object that has translatable values but is missing langcode, and one config object that has langcode but has no translatable values. That test should then fail.

The MR is now 99% green. Unassigning to let somebody else push this to the finish line.

#4: very interesting question! 🤔 I think we can do something similar to what we have for \Drupal\locale\LocaleConfigManager::updateDefaultConfigLangcodes() but then performed by the system module whenever modules are installed or uninstalled, to automatically add/remove langcode as needed.

#7: Follow-up question then: Is that complexity really worth not needing a langcode key? ;)

@Berdir: I actually agree with you 😇 IMHO just adding langcode: … to all config is totally fine and reasonable.

But @alexpott disagreed over at #3426429-11: [PP-1] Mark layout_builder.settings fully validatable → 🙈

@Wim Leers can you resolve the phpcs issue, very curious to see the tests run.

@smustgrave See #6, I can't. But 99.9% of tests are green already, see the last full CI run 😄

RE #4 - third party settings are not for simple config and config entities always have a langcode.

I think this issue is valuable because adding a langcode to simple configuration when there's nothing translatable in it is confusing. What does it mean, what happens if I change it. I've already seen a site where people used language overrides to override things that are not translatable. The problem with doing this is the moment you save that config via the UI your overrides will disappear. (Now we're getting on to the subject of validating language config translation overrides - definitely one for the future!)

D'oh, right, I wish I'd thought of that, @alexpott! 😄 Clarified issue title.

Now we're getting on to the subject of validating language config translation overrides - definitely one for the future!

🤓🤓🤓 Is there an issue for this already? 😄

I don't have a ton of complaints here!

But we still need to get everything green, as well as that update path, per #6.

omkar.podey → made their first commit to this issue’s fork.

Alright - I've written an update path and a test for it.

So locally I added a langcode to a random settings config file that didn't have any translation.
Applied the MR and ran the update hook
Hook ran fine and exported config and see the langcode has been removed.

Question on the MR since we are going to 11.x should we not do the trigger_error first instead add the violation.

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.

I wrote the logic. @phenaproxima was happy with that.

I knew an update path (+ tests) was necessary, but I don't have the time to do that right now. @phenaproxima did 🥳

Then @phenaproxima wrote the update path + tests. I just reviewed it. It looks GREAT! I found only a few things:

1. One comment nit: https://git.drupalcode.org/project/drupal/-/merge_requests/7018#note_295315
2. I think rather than deleting the body of a post-update function we should use hook_removed_post_updates(): https://git.drupalcode.org/project/drupal/-/merge_requests/7018#note_295312
3. We still need the @todo @phenaproxima asked for at https://git.drupalcode.org/project/drupal/-/merge_requests/7018#note_281429

4. P.S.: I also responded to @smustgrave's and @phenaproxima's questions about historical config system oddities 👍🤓

Welp, removing the defunct (unshipped) post-update from dblog makes tests pass, so I think that's about it. All I gotta do is file a follow-up.

Added 📌 Simple config with a `langcode` element, but no translatable elements, should raise a validation error Active to convert the "superfluous langcode" deprecation error into a validation error, and linked it in the code.

Dear committer, see #20 for why it's okay for me to RTBC this — @phenaproxima reviewed and RTBC'd the logic I wrote, I reviewed and RTBC'd the update path+tests he wrote.

Left some comments on the MR

This is now blocking several config validation issues, so tagging accordingly.

I've addressed the feedback in the update path (which is the only part of this patch which I wrote); leaving the rest for Wim to do, so I can re-RTBC those parts.

I agree with all changes here so far.

I responded to all MR threads.

2 MR threads still left:

1. BC dance: https://git.drupalcode.org/project/drupal/-/merge_requests/7018#note_296154
2. one doubt about the batch update @phenaproxima added: https://git.drupalcode.org/project/drupal/-/merge_requests/7018#note_296160

IMHO after those 2 trivial things are fixed, @phenaproxima should re-RTBC. 😊

Okay, I think everything is addressed now. Restoring RTBC as per #26.

Adding issue credit.

Committed 83a53ba and pushed to 11.x. Thanks!
Committed 2c3cb5e and pushed to 10.3.x. Thanks!

I don't think we need release notes for this because the validation system will tell you what to do and the changes are not mandatory for contrib / custom yet.

Wonderful to see this in 🥳 Updated blocked issues.

::validate() should be return typehinted. Opened 📌 Fix return typehinting for LangcodeRequiredIfTranslatableValuesConstraintValidator::validate() Active .

@mondrake Whoops — sorry. That seems a trivial follow-up fortunately :) Reviewed it.

Automatically closed - issue fixed for 2 weeks with no activity.

This is breaking the ckeditor5 UI in 10.3. I think we should consider entirely rolling back and trying again. See 🐛 Replace LogicException with trigger_error in LangcodeRequiredIfTranslatableValues constraint RTBC for discussion.

🐛 Replace LogicException with trigger_error in LangcodeRequiredIfTranslatableValues constraint RTBC was fixed. Do we need to close this issue?

Hello all, I've read through this thread and I'm looking for some guidance with an update issue from 10.2.7 > 10.3.1 in which I'm receiving the following (possibly) related error... in which I'm seeking a direction as Google search results have not been too helpful. TR pointed me to this thread (Ref: https://www.drupal.org/project/typed_data/issues/3464120#comment-15701671 💬 Error: Class name must be a valid object or a string Postponed: needs info ). Thank you in advance for any assistance any of you may be able to contribute.

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /update.php/start?id=383&op=do_nojs&op=do
StatusText: parsererror
ResponseText: Error: Class name must be a valid object or a string in Drupal\Core\TypedData\TypedDataManager->createInstance() (line 99 of /chroot/home/oceanbe1/oceanbeachsandiego-rv1.com/html/core/lib/Drupal/Core/TypedData/TypedDataManager.php).

-----------------------

Drupal database update

Error message
Warning: Undefined array key 1 in Drupal\system\Controller\DbUpdateController->results() (line 432 of core/modules/system/src/Controller/DbUpdateController.php).

The update process was aborted prematurely while running update # in system_post_update_add_langcode_to_all_translatable_config.module. All errors have been logged. You may need to check the watchdog database table manually.

The following updates returned messages:
system module
Update add_langcode_to_all_translatable_config

Processed 550 items of 876.

-----------------------

Error: Class name must be a valid object or a string in Drupal\Core\TypedData\TypedDataManager->createInstance() (line 99 of /html/core/lib/Drupal/Core/TypedData/TypedDataManager.php)

#0 /html/core/lib/Drupal/Core/TypedData/TypedDataManager.php(108): Drupal\Core\TypedData\TypedDataManager->createInstance()
#1 /html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php(155): Drupal\Core\TypedData\TypedDataManager->create()
#2 /html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php(57): Drupal\Core\Config\Schema\ArrayElement->createElement()
#3 /html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php(104): Drupal\Core\Config\Schema\ArrayElement->parse()
#4 /html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php(138): Drupal\Core\Config\Schema\ArrayElement->getElements()
#5 /html/core/lib/Drupal/Core/Config/Schema/ArrayElement.php(24): Drupal\Core\Config\Schema\ArrayElement->getIterator()
#6 /html/core/modules/system/system.post_update.php(270): Drupal\Core\Config\Schema\ArrayElement->hasTranslatableElements()
#7 /html/core/includes/update.inc(236): system_post_update_add_langcode_to_all_translatable_config()
#8 /html/core/includes/batch.inc(296): update_invoke_post_update()
#9 /html/core/includes/batch.inc(138): _batch_process()
#10 /html/core/includes/batch.inc(94): _batch_do()
#11 /html/core/modules/system/src/Controller/DbUpdateController.php(195): _batch_page()
#12 [internal function]: Drupal\system\Controller\DbUpdateController->handle()
#13 /html/core/lib/Drupal/Core/Update/UpdateKernel.php(115): call_user_func_array()
#14 /html/core/lib/Drupal/Core/Update/UpdateKernel.php(76): Drupal\Core\Update\UpdateKernel->handleRaw()
#15 /html/update.php(27): Drupal\Core\Update\UpdateKernel->handle()
#16 {main}

After implementing this change, when installing a fresh install in a language other than English and using the “standard” installation profile, for example”, I get the following error:

the configuration objects have different language codes so they cannot be translated