EntityViewsData::getViewsData() does this:

      $data[$revision_table]['operations'] = [
        'field' => [
          'title' => $this->t('Operations links'),
          'help' => $this->t('Provides links to perform entity operations.'),
          'id' => 'entity_operations',
        ],
      ];

but for some entities -- eg user -- $revision_table will be an empty string.

Therefore, the views data array gets some pointless entity operations declared under a key that's just an empty string.

CommentFileSizeAuthor
#13 interdiff-6-12.txt474 bytesmitrpaka
#13 EntityViewsData_3002927-12.patch993 bytesmitrpaka
#11 EntityViewsData_3002927-11.patch1019 bytesbn_code
#6 EntityViewsData_3002927-6.patch1001 bytesdhirendra.mishra
#6 interdiff_2-6.txt474 bytesdhirendra.mishra
#2 3002927.patch993 byteslongwave

Comments

joachim created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3002927.patch993 bytes

Not sure how to write a test for this, or if we actually need one.

joachim’s picture

joachim commented
Status: Needs review » Reviewed & tested by the community

Patch looks good.

I agree, I don't think this needs to be tested.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3002927.patch, failed testing. View results

dhirendra.mishra’s picture

dhirendra.mishra commented
Assigned: Unassigned » dhirendra.mishra
dhirendra.mishra’s picture

dhirendra.mishra commented
Assigned: dhirendra.mishra » Unassigned
Status: Needs work » Needs review
StatusFileSize
new interdiff_2-6.txt474 bytes
new EntityViewsData_3002927-6.patch1001 bytes

Adding condition of !empty.Please check.

joachim’s picture

joachim commented

There's no need for an !empty check, as the code further up initializes the variable to an empty string.

dhirendra.mishra’s picture

dhirendra.mishra commented

@joachim, But not using !empty condition will allow to create $data element array with empty key.

Instead can we add !empty condition for variable $revision_table everywhere in code. For example please see below line of code. can you please confirm?

if ($revision_table) {
$data[$revision_table]['table']['group'] = $this->t('@entity_type revision', ['@entity_type' => $this->entityType->getLabel()]);
$data[$revision_table]['table']['provider'] = $this->entityType->getProvider();

$views_revision_base_table = $revision_table;
if ($revision_data_table) {
$views_revision_base_table = $revision_data_table;
}
$data[$views_revision_base_table]['table']['entity revision'] = TRUE;
$data[$views_revision_base_table]['table']['base'] = [
'field' => $revision_field,
'title' => $this->t('@entity_type revisions', ['@entity_type' => $this->entityType->getLabel()]),
];
// Join the revision table to the base table.
$data[$views_revision_base_table]['table']['join'][$views_base_table] = [
'left_field' => $revision_field,
'field' => $revision_field,
'type' => 'INNER',
];

if ($revision_data_table) {
$data[$revision_data_table]['table']['group'] = $this->t('@entity_type revision', ['@entity_type' => $this->entityType->getLabel()]);
$data[$revision_data_table]['table']['entity revision'] = TRUE;

$data[$revision_table]['table']['join'][$revision_data_table] = [
'left_field' => $revision_field,
'field' => $revision_field,
'type' => 'INNER',
];
}

// Add a filter for showing only the latest revisions of an entity.
$data[$revision_table]['latest_revision'] = [
'title' => $this->t('Is Latest Revision'),
'help' => $this->t('Restrict the view to only revisions that are the latest revision of their entity.'),
'filter' => ['id' => 'latest_revision'],
];
}

joachim’s picture

joachim commented

I don't understand what you mean at all, sorry.

The variable is initialised to an empty string:

  public function getViewsData() {
...
    $revision_table = '';

so if ($revision_table) {} is enough to check whether it is either an empty string (and so evaluates to FALSE) or a non-empty string (and so evaluates to TRUE).

bn_code’s picture

bn_code commented
Assigned: Unassigned » bn_code
bn_code’s picture

bn_code commented
StatusFileSize
new EntityViewsData_3002927-11.patch1019 bytes

Agree with #9

Status: Needs review » Needs work

The last submitted patch, 11: EntityViewsData_3002927-11.patch, failed testing. View results

mitrpaka’s picture

mitrpaka commented
Status: Needs work » Needs review
StatusFileSize
new EntityViewsData_3002927-12.patch993 bytes
new interdiff-6-12.txt474 bytes

Re-rolled latest patch (which seems to be same as original by @longwave)

Status: Needs review » Needs work

The last submitted patch, 13: EntityViewsData_3002927-12.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

In order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

  1. https://www.drupal.org/docs/8/testing
  2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.7.x
joachim’s picture

joachim commented

@alexpott we discussed writing tests earlier in the issue, but couldn't figure out how to test this. Any ideas?

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vacho’s picture

vacho commented

Hi. Some ideas about how to test this.

The logic
This issue is to solve this logic:
If the entity is revisionable => they can have revision table operations.

Currently,
If the entity is not revisionable => they have a revision table anyway.

Test logic
- Step 1: Set A view that show an entity that is not revisionable such as User.
- Step 2: Add to view operations column.
- Step 3: Valid if generate operations anyway or not.

Tecnical options
- Unit: Is for an environment
- Kernel: Is for test full Drupal environment.
- Browser: Is for test full-environment and internal simulate a web browser result. (It is the test that we can implement)
- Javascript: Javascript execution.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

vsujeetkumar’s picture

vsujeetkumar commented
Related issues: +#2973137: EntityViewsData missed revisionable validation

@alexpott I think this one is duplicate with #2973137: and already fixed, Please review.

vsujeetkumar’s picture

vsujeetkumar commented
Status: Needs work » Needs review

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Closed (duplicate)

Closing as a duplicate of https://www.drupal.org/project/drupal/issues/2973137. after confirming the fix for this ticket was included in that ticket.