I have a view with a Panels Pane display with two contextual arguments. These arguments are optional and both set to retrieve their content from a Node-ID argument from the panel. In Panels I have attached these arguments to two different node-type contexts, which are optional.

When the first context is not available, the second context is passed as the first context. It think this relates to the following code in views_panes.inc:

      case 'context':
        $key = array_shift($context_keys);
        if (isset($contexts[$key])) {
          if (strpos($argument['context'], '.')) {
            list($context, $converter) = explode('.', $argument['context'], 2);
            $args[] = ctools_context_convert_context($contexts[$key], $converter);
          }
          else {
            $args[] = $contexts[$key]->argument;
          }
        }
        break;

There is no } else { for the outer if. When I change it to the following, it works. The only thing is: 'all' is configurable, so this shouldn't be hard-coded, but retrieved from the view. Note: passing NULL here doesn't work.

        $key = array_shift($context_keys);
        if (isset($contexts[$key])) {
          if (strpos($argument['context'], '.')) {
            list($context, $converter) = explode('.', $argument['context'], 2);
            $args[] = ctools_context_convert_context($contexts[$key], $converter);
          }
          else {
            $args[] = $contexts[$key]->argument;
          }
        }
        else {
          $args[] = 'all';
        }
        break;
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TripleEmcoder’s picture

I can confirm this issue and the fix above. Instead of 'all' it is correct to insert NULL into the array, exactly as the comment points out a few lines further down:

     case 'none':
     default:
       // Put in NULL.
       // views.module knows what to do with NULL (or missing) arguments
       $args[] = NULL;
       break;
Jorrit’s picture

Version: 7.x-1.2 » 7.x-1.x-dev
Status: Active » Needs review
FileSize
1.02 KB

I recall that I tried NULL as well, that is of course the first thing that comes to mind. But it didn't work. I eventually found out where to find the default setting for the display argument, so the attached patch also works when people change 'all' to something else.

Maybe the $args[] = NULL part needs to be changed too.

fago’s picture

Status: Needs review » Reviewed & tested by the community

I ran over the same problem. After debugging I figured that NULL does not work either, I agree that passing the exception value is a reasonable fix. Maybe Views should not stop processing if it does not find an argument, but as long as Views behaves it it does now I think that's the best fix - thus marking RTBC.

Marked #1946000: Passing optional arguments to content pane as duplicate.

Maybe the $args[] = NULL part needs to be changed too.

I guess so, but that needs a bit more thought so unnecessary NULL values at the end are still removed before passing things on. I'd suggest committing #2 right now and handling the 'none' case in a separate issue.

codycraven’s picture

I believe this is a duplicate of #1917658: Empty context value results in missing argument in views argument.

The patch I supplied in the related issue does not assume 'all' but instead passes NULL as that seems to be the intended functionality in views_content_views_content_type_render() of views.inc.

Had I found this issue when searching for my problem I would have posted my patch to this issue (posted earlier the same day as the patch in #2).

Jorrit’s picture

I don't think they are duplicates. Your patch patches 'views.inc', which handles the inclusion of all views as a pane, while my patch is on 'views_panes.inc', which only handles displays of type Panels Pane.

Fago confirmed that NULL does not work like the default value. I have been digging a little deeper and I think the root cause is a bug in Views. When a missing argument is encountered in view->_build_arguments(), the loop foreach ($this->argument as $id => $arg) { is ended with a break (around line 865). Replacing that with a continue; will allow NULLs.

[edit]
I found a related Views issue: #1250336: Any argument after a missing one is silently ignored (D7).

japerry’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

lex0r’s picture

Status: Closed (fixed) » Needs work

I'm sorry to re-open this but I found a case when this change leads to some undocumented and quite unexpected behavior. Imagine you have a panel page with a context and a view on it. The view has a contextual filter, but it is not configured to use panel context and does its own processing and context detection from URL. When the view's pane is called, the last "if" condition

        else {
          $args[] = isset($arguments[$id]['exception']['value']) ? $arguments[$id]['exception']['value'] : 'all';
        }

is always hit in views_panes.inc and thus we force an exception value on that view. By doing that we prevent the view from processing the arguments the way it wants to, making its contextual filter ineffective...

I can see your objections like "why not using panels context then for that view?" and I completely agree this is a good solution. However you should be aware that using own view argument handling (when the view is going to be rendered as view pane) is then an unwanted behavior and should be documented at least. Also, in theory, a view may want to have its own argument handling logic, not relying on panels context.

  • japerry committed 41c0d5f on 8.x-2.x authored by Jorrit
    Issue #1863166 by Jorrit: Bad arguments when first optional context...

  • japerry committed 41c0d5f on 8.x-3.x authored by Jorrit
    Issue #1863166 by Jorrit: Bad arguments when first optional context...
dsnopek’s picture

The change committed on this issue makes it impossible to have an empty context. Here's the use-case:

  • You have content type which an optional Entity Reference to a parent of some kind
  • You override display of the content type using Panelizer
  • In Panelizer, add a relationship to the entity referred to by the Entity Reference
  • Then you have a View which provides a "Content pane" taking the optional parent as context and contextual filter - it's configured to "Hide view" if the context is missing
  • If the peice of content has the parent, the View should display; if not, then the View should be hidden completely

But what actually happens is: if the optional parent is empty, all possible results are shown!

I propose that:

  1. This patch is reverted
  2. The underlying Views issue is fixed. It could be one of these:
  3. A new patch is written similar to the one committed here, but passing NULL rather than the exception value

For now, reverting this patch is enough to fix my use case, though. :-)

dsnopek’s picture

FileSize
621 bytes

To get things started, here's a patch to revert. Leaving at "Needs work" until the rest of the solution can be worked out.

cboyden’s picture

This patch no longer applies to the latest dev, it looks like the function it touched has been completely rewritten. The bug still occurs.

dsnopek’s picture

Status: Needs work » Needs review
FileSize
713 bytes

This code was moved in #1910608: Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager). I've updated the patch for the new location!

jweowu’s picture

I've encountered issues with the original commit (#6) from this issue, ultimately resulting in a PDO Exception where views_handler_argument_term_node_tid::title_query() attempts to SELECT name FROM taxonomy_term_data WHERE tid = 'all'.

Similarly to #8 I have a view which takes a single argument (which is also optional): "Content: Has taxonomy term ID". The "Exceptions" settings for this argument are the defaults of "Exception value" = 'all', and "Override title" = FALSE.

(I also set "When the filter value IS available or a default is provided" to "Specify validation criteria" of "Taxonomy term", but that part doesn't seem to be causing problems.)

Still like #8 the view display is a "Content pane" rendered by Panels, but in my case the View's pane display does takes its "argument input" from the panel context "Term ID" (with "Context is optional" selected).

The problem then occurs when the particular panel in question does not supply that optional context, as pane_process_conf() was then electing to set the argument value to 'all'. (n.b. I have multiple panel displays using this pane, and one of them does not set this context, in order that "When the filter value is NOT available" Views should "Display all results for the specified field".)

The specific error is then triggered on account of the lack of an Override title value, because Views then calls views_handler_argument_term_node_tid::title_query() and it passes the string 'all' as the argument value, which results in the following query and consequent PDO Exception:

    $result = db_select('taxonomy_term_data', 'td')
      ->fields('td', array('name'))
      ->condition('td.tid', $this->value)
      ->execute();

PDOException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "all"

(And again, 'Override title'=false and 'Exception value'='all' are the default values -- and hidden inside a collapsed fieldset -- so this bug will be triggered by default in these circumstances.)

With the original commit reverted (as per the most recent patch in #14), this error does not occur, and the View output is as expected.

In my case I could alternatively work around the problem by setting 'Override title' to true, as I wasn't actually displaying the title in question, so it didn't matter what it was set to.

cboyden’s picture

See related Views issue #2341249: Do not skip processing all arguments if one "fails". The latest patch there should fix the Views issue such that the revert in #14 isn't needed anymore and can be replaced by the patch I'm attaching here.

cboyden’s picture

Here's a patch to update the one in #14 so it can apply to the latest release 1.14.

dsnopek’s picture

Unfortunately, I think this patch depends on the Views patch on #2341249: Do not skip processing all arguments if one "fails". Without that patch, this patch will change the behavior of Views panes where an optional context that isn't the last in the list is omitted.

There's a couple options:

  1. Decide not to care and commit any way :-)
  2. Wait until Views releases with #2341249, and then make views_content (the CTools module this patch is for) depend on the later Views version in it's views_content.info file
  3. Put some logic in here that some how checks for the presence of the change and alters its behavior. It could look for the method defined there, like method_exists('views_handler_argument', 'skip_processing_arguments_on_fail')?

Do the maintainers have a preferred option?

joelpittet’s picture

@dsnopek I'm leaning to 18.2

Wait until Views releases with #2341249: Do not skip processing all arguments if one "fails", and then make views_content (the CTools module this patch is for) depend on the later Views version in it's views_content.info file