Problem/Motivation
The method \Drupal\views\ViewExecutableFactory::get raises an error when a session is not available - for example when invoked via drush.
\Symfony\Component\HttpFoundation\RequestStack::getCurrentRequest can return NULL, but the return value is not checked before passing it as an argument to \Drupal\views\ViewExecutable::setRequest.
Proposed resolution
- Check return value from
getCurrentRequestbefore callingsetRequest
Remaining tasks
Test coverage to demonstrate issueUpdate method
User interface changes
none
Introduced terminology
none
API changes
none
Data model changes
none
Release notes snippet
N/A ?
| Comment | File | Size | Author |
|---|
Issue fork drupal-3532360
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
aaronbauman3532360-TEST-ONLY MR !12485 - test only to demonstrate bug.
3532360-check-for-session MR !12486 - includes test and fix, ready for review
Comment #7
oily commented@aaronbauman A separate 'test-only' branch is not necessary. There is a 'test-only' manually activated test in the pipeline.
Comment #8
aaronbaumanOh, thanks for the heads up - TIL!
Is that a configuration for core only, or for all projects?
Comment #9
oily commentedThere is a gitlab template that can be used in projects. Can use it at default settings or vary them. I think most projects will have test-only test in pipeline unless the maintainer had a good reason to exclude it.
Comment #10
oily commented@aaronbauman you also probably need to create a new 11.x branch and merge into that. Core maintainers then decide which branches to port to.
Comment #11
quietone commentedIn Drupal core changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies. Also mentioned on the version section of the list of issue fields documentation.
Comment #12
smustgrave commentedMR needs to be updated for 11.x please.
Comment #15
aaronbaumanOpened a clean Merge request !12486 against 11.x for review
Comment #16
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #17
aaronbaumanComment #20
smustgrave commentedRan the test-only run here https://git.drupalcode.org/issue/drupal-3532360/-/jobs/6995154 to show the coverage
Left a comment on the MR.
If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
Comment #21
aaronbaumanMerged 11.x again, and updated per your comment.
Thanks for the feedback.
Comment #22
smustgrave commentedThanks feedback appears to be addressed
Comment #23
catchThe MR doesn't match the issue title or summary at all.
Comment #24
aaronbaumanSorry, big copy-pasta error on my part.
Updated IS.
Comment #25
aaronbaumanComment #26
smustgrave commentedThat was my mistake for not catching before but title looks much better
Comment #27
catchComment #31
catchCommitted/pushed to main, 11.x and 11.3.x, thanks!