Updated: Comment #N

Problem/Motivation

Proposed resolution

Instead of injecting request service, rather use the Symfony 2.4 request_stack in LocalActionManager and LocalTaskManager.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#13 2223615-use-request-stack-in-local-task-action-13.diff15.51 KBznerol
#7 2223615-use-request-stack-in-local-task-action-7.diff15.5 KBznerol
#4 2223615-use-request-stack-in-local-task-action-4.diff15.49 KBznerol
#3 interdiff.txt1.31 KBznerol
#3 2223615-use-request-stack-in-local-task-action-2.diff15.47 KBznerol
#1 2223615-use-request-stack-in-local-task-action.diff14.16 KBznerol

Comments

znerol’s picture

znerol commented
Status: Active » Needs review
StatusFileSize
new 2223615-use-request-stack-in-local-task-action.diff14.16 KB

Status: Needs review » Needs work

The last submitted patch, 1: 2223615-use-request-stack-in-local-task-action.diff, failed testing.

znerol’s picture

znerol commented
Status: Needs work » Needs review
StatusFileSize
new 2223615-use-request-stack-in-local-task-action-2.diff15.47 KB
new interdiff.txt1.31 KB

Also fix LocalTaskIntegrationTest.

znerol’s picture

znerol commented
StatusFileSize
new 2223615-use-request-stack-in-local-task-action-4.diff15.49 KB

Reroll, no changes.

dawehner’s picture

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

Perfect!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2223615-use-request-stack-in-local-task-action-4.diff, failed testing.

znerol’s picture

znerol commented
Status: Needs work » Needs review
StatusFileSize
new 2223615-use-request-stack-in-local-task-action-7.diff15.5 KB

Reroll after #2221065: Rename default 'cache' cache bin to 'default'.

dawehner’s picture

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

re-rtbc

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 2223615-use-request-stack-in-local-task-action-7.diff, failed testing.

znerol’s picture

znerol commented
Status: Needs work » Needs review

7: 2223615-use-request-stack-in-local-task-action-7.diff queued for re-testing.

dawehner’s picture

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

re-rtbc

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 2223615-use-request-stack-in-local-task-action-7.diff, failed testing.

znerol’s picture

znerol commented
Status: Needs work » Needs review
StatusFileSize
new 2223615-use-request-stack-in-local-task-action-13.diff15.51 KB

Reroll after #2228291: Move all plugin and other discovery data into a cache_discovery bin

dawehner’s picture

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

Still fine.

alexpott’s picture

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

Committed bd468f2 and pushed to 8.x. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Looks like the https://drupal.org/node/2026025 could do with an update

  • Commit bd468f2 on 8.x by alexpott: 
    Issue #2223615 by znerol: Use request stack in local task/action manager...
znerol’s picture

znerol commented

@alexpot: see #2236207: Use router request context instead of request service in system breadcrumb builder and also the other child issues of #2223189: Use regular upstream HttpKernel instead of Drupal's custom.

Status: Fixed » Closed (fixed)

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

yesct’s picture

yesct commented
Related issues: +#2081153: Remove unused local variables from core/tests

this issue took out where $request was used, but didn't take out where it was set.
core/tests/Drupal/Tests/Core/Menu/LocalTaskIntegrationTest.php
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');

I suppose taking out the now unused var in #2081153: Remove unused local variables from core/tests is fine.
adding that as a related issue to this.