When building a view to display some recurly data I created a relationship to 'Recurly Account from User' and when I did suddenly my view broke and I was receiving the following errors:

Warning: Invalid argument supplied for foreach() in views_join->build_join() (line 1551 of /var/www/html/docroot/sites/all/modules/contrib/views/includes/handlers.inc).
Warning: Invalid argument supplied for foreach() in views_join->build_join() (line 1551 of /var/www/html/docroot/sites/all/modules/contrib/views/includes/handlers.inc).

When looking into this in handlers.inc I can see that $this->extra is expected to be an array. Using dpm() to inspect some of the data structures used by other views integrations I confirmed this to be the case. Other modules are presenting $extra as an array like so:

$this->extra = array(
  array(
    'field' => 'entity_type',
    'value' => 'users',
  ),
);

I noticed that in recurly/views/recurly_entity_owner_handler.inc and recurly/views/recurly_entity_owner_reverse_handler.inc that $extra was being defined as a string as below:

$def['extra'] = sprintf("%s.entity_type = '%s'", $alias, $def['base type']);

This causes the relationship to fail. This needs to be converted to use the array structure as mentioned above.

CommentFileSizeAuthor
#10 recurly-views_relationships-2895500-10.patch1.5 KBapotek
#6 Screen Shot 2017-07-18 at 4.48.00 PM.png78.87 KBowenbush
#2 recurly-views_relationships-2895500-2.patch1.44 KBowenbush
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

owenbush created an issue. See original summary.

owenbush’s picture

owenbush CreditAttribution: owenbush commented
FileSize
recurly-views_relationships-2895500-2.patch1.44 KB

Attached is a patch to address this issue for both of the custom field handlers.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Active » Needs review
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@owenbush These changes look good, and seem to work when in place, but I was unable to reproduce the error you describe. Could you provide an export of a test view so that I can see this issue?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Version: 7.x-3.0 » 7.x-3.x-dev
owenbush’s picture

owenbush CreditAttribution: owenbush commented
FileSize
Screen Shot 2017-07-18 at 4.48.00 PM.png78.87 KB

Sure no problem. I set this up and when I visit the URL /test-recurly-view I see a couple of errors. They are also in watchdog:

$view = new view();
$view->name = 'test_recurly_view';
$view->description = '';
$view->tag = 'default';
$view->base_table = 'users';
$view->human_name = 'Test Recurly View';
$view->core = 7;
$view->api_version = '3.0';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Master */
$handler = $view->new_display('default', 'Master', 'default');
$handler->display->display_options['title'] = 'Test Recurly View';
$handler->display->display_options['use_more_always'] = FALSE;
$handler->display->display_options['access']['type'] = 'perm';
$handler->display->display_options['access']['perm'] = 'access user profiles';
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'some';
$handler->display->display_options['pager']['options']['items_per_page'] = '10';
$handler->display->display_options['style_plugin'] = 'table';
$handler->display->display_options['style_options']['columns'] = array(
  'name' => 'name',
  'account_code' => 'account_code',
  'status' => 'status',
);
$handler->display->display_options['style_options']['default'] = '-1';
$handler->display->display_options['style_options']['info'] = array(
  'name' => array(
    'sortable' => 0,
    'default_sort_order' => 'asc',
    'align' => '',
    'separator' => '',
    'empty_column' => 0,
  ),
  'account_code' => array(
    'sortable' => 0,
    'default_sort_order' => 'asc',
    'align' => '',
    'separator' => '',
    'empty_column' => 0,
  ),
  'status' => array(
    'sortable' => 0,
    'default_sort_order' => 'asc',
    'align' => '',
    'separator' => '',
    'empty_column' => 0,
  ),
);
/* Relationship: User: Recurly Account from User */
$handler->display->display_options['relationships']['recurly_user']['id'] = 'recurly_user';
$handler->display->display_options['relationships']['recurly_user']['table'] = 'users';
$handler->display->display_options['relationships']['recurly_user']['field'] = 'recurly_user';
/* Field: User: Name */
$handler->display->display_options['fields']['name']['id'] = 'name';
$handler->display->display_options['fields']['name']['table'] = 'users';
$handler->display->display_options['fields']['name']['field'] = 'name';
$handler->display->display_options['fields']['name']['label'] = 'Username';
$handler->display->display_options['fields']['name']['alter']['word_boundary'] = FALSE;
$handler->display->display_options['fields']['name']['alter']['ellipsis'] = FALSE;
$handler->display->display_options['fields']['name']['element_label_colon'] = FALSE;
/* Field: Recurly account: Account code */
$handler->display->display_options['fields']['account_code']['id'] = 'account_code';
$handler->display->display_options['fields']['account_code']['table'] = 'recurly_account';
$handler->display->display_options['fields']['account_code']['field'] = 'account_code';
$handler->display->display_options['fields']['account_code']['relationship'] = 'recurly_user';
/* Field: Recurly account: Account status */
$handler->display->display_options['fields']['status']['id'] = 'status';
$handler->display->display_options['fields']['status']['table'] = 'recurly_account';
$handler->display->display_options['fields']['status']['field'] = 'status';
$handler->display->display_options['fields']['status']['relationship'] = 'recurly_user';
$handler->display->display_options['fields']['status']['element_label_colon'] = FALSE;
/* Sort criterion: User: Created date */
$handler->display->display_options['sorts']['created']['id'] = 'created';
$handler->display->display_options['sorts']['created']['table'] = 'users';
$handler->display->display_options['sorts']['created']['field'] = 'created';
$handler->display->display_options['sorts']['created']['order'] = 'DESC';
/* Filter criterion: User: Active */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'users';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['value'] = '1';
$handler->display->display_options['filters']['status']['group'] = 1;
$handler->display->display_options['filters']['status']['expose']['operator'] = FALSE;

/* Display: Page */
$handler = $view->new_display('page', 'Page', 'page');
$handler->display->display_options['path'] = 'test-recurly-view';

And just in case it is of any relevance I am using the following versions:

Views: 7.x-3.16
Recurly: 7.x-3.0

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

I tested with Views 7.x-3.16 and Recurly on both 7.x-3.0 and 7.x-3.x-dev; still unable to reproduce. I am going to leave this open for a bit and see if someone else can reproduce the issue.

apotek’s picture

apotek CreditAttribution: apotek commented
Related issues: +#2866370: Warning: Invalid argument supplied for foreach() in views_join->build_join() (line 1548 of ./sites/all/modules/views/includes/handlers.inc).

I tested with Views 7.x-3.16 and Recurly on both 7.x-3.0 and 7.x-3.x-dev; still unable to reproduce. I am going to leave this open for a bit and see if someone else can reproduce the issue.

@markdorison This thread relates to this issue: https://www.drupal.org/node/2866370

I just noticed it myself. Two symptoms:

  1. A warning Warning: Invalid argument supplied for foreach() in views_join->build_join()
  2. In some cases, extra results in the view.

The views api now requires the "extra" parameter to be an array rather than a string. However, they don't seem to specify that it has to be an associative array.

In any case, I am going to try to apply this patch to our codebase to see if it will remove some 504 and 502 errors we are getting since updating views.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@apotek Great. Let us know if this works for you and we will get it committed. Appreciate the help.

apotek’s picture

apotek CreditAttribution: apotek commented
FileSize
recurly-views_relationships-2895500-10.patch1.5 KB

I found that the previous patch created some PDO exceptions. The recurly module does some very specific things with regard to the the table aliases here, so I decided a possible fix would be to explicitly set the 'table' property of the array, based on what the old version of recurly was doing in the string. 

    $def['extra'] = array(
      array(
        'table' => $alias,
        'field' => 'entity_type',
        'value' => $def['base type'],
      ),
    );

The old recurly code explicitly sets the table in one case (reverse handler) to an *alias* rather than a table name. 

$def['extra'] = sprintf("%s.entity_type = '%s'", $alias, $def['base type']);

And in the other case (the non-reverse handler), the old code sets the table to the value of $def['left_table'] (which is probably what the views module defaults to anyway). 

$def['extra'] = sprintf("%s.entity_type = '%s'", $def['left_table'], $def['entity type']);

In any case, the patch I have attached sets the table property explicitly based on what this module was defining the relationship to be rather than leaving it up to views.

I might note that it appears, based on latest views code, that one can also send the old string as before, as long as it is wrapped in an array (which falls into the else if is_string() part of the views module code. ... But I did not test this approach.

In any case, patch 10 here is giving us decent results so far. We'll send this through QA and see if anything else breaks horribly, but would be interested in others' results as well.

apotek’s picture

apotek CreditAttribution: apotek commented

@markdorison I wasn't sure whether the latest patch has passed review, so I was hesitant to put this into status RTBC. I can say that we have been using this on one application in production for two weeks. Does that prove anything? No. Just that our use case is certainly fine with it. Would be nice if this could at least pass review, and then see if anyone else has any issues here.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Reviewed & tested by the community

@apotek This looks good in my testing. Thanks for your feedback from production.

  • markdorison committed cc03970 on 7.x-3.x authored by apotek 
    Issue #2895500 by owenbush, apotek: Views integration throws errors
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

apotek’s picture

apotek CreditAttribution: apotek commented

This seems to have been committed to 7.x-3.x-dev, and there is no upcoming release on 7.x correct?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

> and there is no upcoming release on 7.x correct?

@apotek Do you mean 7.x-3.x or a different 7.x branch? We can definitely cut a 7.x-3.x release. As for other 7.x branches, it has been over 2 years since either of them have been touched and they have been marked as "unsupported" but I suppose if there is a desire/need to get certain commits on those branches we could.