Issue Summary

Incorrect way to fetch view id in hook_views_query_alter in views.api.php

https://api.drupal.org/api/drupal/core%21modules%21views%21views.api.php...

$view->name == 'my_view'

should be changed to

$view->storage->id() == 'my_view'

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug
Issue priority Not critical because minor
Unfrozen changes Unfrozen because it only changes code to improve DX by making ViewExecutable more in line with View objects (and other entity types)
CommentFileSizeAuthor
#12 interdiff.txt682 bytesmrjmd
#12 2388941-update-helper-12.patch1.54 KBmrjmd
#8 2388941-diff-1-8.txt1.5 KBvijaycs85
#8 2388941-update-helper-8.patch1.49 KBvijaycs85
#1 fix_for_hook_views_query_alter-2388941.patch906 bytesabhishek-anand
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

abhishek-anand’s picture

abhishek-anand CreditAttribution: abhishek-anand commented
FileSize
fix_for_hook_views_query_alter-2388941.patch906 bytes
abhishek-anand’s picture

abhishek-anand CreditAttribution: abhishek-anand commented
Status: Active » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: +VDC

Can you just use $view->id() rather than $view->storage->id()?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Can you just use $view->id() rather than $view->storage->id()?

No you can't, $view is no the entity but the view executable which contains the logic of executing a view.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Ah, OK. Then is this RTBC dawehner?

dawehner’s picture

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

I'm drunken and fine with that

alexpott’s picture

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

Is it not worth adding a method to ViewExecutable called id() that just delegates to the $this->storage->id()?

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
FileSize
2388941-update-helper-8.patch1.49 KB
2388941-diff-1-8.txt1.5 KB

Here we go.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/views/src/ViewExecutable.php
@@ -446,6 +446,17 @@ public function __construct(ViewStorageInterface $storage, AccountInterface $use
+   * @return string|int|null

Why should it ever be an int?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs review » Needs work

Setting to needs work based on #9. I don't think it ever would return an int.

Also:

+++ b/core/modules/views/src/ViewExecutable.php
@@ -446,6 +446,17 @@ public function __construct(ViewStorageInterface $storage, AccountInterface $use
    * @todo.

While we're here, can we update this '@todo' to an actual docblock comment?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Component: documentation » views.module

Not docs any more. :)

mrjmd’s picture

mrjmd CreditAttribution: mrjmd commented
Status: Needs work » Needs review
FileSize
2388941-update-helper-12.patch1.54 KB
interdiff.txt682 bytes

Fixes from #10 .

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

I think this now addresses #7.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Indeed.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 74128e4 on 8.0.x 
    Issue #2388941 by vijaycs85, mrjmd, abhishek-anand, dawehner, alexpott:...

Status: Fixed » Closed (fixed)

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