add a views_data handler

So should I add patches here?

This is related here #2378729: JoinPluginBase doesn't allow extra conditions on left table. It will allow to create a relationship between flagged to entity type data table.
Something like this.

    $data['flagging'][$base_table] = [
      'title' => t('Flagged @label', ['@label' => $info->getLabel()]),
      'help' => t('The @label flagged by this flag.', ['@label' => $info->getLabel()]),
      'relationship' => [
        'base' => $base_table,
        'entity type' => $info->id(),
        'base field' => $info->getKey('id'),
        'relationship field' => 'entity_id',
        'title' => t('@label flagged using this flag.', ['@label' => $info->getLabel()]),
        'label' => t('Flagged: @label', ['@label' => $info->getLabel()]),
        'group' => t('Flag'),
        'id' => 'standard',
        'extra' => [
          0 => [
            'left_field' => 'entity_type',
            'value' => $info->id(),
          ],
        ],
      ],
    ];

After adding the relationship it results in

SELECT flagging.id AS id, node_field_data_flagging.nid AS node_field_data_flagging_nid, nid AS nid
FROM 
{flagging} flagging
INNER JOIN {node_field_data} node_field_data_flagging ON flagging.entity_id = node_field_data_flagging.nid AND flagging.entity_type = 'node'
LIMIT 10 OFFSET 0

Above issue allows us to add AND flagging.entity_type = 'node' condition to join.

+++ b/src/FlagViewsData.php
@@ -0,0 +1,96 @@
+    // @todo Remove this when data table is actually used.
+    unset($data['flagging_data']);
+

Instead of unsetting it here, you should remove it from the annotation. It only makes sense to have it when Translatable there is set to true.

Oh I kind a forgot this issue. Now that #2321721: Provide a views relationship for each dynamic entity reference field is RTBC I have time and I think I can move forward here as join issue is also in, views data using data tables now and views is using fields plugin for views fields. I'll try to have a look at it during this weekend.

Oops.

I think we should reduce the focus of this issue to providing a views_data handler for flaggings that subclasses from EntityViewsData. We can rename all the handler classes as a follow-up.

I'm in the process of looking at our views support now that our flagging field names are fixed, and I've filed a number of bugs both here and in core.

Some things to take into account:

- we should omit the delete link field
- until #2491875: EntityViewsData adds Operations links to all entities, which won't work if the entity type has no list builder, leading to WSOD on some views is fixed, we have to omit the entity operations field

Rerolled the patch with the renamings.

Added the rendering_language in the default config, to get rid of the for the SQL errors.

Next up: tests.

Added the test and the default config for the view.

Thanks for working on this.

It does still need scaling back as detailed in #10: filename changes and plugin name changes should be left for future issues. This should be just about making use of a views_data handler.

Tests are a good idea, but I'm not sure what they are covering.

1. +++ b/src/Tests/FlagViewsTest.php
   @@ -0,0 +1,174 @@
   +   * Flag creation.
   +   */
   +  public function doTestFlagAdd() {
   +    // Create content type.
   +    $this->drupalCreateContentType(['type' => $this->nodeType]);
   +
   +    // Test with minimal value requirement.
   +    $this->drupalPostForm('admin/structure/flags/add', [], t('Continue'));
   +    // Check for fieldset titles.
   +    $this->assertText(t('Messages'));
   +    $this->assertText(t('Flag access'));
   +    $this->assertText(t('Display options'));
   +
   +    $edit = [
   +      'label' => $this->label,
   +      'id' => $this->id,
   +      'types[' . $this->nodeType . ']' => $this->nodeType,
   +    ];
   +    $this->drupalPostForm(NULL, $edit, t('Create Flag'));
   +
   +    $this->assertText(t('Flag @this_label has been added.', ['@this_label' => $this->label]));
   

   This is covered by other tests -- we don't need to go via the admin UI to create our flag here, just create it programatically. Or for that matter, your test module seems to contain a flag in config code.

2. +++ b/src/Tests/FlagViewsTest.php
   @@ -0,0 +1,174 @@
   +   * Node test flags for views.
   +   */
   +  public function doTestFlagView() {
   +    $this->drupalGet('admin/structure/views');
   +    $this->assertText('Flags');
   

   I'm not sure what this is testing! Can a comment be added please?

3. +++ b/src/Tests/FlagViewsTest.php
   @@ -0,0 +1,174 @@
   +  /**
   +   * Node creation and flagging.
   +   */
   +  public function doTestCreateNodeAndFlagIt() {
   

   This seems to be copied from another test? I don't know why we're testing flagging a node all over again here.

BTW, I don't understand what this bit does:

+++ b/src/FlagViewsData.php
@@ -0,0 +1,96 @@
+    $data['flagging']['entity_id']['argument']['id'] = 'flag';

Work in progress, rushed for time... not had a chance to look at the tests.

Also, the relationship from entity to flagging seems to be broken, so maybe this issue should be postponed on fixing that.

+++ b/flag.views.inc
@@ -145,9 +56,7 @@ function flag_views_data_alter(array &$data) {
   foreach ($flags as $flag_id => $flag) {
-    $flag_type_plugin = $flag->getFlagTypePlugin();
-    $flag_type_def = $flag_type_plugin->getPluginDefinition();
-    $entity_type = $flag_type_def['entity_type'];
+    $entity_type = $flag->getFlaggableEntityTypeId();
 

This is the problem. It's crept into this patch, despite being part of another issue which has now been committed -- hence why this patch is not applying.

Rebased my branch and rerolled.

Oops.

This looks fine. We're just refactoring code, so I don't think we need the needs tests tag anymore.

Thanks for the review!

Committed.