Reintroduce Views integration for locale.module

Wow the schema of locale really changed a lot since the last time someone had a look at it.

Should we actually also apply the full codestyles to new/reintroduced files?

The handlers all patch is simply great as it automatically tests that you don't have fields which aren't in the database anymore.

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/field/LinkEdit.phpundefined
@@ -0,0 +1,77 @@
+  protected function defineOptions() {

All those functions need a docblock. Something like "Overrides ..." or "Implements ..."

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/filter/Version.phpundefined
@@ -0,0 +1,42 @@
+  function get_value_options() {

Same

A basic view that uses this intgeration would increase our test coverage a lot.

This just fixes the comments, we didn't came to a good conclusion regarding the test coverage, but i agree at least the custom handlers should be tested.

Reroll. Do we need test for this?

Did you maybe forget to add a "use" statement for this annotation? Yes

Thanks for taking this up!!!!

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/field/LinkEdit.phpundefined
@@ -0,0 +1,89 @@
+ * Definition of Drupal\locale\Plugin\views\field\LinkEdit.

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/filter/Version.phpundefined
@@ -0,0 +1,42 @@
+ * Definition of Drupal\locale\Plugin\views\filter\Version.

Should be Contains ...

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/field/LinkEdit.phpundefined
@@ -0,0 +1,89 @@
+  protected function defineOptions() {

Needs an @inheritdoc

+++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/filter/Version.phpundefined
@@ -0,0 +1,42 @@
+      $result = db_query('SELECT DISTINCT(version) FROM {locales_source} ORDER BY version');

You could also inject the database connection into the handler.

+++ b/core/modules/locale/locale.views.incundefined
@@ -0,0 +1,455 @@
+ * Provides views data and handlers for locale.module.

It doesn't provide handlers anymore.

+++ b/core/modules/locale/locale.views.incundefined
@@ -0,0 +1,455 @@
+  // Basic table information.
...
+  // Define the base group of this table.
...
+  // Advertise this table as a possible base table.
...
+  // lid
...
+  // Source field
...
+  // Context field
...
+  // Version field
...
+  // Locales target table
...
+  // Define the base group of this table. Fields that don't
+  // have a group defined will go into this field by default.
...
+  // Join information
...
+  // Translation field
...
+  // Language field
...
+  // Plural

Let's drop all this pointless comments.

+++ b/core/modules/locale/locale.views.incundefined
@@ -0,0 +1,455 @@
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,

With drupal 8 all this is click sortable by default, so we can remove those.

Thanks @dawehner for the great review. Fixed all #8 and failure in #7.

Sadly the current version of the test does not contain any kind of test :(

@jibran pinged me about this in IRC. I think this would still be great to include for custom admin pages. However the locale tables changed significantly since then, so the code would need to be updated significantly.

1. +++ b/core/modules/locale/lib/Drupal/locale/Plugin/views/field/LinkEdit.php
   @@ -0,0 +1,93 @@
   +    $options['text'] = array('default' => '', 'translatable' => TRUE);
   

   What does translatable mean here?

2. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +    'help' => t('A source string for translation, in English or the default site language.'),
   

   Should only be English in Drupal 8. Non-English sources are never maintained in locale, at least definitely not by core and not intended for contrib either.

3. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +    'help' => t('The release version of the project.'),
   

   Not true. This is the last version the string was used with. See locale.install for a more accurate description.

4. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +  $data['locales_target']['plid'] = array(
   +    'title' => t('Singular LID'),
   +    'help' => t('The ID of the parent translation.'),
   +    'field' => array(
   +      'id' => 'standard',
   +    ),
   +  );
   +
   +  $data['locales_target']['plural'] = array(
   +    'title' => t('Plural'),
   +    'help' => t('Whether or not the translation is plural.'),
   

   These columns do not exist anymore, however there is a "Customized" column which would be important to expose.

5. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +  $data['locales_location']['table']['join'] = array(
   +    'locales_source' => array(
   +      'left_field' => 'sid',
   +      'field' => 'lid',
   +    ),
   +    'locales_target' => array(
   +      'left_field' => 'lid',
   +      'field' => 'lid',
   

   What's sid in locales_location is lid in both locales_source and locales_target. (I know the DB should have been cleaned up, but we did not get around to that). It is 100% sure that joining from lid to lid will not be good because they mean different things in the two tables.

6. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +  $data['locale_file']['table']['group'] = t('Locale group');
   

   Locale file instead of group IMHO.

   Also lots of columns missing from this integration (project, version, last_checked).

7. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +  $data['locale_project']['table']['group'] = t('Locale project');
   +
   +  $data['locale_project']['table']['base'] = array(
   +    'field' => 'name',
   +    'title' => t('Locale project'),
   +  );
   

   Seems to be a nonexistent table at this point. Most columns integrated to locale_file, see above.

8. +++ b/core/modules/locale/locale.views.inc
   @@ -0,0 +1,425 @@
   +function locale_views_project_types() {
   

   Don't think this one is needed either given the above notes about the table being removed.

Also definitely needs a test view or two.

Discussed with @Gábor Hojtsy at ddd23, we agreed, this is still worth adding.

If this is useful for finding exact matches for a translatable string, then how does it relate to the 'User interface translation' page at /admin/config/regional/translate? Why have 2 UIs?

There is no proposed resolution here so what needs to be done?