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;
Comment | File | Size | Author |
---|---|---|---|
#17 | ctools-optional-views-arguments-1863166-17-do-not-test.patch | 621 bytes | cboyden |
#16 | ctools-optional-views-arguments-1863166-16.patch | 577 bytes | cboyden |
| |||
#14 | ctools-optional-views-arguments-1863166-14.patch | 713 bytes | dsnopek |
|
Comments
Comment #1
TripleEmcoder CreditAttribution: TripleEmcoder commentedI 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:
Comment #2
Jorrit CreditAttribution: Jorrit commentedI 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.Comment #3
fagoI 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.
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.
Comment #4
codycraven CreditAttribution: codycraven commentedI 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).
Comment #5
Jorrit CreditAttribution: Jorrit commentedI 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 inview->_build_arguments()
, the loopforeach ($this->argument as $id => $arg) {
is ended with a break (around line 865). Replacing that with acontinue;
will allow NULLs.[edit]
I found a related Views issue: #1250336: Any argument after a missing one is silently ignored (D7).
Comment #6
japerryTested, fixed, and committed:
http://drupalcode.org/project/ctools.git/commit/41c0d5f9755d9857024ffbef...
Comment #8
lex0r CreditAttribution: lex0r commentedI'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
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.
Comment #11
dsnopekThe change committed on this issue makes it impossible to have an empty context. Here's the use-case:
But what actually happens is: if the optional parent is empty, all possible results are shown!
I propose that:
For now, reverting this patch is enough to fix my use case, though. :-)
Comment #12
dsnopekTo get things started, here's a patch to revert. Leaving at "Needs work" until the rest of the solution can be worked out.
Comment #13
cboyden CreditAttribution: cboyden commentedThis patch no longer applies to the latest dev, it looks like the function it touched has been completely rewritten. The bug still occurs.
Comment #14
dsnopekThis 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!
Comment #15
jweowu CreditAttribution: jweowu commentedI'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 toSELECT 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:(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.
Comment #16
cboyden CreditAttribution: cboyden commentedSee 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.
Comment #17
cboyden CreditAttribution: cboyden commentedHere's a patch to update the one in #14 so it can apply to the latest release 1.14.
Comment #18
dsnopekUnfortunately, 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:
method_exists('views_handler_argument', 'skip_processing_arguments_on_fail')
?Do the maintainers have a preferred option?
Comment #19
joelpittet@dsnopek I'm leaning to 18.2