I created a VBO action plugin which pass_view is true, after executing the action, it is broken and throws the following message. I also tested the views_bulk_operations_example action, there is the same problem on it.

Error: Call to a member function addMetaData() on string in Drupal\views\Plugin\views\query\Sql->execute() (line 1408 of XXX/core/modules/views/src/Plugin/views/query/Sql.php).

My Drupal core version is 8.4.2. The snippet of Sql.php is:

  public function execute(ViewExecutable $view) {
    $query = $view->build_info['query'];
    $count_query = $view->build_info['count_query'];

    $query->addMetaData('view', $view);
    $count_query->addMetaData('view', $view);
CommentFileSizeAuthor
#13 views_bulk_operations_pass_view_true-2923621-13.patch4.25 KBgraber
#12 views_bulk_operations_pass_view_true-2923621-12.patch3.92 KBgraber
#9 views_bulk_operations_pass_view_true-2923621-9.patch976 bytesjohnhuang0808
#8 vbo_excpetion.jpg65.51 KBjohnhuang0808
#8 content_vbo_views_field.jpg181.09 KBjohnhuang0808
#8 content_context_filter.jpg213.26 KBjohnhuang0808
#8 content_path.jpg171.06 KBjohnhuang0808
#2 views_bulk_operations_pass_view_true-2923621-2.patch548 bytesjohnhuang0808

Comments

johnhuang0808 created an issue. See original summary.

johnhuang0808’s picture

johnhuang0808 commented
Issue summary: View changes
StatusFileSize
new views_bulk_operations_pass_view_true-2923621-2.patch548 bytes

In the patch, before executing the query, it should be build necessary info, otherwise, the query object is null.
Please take a look.

johnhuang0808’s picture

johnhuang0808 commented
Status: Active » Needs review
graber’s picture

graber commented

I restructured the ViewsBulkOperationsActionProcessor class a bit in the last commit (f42efa5), it contains the setView() method that is called on initialization and includes $this->view->build(); instruction that should build the query under normal circumstances (there are some cases where the query isn't built, see public function ViewExecutable::build).

Can you pull the latest dev and see if the problem persists?

If yes, it'd be good to provide steps to reproduce, as I couldn't with the last version.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

We may want to run pre_execute(), so that those hook_views_pre_execute() hooks still fire, I've a related D7 patch in the queue.

johnhuang0808’s picture

johnhuang0808 commented

Hi @Graber, in the latest commit (f42efa5), the problem still exists in my development site.
But, I cannot reproduce it in a whole new Drupal site. I need some time to look for the root problem.

@joelpittet Could you share the issue link?

johnhuang0808’s picture

johnhuang0808 commented
Status: Needs review » Active
johnhuang0808’s picture

johnhuang0808 commented
StatusFileSize
new content_path.jpg171.06 KB
new content_context_filter.jpg213.26 KB
new content_vbo_views_field.jpg181.09 KB
new vbo_excpetion.jpg65.51 KB

After digging into the ViewExecutale::build(), the problem is caused by the views' contextual filters without default value.

Reproduce steps (Click to see the larger image):
1. Enable the views_bulk_operations_example module
2. Use the exiting content views in Drupal and create a new page display
3. Set the path (eg. /vbo_testing)
Views path
4. Add a user id context filter
5. Choose Show "Page not found"
Views path
6. Add a "Views bulk operations" views field
7. Select "VBO example action"
Views path
8. Save the views
9. Go to /vbo_testing/1
10. Perform the "VBO example action"
11. Receive an exception
Views path

Since the ViewsBulkOperationsActionProcessor::setView() includes $this->view->build();, I traced build() via XDebug to understand the behavior. I found $this->_buildArguments() in ViewExecutale returns FALSE, so $this->query->build($this); never executed to build the necessary info.

The snippet code of ViewExecutale::build():

  public function build($display_id = NULL) {
    /* Ignore... */

    // If that fails, let's build!
    $this->build_info = [
      'query' => '',
      'count_query' => '',
      'query_args' => [],
    ];

    $this->initQuery();

    /* Ignore... */

    // Arguments can, in fact, cause this whole thing to abort.
    if (!$this->_buildArguments()) {
      $this->build_time = microtime(TRUE) - $start;
      $this->attachDisplays();
      return $this->built;
    }

    /* Ignore... */

    // Only build the query if we weren't interrupted.
    if (empty($this->built)) {
      // Build the necessary info to execute the query.
      $this->query->build($this);
    }

    /* Ignore... */

    return TRUE;
  }
johnhuang0808’s picture

johnhuang0808 commented
StatusFileSize
new views_bulk_operations_pass_view_true-2923621-9.patch976 bytes

I removed the if ($form_state->getValue('select_all')) condition in ViewsBulkOperationsBulkForm::viewsFormSubmit(), the views argument and exposed input will pass into ViewsBulkOperationsActionProcessor. I have no idea why should user need to select all.

graber’s picture

graber commented

Good find @johnhuang0808!
Those values should be passed to the tempstore, regardless of the select_all option to build the view the proper way if it needs to be passed to the selected action object or used on a custom configuration / confirmation form.

@joelpitet, yes, the way we currently execute the view query omits the $module_handler->invokeAll('views_pre_execute', array($this)); that is a part of standard ViewExecutable::execute(). I'll check if we can change $this->view->query->execute($this->view); to $this->view->execute(); in ViewsBulkOperationsActionProcessor.

To solve this issue, committing the #9 patch.

Thanks!

  • Graber committed 21154c7 on 8.x-1.x authored by johnhuang0808 
    Issue #2923621 by johnhuang0808: If a action's pass_view is true, the...
graber’s picture

graber commented
StatusFileSize
new views_bulk_operations_pass_view_true-2923621-12.patch3.92 KB

I rushed a bit, those parameters should be populated and passed to the action processor even if there is no redirect route (no batching, confirmation or configuration), but in such a case we can just pass the entire view object instead of building it.

New patch attached, same effect but broader scope.

graber’s picture

graber commented
Status: Active » Needs review
StatusFileSize
new views_bulk_operations_pass_view_true-2923621-13.patch4.25 KB

Forgot to actually include the view as an argument.

johnhuang0808’s picture

johnhuang0808 commented
Status: Needs review » Reviewed & tested by the community

@Graber Thanks for the quick reply! The patch #13 works for me and looks great! Sorry, I ignored the no confirmation action in my patch #9.
I also tested on the actions with pass_view true and false.

  • Graber committed 1dd093f on 8.x-1.x 
    Issue #2923621 by johnhuang0808, Graber: If a action's pass_view is true...
graber’s picture

graber commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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