Problem/Motivation

#2165989: Add a Views field handler for multiple entity operations contained a bug where the 'destination' query parameter is not set correctly.

Proposed resolution

In EntityOperations, modify $operation['query'] rather than $operation['options']['query'].

Remaining tasks

Patch EntityOperations
Make sure it is tested

User interface changes

None

API changes

None

CommentFileSizeAuthor
#14 operations-destination-2419905-14.interdiff.txt878 bytesArla
#14 operations-destination-2419905-14.patch7.56 KBArla
#11 operations-destination-2419905-11.patch7.8 KBArla
#7 operations-destination-2419905-7.interdiff.txt1.55 KBArla
#7 operations-destination-2419905-7.patch8.13 KBArla
#5 operations-destination-2419905-5.interdiff.txt1.94 KBArla
#5 operations-destination-2419905-5.patch8.04 KBArla
#1 operations-destination-2419905-1-FIX_ONLY.patch1.07 KBArla
#1 operations-destination-2419905-1.patch7.3 KBArla
#1 operations-destination-2419905-1-TESTS_ONLY.patch6.38 KBArla
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Active » Needs review
Issue tags: -Novice
FileSize
operations-destination-2419905-1-TESTS_ONLY.patch6.38 KB
operations-destination-2419905-1.patch7.3 KB
operations-destination-2419905-1-FIX_ONLY.patch1.07 KB

The tests missed this bug because they are using the Node entity type, which adds the destination parameter anyway in the list builder. This patch modifies one of the tests to check for the destination parameter, using another entity type that does not set the parameter by itself.

The last submitted patch, 1: operations-destination-2419905-1.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 1: operations-destination-2419905-1-FIX_ONLY.patch, failed testing.

The last submitted patch, 1: operations-destination-2419905-1-TESTS_ONLY.patch, failed testing.

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Needs work » Needs review
FileSize
operations-destination-2419905-5.patch8.04 KB
operations-destination-2419905-5.interdiff.txt1.94 KB

Oops, bad tests.

Status: Needs review » Needs work

The last submitted patch, 5: operations-destination-2419905-5.patch, failed testing.

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Needs work » Needs review
FileSize
operations-destination-2419905-7.patch8.13 KB
operations-destination-2419905-7.interdiff.txt1.55 KB

Er.. not sure why I thought $entity->url('collection') would be appropriate to use there.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

looks good, +1 rtbc

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +VDC

@Arla thanks for the bug report and patch

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: operations-destination-2419905-7.patch, failed testing.

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
Status: Needs work » Needs review
FileSize
operations-destination-2419905-11.patch7.8 KB

Rerolled.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_operations.yml
@@ -158,27 +136,7 @@ display:
-      sorts:
-        created:
-          id: created
-          table: node_field_data
-          field: created
-          order: DESC
-          plugin_id: date
-          entity_type: node
-          entity_field: created

Why are we removing the sorts? This kind of thing can break running the test on postgres.

Arla’s picture

Arla
they/them
CreditAttribution: Arla commented
FileSize
operations-destination-2419905-14.patch7.56 KB
operations-destination-2419905-14.interdiff.txt878 bytes

I think there was no reason other than clean code. I know nothing about Postgres, but if you say so :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Also the filter removal kinda looks out of scope, but I agree, its not needed to be as part of the test.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

re the filter removal - I'm not a huge fan of unrelated changes in patches but given I've knocked this back once I'm not inclined to again since it is test code and the tests pass.

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed c2f9434 and pushed to 8.0.x. Thanks!

  • alexpott committed c2f9434 on 8.0.x 
    Issue #2419905 by Arla: Views field 'Operations links' fails to set...
Arla’s picture

Arla
they/them
CreditAttribution: Arla commented

Thank you! Point taken about unrelated changes, sorry for the inconvenience.

Status: Fixed » Closed (fixed)

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