locale_storage() has been replaced with \Drupal::service('locale.storage');

https://drupal.org/node/2016569, change notice for locale_storage().

Files: 
CommentFileSizeAuthor
#8 2020343-diff-1-8.txt1.32 KBvijaycs85
#8 2020343-locale_storage-removal-8.patch6.78 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-8.patch. Unable to apply patch. See the log in the details link for more information. View
#6 2020343-locale_storage-removal-6.patch6.74 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-6.patch. Unable to apply patch. See the log in the details link for more information. View
#6 2020343-diff-1-6.txt575 bytesvijaycs85
#1 2020343-locale_storage-removal-1.patch6.72 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-1.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

vijaycs85’s picture

Priority: Normal » Major
Status: Active » Needs review
FileSize
6.72 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-1.patch. Unable to apply patch. See the log in the details link for more information. View

Initial patch...

YesCT’s picture

So you're injecting it.

Looks like you got all of the locale_storage()'s.

---

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -56,7 +64,7 @@ class ConfigTranslationUITest extends WebTestBase {
+    $this->localeStorage = $this->container->get('locale.storage');

I get the rest of this, except for this bit in the tests. Is it like this because it's a test?

vijaycs85’s picture

sorry don't get your question here.. but there can be 2 questions out of this line :)

1. why aren't we using \Drupal::service('locale.storage) - Yes in test cases we use it's own container.
2. why assigning in ->localStorage, we can call $this->container->get('locale.storage'); all the place where we need it. but thought would be nice to have as property.

YesCT’s picture

OK.

I had started to take out locale_storage() and inject it while looking at cleaning up the manage form issue, but thought it should be it's own issue. Then I saw this issue already. Most of this is the same as what I had started, but probably better.

I talked with @vijaycs85 in irc too, to clarify some things about this patch.
I also see similar patterns used else where.

I'm going to run this and #2020347: Temporary fix for config_translation_enter_context until language context becomes an option, and if the tests pass, I think they are ok to commit, so that we can unbreak the config translation head. Then we can move on and try and improve things from there.

[update] Not all tests are passing, so I'll wait for that or word from Gabor.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

This looks like a good code change as-is. Some minor notes:

+++ b/lib/Drupal/config_translation/Controller/ConfigTranslationController.phpundefined
@@ -163,7 +163,8 @@ class ConfigTranslationController implements ControllerInterface {
+    $locale_storage = \Drupal::service('locale.storage');
+    return drupal_get_form(new ConfigTranslationManageForm($locale_storage), $group, $language, $base_config);

+++ b/lib/Drupal/config_translation/Form/ConfigTranslationManageForm.phpundefined
@@ -46,6 +47,22 @@ class ConfigTranslationManageForm implements FormInterface {
+   * Storage object.
...
+   * Creates manage form object with storage.

+++ b/lib/Drupal/config_translation/Tests/ConfigTranslationUITest.phpundefined
@@ -30,6 +31,13 @@ class ConfigTranslationUITest extends WebTestBase {
+   * Storage object.

Are we supposed to invoke this as \Drupal...? Not Drupal:: (without backslash)?

Would be good to qualify what storage are these? :) Eg. "String translation storage object" for the variable docs and argument.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
575 bytes
6.74 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-6.patch. Unable to apply patch. See the log in the details link for more information. View

As explained by @YesCT on IRC, we need to have \Drupal:: in class and Drupal:: in procedural code. so updating just comment from #5

Gábor Hojtsy’s picture

Status: Needs review » Needs work

String translation storage object only changed at one place instead of 3.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
6.78 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2020343-locale_storage-removal-8.patch. Unable to apply patch. See the log in the details link for more information. View
1.32 KB

Fixing all 3 instances.

Gábor Hojtsy’s picture

Status: Needs review » Fixed

Thanks, committed this one!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

adding link to change notice.