Problem/Motivation

Suppose I use views where I set contextual filter to filter by NID and I use Provide default value - Content ID from URL. This is perfectly working on node view or node edit pages but in case the view is being rendered for ajax callback where the ajax callback is called from node/edit page no NID value is provided. This is due to a fact that the url of such a callback is system/ajax and Provide default value - Content ID from URL is trying to get node with menu_get_object from url (implemented in views_plugin_argument_default_node->get_argument()).

One of concrete cases where this issue is happening is when I add Entity Reference field to the node then set number of values to unlimited and use ENTITY SELECTION
mode: Views: Filter by an entity reference view. Then every time when "Add another item" is pressed the view display return 0 results because Content ID from URL does not provide NID value. The issue is also documented on entity reference module here: #1823506: Ajax Calls Generated for Entity Reference Views cause issues

I made a bit of research because I was not sure if this is the problem of views module or if it is a problem of entity reference module or even the drupal ajax. Issue number 1 is that the system/ajax callback does not provide any data about node when is called from node/edit form. Only way to get the node info is from Header data for ajax callback from referer value. With the current implementation of views and ajax I think this issue should be addressed in this module.

Proposed resolution

I am attaching patch that alters get_argument() method of views_plugin_argument_default_node class where I added functionality to retrieve node NID value from referer value from header of ajax callback.

Files: 
CommentFileSizeAuthor
#32 views-ajax-content-id-issue-2358049-32.patch5.3 KBgauravmanerkar
#31 views-ajax_default_argument_paths_strip_lang-2358049-30-7.patch5.55 KBjesse.voogt
#28 views_ajax_default_argument_paths-2358049-28.patch5.37 KBclemens.tolboom
None View
#23 views_ajax_default_argument_paths-2358049-23.patch5.36 KBJvE
None View
#5 views-ajax_node_id_issue-2358049-5.patch864 bytesladybug_3777
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: tests were executed, but no results were found. View
#4 views-2358049-4.patch992 bytesbarancekk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-4.patch. Unable to apply patch. See the log in the details link for more information. View
#3 views-2358049-3.patch998 bytesbarancekk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-3.patch. Unable to apply patch. See the log in the details link for more information. View
#1 views-2358049-1.patch1008 bytesbarancekk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-1.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

barancekk’s picture

FileSize
1008 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-1.patch. Unable to apply patch. See the log in the details link for more information. View

Patch attached. Thank you

Jody Lynn’s picture

Component: Code » node data
Status: Active » Needs work

I think you should add your new condition as the last condition in the function as it's sort of a worst case scenario.

Also you have an extra line break between the condition and its statement.

barancekk’s picture

FileSize
998 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-3.patch. Unable to apply patch. See the log in the details link for more information. View

@Jody

You are right I created new patch that adds the new functionality to the end of the method. But it looks like that to code ... if (arg(0) == 'node' && is_numeric(arg(1))) { .... is doing exactly the same thing as ... $node = menu_get_object('node', $i); ... before in method but maybe I am wrong.

Thank you

barancekk’s picture

FileSize
992 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch views-2358049-4.patch. Unable to apply patch. See the log in the details link for more information. View

This one patch is with fixed line break formatting.

Thanks

ladybug_3777’s picture

FileSize
864 bytes
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: tests were executed, but no results were found. View

The last patch does not contain the correct directory structure. It assumes that views is installed in /sites/all/modules/contrib/views/modules/node/views_plugin_argument_default_node.inc

I have updated the patch to use the proper structure so it can be saved and applied directly from the veiws main module folder.

See attached. (NOTE: I have not yet tested the patch code you wrote yet, I wanted to fix the path problem first)

ladybug_3777’s picture

Status: Needs work » Needs review

I've applied the patch and it does resolve the error. We can really use some regression testing to see if this negatively impacts any other functionality though. I'm going to keep it installed on my dev server and will report back if I come across new issues due to the patch.

Updating status to "Needs review"

The last submitted patch, 1: views-2358049-1.patch, failed testing.

The last submitted patch, 3: views-2358049-3.patch, failed testing.

The last submitted patch, 4: views-2358049-4.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: views-ajax_node_id_issue-2358049-5.patch, failed testing.

ladybug_3777’s picture

Version: 7.x-3.8 » 7.x-3.x-dev

Status: Needs work » Needs review
ladybug_3777’s picture

Status: Needs review » Needs work
ladybug_3777’s picture

Status: Needs work » Needs review
ladybug_3777’s picture

How can I get my .patch fle re-tested? It's been sitting with "Test Request Sent" status for a couple of weeks now..... I've been using the patch on my development server for a bit and so far I haven't seen any apparent issues.

ladybug_3777’s picture

If anyone has input on this patch feel free to let me know. THANKS!

ladybug_3777’s picture

Any input on this patch yet? I keep having to apply every time a new stable release comes out.

Jody Lynn’s picture

@ladybug_3777 Use https://github.com/davereid/drush-patchfile and you won't have to manually re-apply patches when you do updates. Then you can just use patches and never worry about them.

ladybug_3777’s picture

Oh cool! Thanks Jody! I will check that out for sure.

JvE’s picture

Status: Needs review » Needs work

You want to use menu_get_item() here in case of path aliases.
And check the current url is system/ajax so this doesn't trigger unexpectedly.

JvE’s picture

Component: node data » Code
Status: Needs work » Needs review
FileSize
4.8 KB
None View

I've created an alternate patch.

Works for nodes, users and taxonomy terms and does not trigger unexpectedly.

JvE’s picture

FileSize
5.31 KB
None View

Updated patch to include non-jQuery ajax callbacks such as ajax file uploads.

JvE’s picture

FileSize
5.36 KB
None View

Fixing missing underscore and adding empty check

ladybug_3777’s picture

@Jody Lynn - I have to tell you, I finally had some time to play around with that Drush Patch File projects you suggested and it is SOOO cool! It is going to save me a bunch of time in the future! My maintenance nightmare is much more manageable now! THANK YOU!

NWOM’s picture

This saved my day. Using the Relation Select module, it opens a view in a Subform via ajax in order to assign relations. However, without this patch, I wasn't able to use contextual views filters. Awesome work and thanks!

clemens.tolboom’s picture

Issue summary: View changes
clemens.tolboom’s picture

clemens.tolboom’s picture

Reviewing #23 it works nice within the restrictions of HTTP_REFERER

I think we should use similar code as modules/file/file.module:249

  list($form, $form_state, $form_id, $form_build_id, $commands) = ajax_get_form();

which contains the node/term etc in $form_state.

+++ b/plugins/views_plugin_argument_default.inc
@@ -87,6 +87,48 @@ class views_plugin_argument_default extends views_plugin {
+      $path = str_replace($GLOBALS['base_url'] . '/', '', $_SERVER['HTTP_REFERER']);

This should use $base_url. Attached patch fixes this.

jproctor’s picture

#28 works for me, where my single-value entity reference selector was getting mangled by a multi-value field collection elsewhere in the node form.

jesse.voogt’s picture

My issue: I have an entity reference field referring to a view with contextual filter of the node id, which defaults to the url. Whenever I uploaded an image to another field on the page, I was getting "An illegal choice has been detected" for the entity reference fields that were referencing the view with the contextual filter when I then tried to save the form. I narrowed the issue down to the fact that the node id was of course not present in the URL of the ajax request, so the view was returning nothing and then invalidating the selected values in my entity reference field.

My resolution:
#28 almost worked for me, except I was using language prefixes in the url because this is a multi-language site. I figured out that current_url(), used for regular pages to set the $path variable in views_plugin_argument_default.inc, was returning the path without the language prefix, but patch #28 was not stripping this. To get the urls acting in a similar fashion I modified the patch by calling language_url_split_prefix.

jesse.voogt’s picture

Here's a modified version of #28 that solves the language issue for me.

gauravmanerkar’s picture

Assigned: Unassigned » gauravmanerkar
Priority: Normal » Critical
FileSize
5.3 KB

My patch works perfectly

clemens.tolboom’s picture

I should have added a diff between #23 and #28 which is

< +      $path = str_replace($GLOBALS['base_url'] . '/', '', $_SERVER['HTTP_REFERER']);
---
> +      $path = str_replace($base_url . '/', '', $_SERVER['HTTP_REFERER']);

#28 and #31

diff views_ajax_default_argument_paths-2358049-28.patch views-ajax_default_argument_paths_strip_lang-2358049-30-7.patch

< +    global $base_url;
---
> +    global $base_url, $language;
133a134,139
> +      if ($language){
> +        $r = language_url_split_prefix($path, array($language));
> +        if ($r && $r[0] && $r[1]) {
> +          $path = $r[1];
> +        }
> +      }

#31 and #32 seems to have removed (introduced?) whitespace and removed documentation.

@gauravmanerkar please check and add an interdiff #31 and your new patch.

I agree with critical as it is disruptive. Note there are more modules breaking on ajax. I listed a few on http://build2be.com/content/ajax-nukes-reference-based-form-fields ; I advised my users not to use the upload button but just save the node on each new file :(

clemens.tolboom’s picture

Status: Needs review » Needs work
NWOM’s picture

I found a bug in the patch when adding a Views Pane to a Panel Page. The view has a default argument as a contextual filter provided by the Views Contextual Filter Query module. I don't think it's related to the module, but instead a problem when adding a view with the default value contextual filter in general, but I figured I'd provide the module name just to be safe.

The following error is displayed when attempting to add the pane:

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /panels/ajax/editor/add-pane/panel_context%3Apage-organizations%3A%3Apage_organizations__panel%3A%3A%3A%3A/content_top/views_panes/organization_view-panel_pane_2
StatusText: OK
ResponseText: 
Fatal error:  Call to undefined function language_url_split_prefix() in /var/aegir/platforms/panopoly-7.x-1.32/profiles/panopoly/modules/contrib/views/plugins/views_plugin_argument_default.inc on line 105

Removing the patch fixes the issue, and allows the views pane to be added without problems.

mobilemelody’s picture

I thought I was having a similar issue, but the patch didn't seem to resolve it. Is this patch is specific to node IDs or should work for things like term IDs as well? I have a node reference dropdown field that is populated using a reference view with a taxonomy term ID as the contextual filter. It's able to grab the correct term ID from the URL when the page initially loads, but after an AJAX request defaults to showing all nodes (which is what I've set it to do under the validation criteria). Any insight would be greatly appreciated

dtamajon’s picture

I have tested #32, and got the error "call to undefined function language_url_split_prefix"

If I add the require_once in get_path(), then it works properly:

require_once DRUPAL_ROOT . '/includes/language.inc';

Not sure it this should be added to the patch or if any other additional check should be done.

caspervoogt’s picture

Just to say that the patch from #32 worked for me and I have not run into "call to undefined function language_url_split_prefix" yet.