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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

znerol’s picture

znerol CreditAttribution: znerol commented
Status: Active » Needs review
FileSize
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 CreditAttribution: znerol commented
Status: Needs work » Needs review
FileSize
2223615-use-request-stack-in-local-task-action-2.diff15.47 KB
interdiff.txt1.31 KB

Also fix LocalTaskIntegrationTest.

znerol’s picture

znerol CreditAttribution: znerol commented
FileSize
2223615-use-request-stack-in-local-task-action-4.diff15.49 KB

Reroll, no changes.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner 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 CreditAttribution: znerol commented
Status: Needs work » Needs review
FileSize
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
CreditAttribution: dawehner 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 CreditAttribution: 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
CreditAttribution: dawehner 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 CreditAttribution: znerol commented
Status: Needs work » Needs review
FileSize
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
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Still fine.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott 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 🇪🇺🌍
CreditAttribution: alexpott 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 CreditAttribution: 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 CreditAttribution: 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.