This is a utility module intended to include a collection of scripts useful for batch processing contents on a Drupal site.

Project Link: http://drupal.org/sandbox/kenyangzj/1904474

git clone --recursive --branch master kenyangzj@git.drupal.org:sandbox/kenyangzj/1904474.git quick_fix

This is a Drupal 7 module.

Comments

ycshen’s picture

Status: Needs review » Needs work

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

  • Remove "version" from the ./quick_fix.info file, it will be added by drupal.org packaging automatically.
  • ./includes/quick_fix_forms.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
    function quickfix_landing() {
    function quickfix_landing_submit($form, &$form_state) {
    
  • ./includes/helpers.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
    function _update_node_title($nid = 0, $new_title = '') {
    
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.


FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
  3 | WARNING | Line exceeds 80 characters; contains 123 characters
 11 | WARNING | Line exceeds 80 characters; contains 129 characters
 11 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/includes/helpers.inc
--------------------------------------------------------------------------------
FOUND 12 ERROR(S) AFFECTING 8 LINE(S)
--------------------------------------------------------------------------------
  2 | ERROR | Missing file doc comment
  3 | ERROR | Function comment short description must end with a full stop
  6 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  6 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  7 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  7 | ERROR | Line indented incorrectly; expected 4 spaces, found 2
  8 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  8 | ERROR | Line indented incorrectly; expected 4 spaces, found 2
  9 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  9 | ERROR | Line indented incorrectly; expected 4 spaces, found 2
 10 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 11 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/includes/quick_fix_forms.inc
--------------------------------------------------------------------------------
FOUND 92 ERROR(S) AFFECTING 32 LINE(S)
--------------------------------------------------------------------------------
  2 | ERROR | Missing file doc comment
  2 | ERROR | Missing function doc comment
  3 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  3 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  4 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  4 | ERROR | Whitespace found at end of line
  5 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  5 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  5 | ERROR | Expected 1 space between "'#type'" and double arrow; 0 found
  5 | ERROR | Expected 1 space between double arrow and "'fieldset'"; 0 found
  5 | ERROR | Expected 1 space before "=>"; 0 found
  5 | ERROR | Expected 1 space after "=>"; 0 found
  5 | ERROR | Expected 1 space between "'#title'" and double arrow; 0 found
  5 | ERROR | Expected 1 space between double arrow and "t"; 0 found
  5 | ERROR | Expected 1 space before "=>"; 0 found
  5 | ERROR | Expected 1 space after "=>"; 0 found
  6 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  6 | ERROR | Whitespace found at end of line
  7 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  7 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  8 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  8 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  9 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  9 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
  9 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 10 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 10 | ERROR | Line indented incorrectly; expected 4 spaces, found 2
 11 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 12 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 12 | ERROR | Whitespace found at end of line
 13 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 13 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 13 | ERROR | If the line declaring an array spans longer than 80 characters,
    |       | each element should be broken into its own line
 13 | ERROR | Expected 1 space between "'#type'" and double arrow; 0 found
 13 | ERROR | Expected 1 space between double arrow and "'checkboxes'"; 0 found
 13 | ERROR | Expected 1 space before "=>"; 0 found
 13 | ERROR | Expected 1 space after "=>"; 0 found
 13 | ERROR | Expected 1 space between "'#title'" and double arrow; 0 found
 13 | ERROR | Expected 1 space between double arrow and "'Content Type'"; 0
    |       | found
 13 | ERROR | Expected 1 space before "=>"; 0 found
 13 | ERROR | Expected 1 space after "=>"; 0 found
 13 | ERROR | Expected 1 space between "'#options'" and double arrow; 0 found
 13 | ERROR | Expected 1 space before "=>"; 0 found
 14 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 14 | ERROR | Whitespace found at end of line
 15 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 15 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 15 | ERROR | Expected 1 space between "'#type'" and double arrow; 0 found
 15 | ERROR | Expected 1 space between double arrow and "'submit'"; 0 found
 15 | ERROR | Expected 1 space before "=>"; 0 found
 15 | ERROR | Expected 1 space after "=>"; 0 found
 15 | ERROR | Expected 1 space between "'#value'" and double arrow; 0 found
 15 | ERROR | Expected 1 space between double arrow and "'Decode title HTML
    |       | entities'"; 0 found
 15 | ERROR | Expected 1 space before "=>"; 0 found
 15 | ERROR | Expected 1 space after "=>"; 0 found
 16 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 16 | ERROR | Whitespace found at end of line
 16 | ERROR | Functions must not contain multiple empty lines in a row; found 2
    |       | empty lines
 17 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 17 | ERROR | Whitespace found at end of line
 18 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 18 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 21 | ERROR | Missing function doc comment
 22 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 22 | ERROR | Whitespace found at end of line
 23 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 23 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 24 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 24 | ERROR | Whitespace found at end of line
 25 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 25 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 25 | ERROR | Concat operator must be surrounded by spaces
 25 | ERROR | Concat operator must be surrounded by spaces
 26 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 26 | ERROR | Whitespace found at end of line
 27 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 27 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 28 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 28 | ERROR | Line indented incorrectly; expected 4 spaces, found 2
 28 | ERROR | If the line declaring an array spans longer than 80 characters,
    |       | each element should be broken into its own line
 28 | ERROR | If the line declaring an array spans longer than 80 characters,
    |       | each element should be broken into its own line
 29 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 30 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 30 | ERROR | Whitespace found at end of line
 31 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 31 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 32 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 33 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 33 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
 34 | ERROR | Spaces must be used to indent lines; tabs are not allowed
 34 | ERROR | Whitespace found at end of line
 35 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/quick_fix.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 4 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/quick_fix.module
--------------------------------------------------------------------------------
FOUND 24 ERROR(S) AND 1 WARNING(S) AFFECTING 15 LINE(S)
--------------------------------------------------------------------------------
  2 | ERROR   | Missing file doc comment
  5 | ERROR   | Function comment short description must start with a capital
    |         | letter
  5 | ERROR   | Function comment short description must end with a full stop
  8 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
  8 | ERROR   | Line indented incorrectly; expected 2 spaces, found 1
  9 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
  9 | ERROR   | Whitespace found at end of line
 10 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 10 | ERROR   | Line indented incorrectly; expected 2 spaces, found 1
 11 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 11 | ERROR   | Do not use t() in hook_menu()
 12 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 12 | ERROR   | Do not use t() in hook_menu()
 13 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 14 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 15 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 16 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 16 | WARNING | A comma should follow the last multiline array item. Found:
    |         | 'includes/quick_fix_forms.inc'
 17 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 17 | ERROR   | Line indented incorrectly; expected 2 spaces, found 1
 18 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 18 | ERROR   | Whitespace found at end of line
 19 | ERROR   | Spaces must be used to indent lines; tabs are not allowed
 19 | ERROR   | Line indented incorrectly; expected 2 spaces, found 1
 20 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

kenyangzj’s picture

Status: Needs work » Needs review

Switched to branch 7.x-dev and deleted master branch.

Fix some coding styles (spaces, tabs ...)

strakkie’s picture

Status: Needs review » Needs work

Hi kenyangzj,

The new git branch isn't correct, you could work in 7.x-1.x for example, not in 7.x-dev.

It appears you are working in the "7.x-dev" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the 7.x-dev branch:

Also see,
http://ventral.org/pareview/httpgitdrupalorgsandboxkenyangzj1904474git

A lot of errors are shown here because you are using tabs in your code, replace these with spaces to remove most of the shown errors. The other errors shown here are mostly formatting errors, try to find these errors on the lines provides by the webpage.

A lot of the conventions for coding standards can be found here:
http://drupal.org/coding-standards

strakkie’s picture

Had some time to look into the code, you should have a look at the following:

When there are no nodes in the selected content types, the following warnings show:
An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /review/batch?render=overlay&id=3&op=do StatusText: OK ResponseText: Fatal error: Call to a member function claimItem() on a non-object in C:\xampp\htdocs\review\includes\batch.inc on line 274

And after going back:
Notice: Undefined variable: batch_ops in quick_fix_landing_submit() (line 59 of C:\xampp\htdocs\review\sites\all\modules\review\quick_fix\includes\quick_fix_forms.inc).

This is because the batch_set function is always called in quick_fix_forms.inc (line 58).

By the way, your module description says:
A utility module that includes a collection of scripts useful for batch processing contents on a Drupal site.

I can only find the "Node title - HTML entity decode" script. Are you planning on creating extra functions for your module?

kenyangzj’s picture

Status: Needs work » Needs review

Coding style now passed the automated check.

Addressed issues mentioned in #4

@strakkie You are correct, currently only Node title - HTML entity decode is available. Will plan to include more functions. Suggestions are always welcome.

a_thakur’s picture

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page.

This project is too short to approve you as git vetted user. This discussion elaborates, 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.

adnasa’s picture

quick_fix_forms.inc

$query = db_query("SELECT nid, title FROM node WHERE type IN ('" . implode("', '", $content_types) . "')");

Did this statement really worked? o_O

Here's a suggestion (and recommended):

$query = db_query("SELECT nid, title FROM {node} WHERE type IN (:content_types)", array(':content_types' => $content_types));

Best of luck!
/ adnasa

AolPlugin’s picture

Status: Needs review » Needs work

Pretty useful module, like the idea!

A few issues, though:

1. If you select a content type which has no nodes in it, nothing is done. A warning should appear.
2. In includes/quick_fix_forms.inc on line 44, it's better to either use the code in #7 or db_select().
3. in quick_fix.module on line 8, it's best to use module_load_include

kenyangzj’s picture

Status: Needs work » Needs review

Thanks both #7 and #8. I have included the changes into the 7.x-dev branch.

@AolPlugin just curious, why module_load_include is preferred compared to require_once?

By the way, currently this module has only one functionality to html decode node titles and I'm look for suggestions/ideas on what to add. Feel free to suggest :)

AolPlugin’s picture

module_load_include is nothing more but a small utility function from Drupal, but it might come in handy sometimes (as per http://stackoverflow.com/questions/7270729/module-load-include-vs-require-once).

If you look over the source code (here), you will see that it includes the full path to the file, and checks for it's existence. If it fails it returns FALSE, which you can then handle. In your case, you know that it exists, but I was suggesting it for future references.

sreynen’s picture

Title: Quick Fix » [D7] Quick Fix
seworthi’s picture

Assigned: Unassigned » seworthi
seworthi’s picture

Assigned: seworthi » Unassigned
Status: Needs review » Needs work

This is a review of the 7.x-1.x

The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.
http://ventral.org/pareview/httpgitdrupalorgsandboxkenyangzj1904474git

This is a security blocker. Other than this issue, the rest of the module is good.

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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