- last update
over 1 year ago Patch Failed to Apply - 🇳🇱Netherlands neograph734 Netherlands
There is already a lot of cleanups happening in
locale_configurable_language_delete()
. So it could be as simple as this right? - last update
over 1 year ago Patch Failed to Apply - last update
over 1 year ago Patch Failed to Apply - last update
over 1 year ago 29,815 pass - 🇳🇱Netherlands neograph734 Netherlands
I was so confinced I had configured git for windows right... :s
- last update
over 1 year ago 29,446 pass - Status changed to Needs work
over 1 year ago 11:55pm 21 July 2023 - 🇺🇸United States smustgrave
Can the IS be updated to match the new solution?
Also ran the test locally without the fix and got
Translation status cache emptied Failed asserting that an array is empty.
Which is good!
- Status changed to Needs review
over 1 year ago 7:30pm 22 July 2023 - 🇳🇱Netherlands neograph734 Netherlands
@smustgrave IS updated. The essence is more or less the same, but more focus on cleaning up during deletion (instead of fixing it when enabling) as suggested in #9.
- Status changed to RTBC
over 1 year ago 9:49pm 22 July 2023 - 🇺🇸United States smustgrave
Thanks! Think this is ready for committer review.
- last update
over 1 year ago 29,879 pass 37:41 34:50 Running- last update
over 1 year ago 29,884 pass - last update
over 1 year ago 29,886 pass, 1 fail The last submitted patch, 18: locale-no-imports-after-language-add-2994951-18.patch, failed testing. View results →
- last update
over 1 year ago 29,887 pass 37:57 35:01 Running- last update
over 1 year ago 29,946 pass - last update
over 1 year ago 29,946 pass - last update
over 1 year ago 29,953 pass - last update
over 1 year ago 29,953 pass - last update
over 1 year ago 29,958 pass - last update
over 1 year ago 29,958 pass - last update
over 1 year ago 29,958 pass - last update
over 1 year ago 29,962 pass - last update
over 1 year ago 30,047 pass - last update
over 1 year ago 30,051 pass - last update
over 1 year ago 30,056 pass - Status changed to Needs work
over 1 year ago 8:49am 23 August 2023 - 🇳🇿New Zealand quietone
I'm triaging RTBC issues → .
Thanks for working on this 5 year old issue and providing a test.
I have read the Issue Summary, comments and the patch. The proposed resolution matches the fix, which is good for reviewers. What I don't see is a fail test but @smustgrave did that locally and reported the results.
I read the patch in #8, which includes an update hook which is no longer in the patch. I am not sure if that should be done here or in a follow up but it is a significant change to the change. I have added discussing this to the remaining items.
https://www.drupal.org/project/drupal/issues/3089764 →
+++ b/core/modules/locale/tests/src/Functional/LocaleUpdateTest.php @@ -407,6 +407,10 @@ public function testEnableLanguage() { + $translation_status = \Drupal::keyValue('locale.translation_status')->getAll(); + $this->assertEmpty($translation_status, 'Translation status cache emptied');
Assertions should only have messages if in situations such as a for loop so that the individual failure can be identified.
This can also be one line, and eliminate the variable, $translation_statusI updated the IS with the standard template. Just a bit more to do here!
- 🇳🇱Netherlands neograph734 Netherlands
@quitone, Thanks for having a look. I've left out the patch because the issue should theoretically be self-healing.
The locale.translation_status key value store has a timestamp and last_checked date. So, when enough time has elapsed the cache should eventually expire and new translations should be downloaded again. Right?
It might be a good idea to validate this though.If somebody decides they still want an update hook, just calling locale_translation_clear_status() should be enough. Drupal core always clears all translation caches at once, so there is no need to select specific languages.
- Status changed to Needs review
over 1 year ago 7:17pm 23 August 2023 - last update
over 1 year ago Patch Failed to Apply - last update
over 1 year ago Patch Failed to Apply - 🇳🇱Netherlands neograph734 Netherlands
Made the requested changed from #24.
Tagging 'needs review' to validate the claim from #25 and determine if the DB update is needed or not.
- last update
over 1 year ago Patch Failed to Apply - Status changed to Needs work
over 1 year ago 7:47pm 23 August 2023 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 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.
- Status changed to Needs review
over 1 year ago 8:06pm 23 August 2023 - last update
over 1 year ago 30,056 pass - last update
over 1 year ago 30,056 pass - last update
over 1 year ago 30,048 pass, 1 fail The last submitted patch, 28: locale-no-imports-after-language-add-2994951-28-TEST-ONLY.patch, failed testing. View results →
- 🇳🇱Netherlands neograph734 Netherlands
That one was supposed to fail. Setting to NR to validate the claim from #25 and determine if the DB update is needed or not.
- Status changed to Needs work
over 1 year ago 2:30pm 29 August 2023 - 🇺🇸United States smustgrave
Believe the update hook is incorrect.
11.x is the development branch that D10 releases will be cut from so if this were to land in 10.2 update hook could start there.