Currently you can configure page displays to provide itself not only as local task/local action but also
as contextual link.

As #2040379: Define contextual entity operations through annotation; remove the context property from hook_menu items might kill the functionality let's ensure that it is not just ignored.
People might interpretate this patch as a kind of asshole patch (as it makes refactoring difficult), but I just care about hidden and potential regressions.

Files: 
CommentFileSizeAuthor
#11 vdc-2051917-11.patch5.85 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,421 pass(es).
[ View ]
#9 vdc-2051917-9.patch5.93 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,057 pass(es).
[ View ]
#9 interdiff.txt2.75 KBdawehner
#6 vdc-2051917-6.patch4.48 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,007 pass(es), 4 fail(s), and 3 exception(s).
[ View ]
#4 vdc-2051917-4.patch4.66 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 57,889 pass(es), 2 fail(s), and 2 exception(s).
[ View ]
#4 interdiff.txt936 bytesdawehner
#1 vdc-2051917-1.patch4.64 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 57,519 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new4.64 KB
FAILED: [[SimpleTest]]: [MySQL] 57,519 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, vdc-2051917-1.patch, failed testing.

I don't think this is an 'asshole patch' :) We have been screwed many times by regressions, this is fair enough imo.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinks.phpundefined
@@ -0,0 +1,61 @@
+namespace Drupal\node\Tests\Views;
+use Symfony\Component\HttpFoundation\Request;

Space between

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinks.phpundefined
@@ -0,0 +1,61 @@
+    /** @var \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel */

Weird place for this comment, use '//' or we could make it a property or somthing and document it there?

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinks.phpundefined
@@ -0,0 +1,61 @@
+    $this->assertEqual(1, count($result), 'One contextual link found.');
+    $this->assertEqual('Test contextual link', (string) $result[0], 'Ensure the right link text was returned');

I like to switch these params round, I find it easier to read - that' just preference though. I think unit tests in phpunit may want them this way?

Status:Needs work» Needs review
StatusFileSize
new936 bytes
new4.66 KB
FAILED: [[SimpleTest]]: [MySQL] 57,889 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Weird place for this comment, use '//' or we could make it a property or somthing and document it there?

That is a phpstorm thing to be able to document the type of variable, which is just a good idea in general, I think.

I don't really know why this failing, as it works fine locally.

Status:Needs review» Needs work

The last submitted patch, vdc-2051917-4.patch, failed testing.

StatusFileSize
new4.48 KB
FAILED: [[SimpleTest]]: [MySQL] 58,007 pass(es), 4 fail(s), and 3 exception(s).
[ View ]

mhh

Status:Needs work» Needs review

.

Status:Needs review» Needs work

The last submitted patch, vdc-2051917-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.75 KB
new5.93 KB
PASSED: [[SimpleTest]]: [MySQL] 58,057 pass(es).
[ View ]

Seriously testing contextual links is hell.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinksTest.phpundefined
@@ -0,0 +1,101 @@
+      'description' => 'Tests contextual links provided by views on nodes.',

Maybe 'Tests views contextual links on nodes' instead?

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinksTest.phpundefined
@@ -0,0 +1,101 @@
+   * Test contextual links.

Tests.. sorry

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/NodeContextualLinksTest.phpundefined
@@ -0,0 +1,101 @@
+    $result = $this->xpath("//a[contains(@href, 'node/1/contextual-links')]");
+    $this->assertEqual(count($result), 1, 'One contextual link found.');
+    $this->assertEqual((string) $result[0], 'Test contextual link', 'Ensure the right link text was returned');

We could still use assertlink() and friends here? I guess you want the count first, so might as well use the xpath result you have?

+++ b/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_contextual_links.ymlundefined
@@ -0,0 +1,98 @@
+label: Frontpage

New label please :)

StatusFileSize
new5.85 KB
PASSED: [[SimpleTest]]: [MySQL] 58,421 pass(es).
[ View ]

Thanks for the review!

Status:Needs review» Reviewed & tested by the community

Who did this to contextual links? Ouch.

The ugliness of this code is not @dawehner's fault.

Test looks great.

#11: vdc-2051917-11.patch queued for re-testing.

#12: you're probably referring to the fact that you need HTTP requests for contextual links to show up; I did that, but it was necessary to fix the render cache, see #914382: Contextual links incompatible with render cache.
If you're referring to test ugliness — then I'm happy to point you to #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests which fixes that :) mondrake just +1'd that issue and tagged it with VDC because you apparently encountered the same problem in Views.

#11: vdc-2051917-11.patch queued for re-testing.

Status:Reviewed & tested by the community» Fixed

Yay, more tests!

It seems like this could benefit from #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests once that's in, but that's still under discussion, so for now, curlExec() it is, I suppose.

Committed and pushed to 8.x. Thanks!

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