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

Comments

abhishek-anand’s picture

abhishek-anand commented
StatusFileSize
new fix_for_hook_views_query_alter-2388941.patch906 bytes
abhishek-anand’s picture

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

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

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

dawehner’s picture

dawehner
Primary language German
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
commented

Ah, OK. Then is this RTBC dawehner?

dawehner’s picture

dawehner
Primary language German
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 🇪🇺🌍
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
commented
StatusFileSize
new 2388941-update-helper-8.patch1.49 KB
new 2388941-diff-1-8.txt1.5 KB

Here we go.

dawehner’s picture

dawehner
Primary language German
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
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
commented
Component: documentation » views.module

Not docs any more. :)

mrjmd’s picture

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

Fixes from #10 .

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
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 🇨🇦
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.