[D7] Multiple Fields Remove Button

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxasghar2028623git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

The PAReview tool identified a few issues which need addressing:

FILE: ...ww/drupal-7-pareview/pareview_temp/multiple_fields_remove_button.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 4 WARNING(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
136 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
143 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
173 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
184 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
--------------------------------------------------------------------------------

URL: http://ventral.org/pareview/httpgitdrupalorgsandboxasghar2028623git

Please check/fix these issues before changing the status back to "needs review"

Master Branch: It appears you are working in the "master" branch in git. You should really be working in a version specific branch. See: http://drupal.org/node/1127732

There is a very similar module called Field Remove Item. At a glance, it seems like the main difference is that your module provides a button to remove while Field Remove Item provides a checkbox. Once the checkbox is checked, then the field is removed via an Ajax Callback.

Are there more ways in which your module differs from this one?

That could be a good argument for differentiation. Can you write up a better explanation of how this differs from the module mentioned?

It seems like FRI might be languishing a bit in that it hasn't been updated in over a year, and there are, what seem to me to be, some pretty major issues in its issue queue.

Does your module play nicely with other commonly used modules (the FRI especially has problems with) like Field Collection and Hierarchical Select? FRI seems to also have some problems with commonly employed fields like Link, File Field, and Image Field (I'm getting all of these from its issue queue). And there is a report that it breaks things like Autocomplete Widgets.

A few places about the module review process (How to Review..., Apply for Permission..., and the Project Application Checklist) explain more about writing up how your module avoids duplication if it is similar.

Hi Asghar,
As a great co-worker that is a great effort of your to put out such an awesome module. Ok here are reviews on fresh install
1- I enabled the module and go to article content type i did not add any unlimited text field yet i got some notices.

Notice: Undefined index: #field_parents in multiple_fields_remove_button_field_widget_form_alter() (line 73 of /var/www/moduletesting/sites/all/modules/multiple_fields_remove_button/multiple_fields_remove_button.module).
Notice: Undefined index: #field_name in multiple_fields_remove_button_field_widget_form_alter() (line 74 of /var/www/moduletesting/sites/all/modules/multiple_fields_remove_button/multiple_fields_remove_button.module).
Notice: Undefined index: #language in multiple_fields_remove_button_field_widget_form_alter() (line 75 of /var/www/moduletesting/sites/all/modules/multiple_fields_remove_button/multiple_fields_remove_button.module).
Notice: Undefined index: #delta in multiple_fields_remove_button_field_widget_form_alter() (line 80 of /var/www/moduletesting/sites/all/modules/multiple_fields_remove_button/multiple_fields_remove_button.module).
Warning: array_merge(): Argument #1 is not an array in multiple_fields_remove_button_field_widget_form_alter() (line 80 of /var/www/moduletesting/sites/all/modules/multiple_fields_remove_button/multiple_fields_remove_button.module).

screenshot attached.
2- Then i add an unlimited text value on article every time if i add another text value i again get these same notices
3- With views module you module is working perfectly if add or remove text values it does effect the views results so that is cool.
4-Is there is any difference with this module https://drupal.org/project/cck_multiple_field_remove?

Hi,

I still get same notices and warnings as in #12.
Besides, I have couple of comments regarding hook_menu implementation

  $items['multiple_fields_remove_button/ajax'] = array(
    'page callback' => 'multiple_fields_remove_button_js',
    'delivery callback' => 'ajax_deliver',
    'access callback' => TRUE,
    'theme callback' => 'ajax_base_page_theme',
    'type' => MENU_CALLBACK,
    'file path' => 'includes',
    'file' => 'form.inc',
  );

1. 'access callback' is set to TRUE, which I guess is not very secure.
2. file and file path seem to be unnecessary here, since form.inc would be included anyway. Any by the way, removing them makes
notices above disappear. Instead, after clicking on Remove I got another notice.
Notice: Undefined index: entity in multiple_fields_remove_button_submit_handler() (line 159 of multiple_fields_remove_button.module).

Hi asghar,
After thinking about it a bit more and not being able to come up with any appropriate permission I checked the 'add_more' button processing and eventually ended up in system.module implementation of hook_menu(). I guess the system/ajax menu item was the inspiration for this part and indeed it's access callback is set to TRUE.
So it kinda seems alright to reuse what's good enough for core. But on the other hand, 'system/ajax' is not intended to be used directly. It's the entry point for handling all Ajax requests coming from '#ajax' Form API property.
So my suggestion would be to remove multiple_fields_remove_button_menu(), and use 'callback' => 'multiple_fields_remove_button_js' instead of 'path' => 'multiple_fields_remove_button/ajax' on $element['remove_button']['#ajax'] (around line 96). This is more appropriate in my opinion, as you will get cleaner code. Not only it eliminates the need for additional hook_menu and menu_item, but also multiple_fields_remove_button_js() will receive $form and $form_state as arguments, thus making first lines of it, i.e.

  if (isset($_POST['ajax_html_ids'])) {
    unset($_POST['ajax_html_ids']);
  }

  list($form, $form_state) = ajax_get_form();
  drupal_process_form($form['#form_id'], $form, $form_state);

unnecessary as well.
Hope you find this comment useful and good luck.

Looks good, can't find anything wrong :D

Changed to "reviewed & tested by the community"

I'll look at this now in the Project applications sprint

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732

manual review:

1. project page is too short, see https://drupal.org/node/997024
2. This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.
3. Since the code seems to be copied from field_collection module http://drupalcode.org/project/field_collection.git/commitdiff/a0dc28f?hp... we cannot approve you as git vetted user either for the number of lines of code.

But otherwise looks good, so ...

Thanks for your contribution, asghar!

I promoted this project for you: https://drupal.org/project/multiple_fields_remove_button

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.