[D7] Delete quick

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.

Hello chernetsky,

Here's my review of your module:

Issues

• (minor) The project page (https://drupal.org/sandbox/chernetsky/2116515) could have a little bit more of information on it. At the time of writing, the screen shots were great (helped me understand what you meant by contextual links) but a module description and instructions would be brilliant. Some advice I found was at: https://drupal.org/node/997024 which I'll probably use on my project's page!

Basic application checks

• I tested functions like quick deleting nodes from the /node/___/delete_quick link (with different permissions), which worked fine.
• A cursory search for similar modules found "Delete tab" (https://drupal.org/project/delete_tab) and "SN Quick delete" (https://drupal.org/project/quick_field_delete), but I couldn't find a module that deleted a module without confirmation. Maybe you could reference your module with this SO question http://stackoverflow.com/questions/10516417/bypass-node-delete-confirm-f... or http://drupal.stackexchange.com/questions/88855/skip-confirmation-page-o...
• I couldn't find any other applications or duplicate projects.

Repository checks

• Git repo contains code on a 7x-1x branch.
• There is only around 146 lines of code for review - which is very close to the minimum amount of code. I don't know if this is an issue or not, but thought I might raise it.

Security review

• The module doesn't accept text input from users so no need for check_plain(), filter_xss() etc security checks.
• t() functions are used for output.

Licensing checks

• No license.txt file in Git repo.
• Could not find any non-GPL code in repo.

Documentation checks

• See above issue on project page.
• Readme.txt file is succinct and well done.
• Given the short size of this module, there are an appropriate relative amount of in-line documentation

Coding standards

• Visual inspection was OK. Passed PAReview.sh tests.

API and best practices review

• Nothing significant to report.

Hope this helps, and well done!
ajosephau

You're welcome chernetsky :-), the project page is good start as it is (I think it was from your readme.txt file?). That link I attached (https://drupal.org/node/997024) would make your good project page great - hence is a minor comment.

Another minor comment I have from another review I did would be to add some doxygen comments to your functions - the page: https://drupal.org/node/1354 under the heading "Drupal API documentation standards for functions" has some guidance. But this is more to make good comments great - a minor priority feedback comment.

As far as the process of project reviews go, you are the very first project I reviewed! So I'm still very new to how the whole review process goes after you have a few reviews on your code...

Good luck with the review process!
ajosephau

Hi chernetsky,

Well done on the new module's project page :-), my only feedback would be the "pledges" are for some of the more global Drupal initiatives like accessibility and Drupal 8 compatibility. Token (https://drupal.org/project/Token) and Pathauto (https://drupal.org/project/Pathauto) are both good examples of what is meant by pledges. Maybe you could put the information about what you'd like to do in the future of the module in the overview?

With regards to the doxygen comments - my apologies I should be more specific. I was thinking that for some of your functions like ___, they could use descriptions of what the parameters are (here's a direct link to what I was talking about https://drupal.org/node/1354#functions). The specific case was with the function "_delete_quick_access" - I didn't know what the parameters "$op, $node, $account" meant and what data type they needed to be. The doxygen comments would be useful (or referencing the hook you're implementing), but I wouldn't worry if you can't/don't want to do it: it is a very tedious process doing that documentation!

Good luck with the rest of your review and keep up the good work!
ajosephau

Hi, fine module.

manual review:
delete_quick_uninstall(): do not run DB queries directly against the variable table, use explicit variable_del() calls instead.

and a small feature request, the ability to stay on the opened overlay page instead of getting redirected after deletion or the ability to define redirect url based on content type, for better workflow.

Quick response, so will give you a quick one from me to, have checked and can't, find any other critical errors so - you gets green light from me :-)

Good luck with the rest of your review process!

This module has a cross-site request forgery problem. http://drupalscout.com/tags/csrf

It could be exploited with something like

 <img src="http://www.example.com/node/1/delete_quick">

There are some tips on fixing csrf at http://drupalscout.com/tags/csrf

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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