follow up of #2004086: The Request service must be synthetic (as per #27)

Problem

The views executable factory ensures that the request is injected.
We no longer need to try/catch whether a request exists.

Proposed solution

Get rid of the try catch.

CommentFileSizeAuthor
#10 2059003-views_request-9.patch1.19 KBvijaycs85
#5 2059003-views_request-5.patch1.24 KBvijaycs85
#3 vdc-2059003-3.patch1.02 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

What should not depend on the request?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes

Updated issue summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

I think the problem is in Drupal/views/Plugin/views/display/DisplayPluginBase::getHandlers()

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
Status: Active » Needs review
FileSize
vdc-2059003-3.patch1.02 KB

I guess you talk about that ...

znerol’s picture

znerol CreditAttribution: znerol commented

Now that #2261181: Always pass in the request into the view executable is in, can we close this issue?

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
FileSize
2059003-views_request-5.patch1.24 KB

Hope this issue is still valid as we need to remove the @todo and the try...catch block?

Status: Needs review » Needs work

The last submitted patch, 5: 2059003-views_request-5.patch, failed testing.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs work » Needs review

5: 2059003-views_request-5.patch queued for re-testing.

dawehner’s picture

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

Yeah we can indeed remove the try/catch statement.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2059003-views_request-5.patch, failed testing.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs work » Needs review
FileSize
2059003-views_request-9.patch1.19 KB

reroll...

Status: Needs review » Needs work

The last submitted patch, 10: 2059003-views_request-9.patch, failed testing.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs work » Needs review

Reg#11: Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 536870912 bytes) in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php on line 336
FATAL Drupal\system\Tests\Entity\EntityCrudHookTest: test runner returned a non-zero error code (255).

vijaycs85 queued 10: 2059003-views_request-9.patch for re-testing.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs review » Reviewed & tested by the community

back to RTBC as per #8

chx’s picture

chx CreditAttribution: chx commented
Title: Remove dependency on Request object. » Remove Views display plugins dependency on Request object.
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Component: base system » views.module
Issue tags: +VDC
alexpott’s picture

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

Proposed solution
Yet to find.

Hmmm yet the patch is rtbc :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Title: Remove Views display plugins dependency on Request object. » Remove try/catch around getRequest() in DisplayPluginBase::getHandlers()
Status: Reviewed & tested by the community » Fixed

Committed 9eba380 and pushed to 8.x. Thanks!

  • alexpott committed 9eba380 on 8.x 
    Issue #2059003 by vijaycs85, dawehner: Remove try/catch around...

Status: Fixed » Closed (fixed)

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