warning: Missing argument 2 for views_node_feed_edit_form() in /xxx/sites/all/modules/views_node_feed/views_node_feed.module on line 90.

Comments

andrewlevine’s picture

Title: error » Missing argument 2 for views_node_feed_edit_form()
Priority: Critical » Normal

Thanks for the report, can you give a little more info about when this happens?

sweetchuck’s picture

In the hook_menu()
$items['admin/settings/views_node_feed/edit']
is MENU_NORMAL_ITEM, and visible in the Navigation block, but the identifier does not passed for the form builder callback function (views_node_feed_edit_form() )
The identifier would be the 4th parts of the URL
'page arguments' => array('views_node_feed_edit_form', 4),
instead of
'page arguments' => array('views_node_feed_edit_form'),

The first argument of a form builder function must be the $form_state

avpaderno’s picture

I have the same problem.
When visiting " Site configuration › Views Node Feed" I get the following error message:

warning: Missing argument 2 for views_node_feed_edit_form() in /Applications/MAMP/htdocs/dr6/sites/all/modules/views_node_feed/views_node_feed.module on line 90.

The page is shown, but all I see in the page is a "the requested page could not be found".

The problem has been already found, and reported here from Sweetchack, but it doesn't seem the code has been fixed, so far.

MiMe’s picture

Try adding a default value to the $ident parameter of the function 'views_node_feed_edit_form': $ident = 'new'
Oh, and I guess that the wildcard in the hook_menu should look something like this:

$items['admin/settings/views_node_feed/edit/%']

aaron.r.carlton’s picture

This menu link doesn't make sense to me. It seems like a previous version of the module may have allowed you to only have one version of a feed and this link was a shortcut to edit that? We could either change the module to provide a sublink for each feed defined, similar to the sub menus for each content type, OR, we could remove the link entirely. Let me know if there's any distinct preference and I'll submit a patch.

andrewlevine’s picture

I believe in the 5 version, this was how you edited each node feed. I think sweetchack has the right idea in #2 but it probably would make more sense for the editing to be more linked to Views UI as I think aaron is suggesting in #5

aaron.r.carlton’s picture

StatusFileSize
new1.91 KB

This patch changes the structure of the menu to remove the edit link. It replaces it with a menu item to add a new feed. I attempted to create a menu link to edit each Node Feed, but I wasn't able to come up with a solution to do this that didn't involve a foreach loop in hook_menu (which is generally not supposed to be done). I also made the decision to forego these links seeing that other popular modules (specifically Views) don't do it either.

Also in the patch is a fix for an issue I noticed where editing an existing feed identifier would create a new feed with the edited identifier and also preserve the feed with the original identifier. Not sure if that's already an issue in the queue, but I'll take a look.

Please provide any suggestions of feedback on implementation or coding standards, as I am a relatively new Drupal contributor and still learning the ropes ;)

aaron.r.carlton’s picture

Status: Active » Needs review
StatusFileSize
new0 bytes

I think I screwed up the diff by executing it from the wrong directories. Please try this one.

aaron.r.carlton’s picture

StatusFileSize
new1.87 KB

Error uploading patch? 0 bytes... new one. Sorry!

andrewlevine’s picture

Status: Needs review » Needs work

Aaron,

#9 is in the right direction...However,

'title' => 'Edit Feed ' . arg(4),

is a SQL injection vulnerability. You will want to do something similar to case 2 here: http://drupal.org/node/140311

It also looks like you are doing something weird with $form['#parameters']...I think you're intending to use $form_state['values']?

aaron.r.carlton’s picture

Status: Needs work » Needs review
StatusFileSize
new2.06 KB

Andrew,

Thanks for the pointers! I've moved the Edit Feed menu item to a MENU_CALLBACK, as its not going to appear in the menu. I've also changed the title to remove the SQL injection by using t() replace per your link.

I agree, it was clunky to use $form['#paramters'], but I am trying to access the original value of the form, not the posted value. I've moved those lines of code to use:

$form['identifer']['#default_value']

is this a better solution or do you have any additional advice? I'll learn this easy stuff quickly, I swear :)

andrewlevine’s picture

Status: Needs review » Needs work

Aaron,

Thanks for following up. A couple things. Where you have !ident in the menu item it should be @ident instead. @ident will escape characters to HTML. An explanation of this can be found in the documentation for t.

I think the cleanest way to do what you're doing with the default value is to save the "original" identifier in the $form array like the drupal 6 example given here and then to access it again in the submit handler. That way it won't be dependent on the logic used to get the #default_value

Thanks again for the work on the patch

aaron.r.carlton’s picture

Status: Needs work » Needs review
StatusFileSize
new2.31 KB

Here is another patch accommodating your suggested changes. Thanks again for the help and let me know if I should change anything else.

andrewlevine’s picture

Status: Needs review » Fixed

Fixed. Many thanks for the patch.

Status: Fixed » Closed (fixed)

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