[D7] Translate Nodes

Coding style
The automated test found quite a few issues that should be addressed.

http://pareview.sh/pareview/httpgitdrupalorgsandboxfermicoding2322931git

The code is quite long, you may want to consider breaking it apart and using a .inc file for for your different pages.

For example I will normally make something like translate_nodes.admin.inc and put all the functions related to the admin pages in there. It will help with readability and reduce the amount of code that has to be loaded during the boot strap process.
Documentation
I would include some more in-line comments. I had a very hard time following your code. Other then that the documentation looks pretty good.
Git repository
Follows the standards for version based branch names.
Bugs
I enabled the modules and tried to create some new content and came across this error:

PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'DEFAULT' for column 'id' at row 1: INSERT INTO {translate_nodes_status} (id, nid, status, uid) VALUES (:default, :nid, :status, :default); Array ( [:default] => DEFAULT [nid] => 2 [status] => 4 ) in translate_nodes_node_insert() (line 213 of /home/beeker/public_html/heislerweb/sites/dev/modules/translate_nodes/translate_nodes.module).

Automated Review

Code needs some work:
http://pareview.sh/pareview/httpgitdrupalorgsandboxfermicoding2322931git

Manual Review

Individual user account

Yes

No duplication

Yes?

https://www.drupal.org/project/gtranslate

Master Branch

Yes

Licensing

No

3rd party code

No

README.txt/README.md

Yes

Code long/complex enough for review

Yes

Secure code

Yes

Coding style & Drupal API usage

Minor finding: Need to break up your code, your form elemenst should be in a .inc on an admin file of sorts

*Major finding: When I enable the module, I get a PDOException error

I would suggest reading DB API:

https://www.drupal.org/developing/api/database

https://api.drupal.org/api/drupal/includes%21database%21database.inc/gro...

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

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.

Hi Thanks for your contribution,

I found something in sql query which can be improved by using PDO functionality
> file translate_nodes.admin.inc

$res = db_query("SELECT COUNT(DISTINCT n1.tnid) c
                     FROM {node} n1
                     JOIN {node} n2 ON n1.tnid = n2.tnid
                     WHERE n1.tnid > 0 AND n2.language = :lang AND (n1.tnid != n1.nid OR n1.language != :lang)",
        array(':lang' => $lang))->fetchField();

> file translate_nodes.module

$res = db_query("SELECT language FROM {node} WHERE tnid = :tnid",
          array(':tnid' => $node->tnid));

I would suggest to use PDO to access database because it is a consistent way to access the database.
https://api.drupal.org/api/drupal/includes!database!database.inc/group/database/7

Hi and thanks for your contribution.

As stated in the project description this module aims at making translation workflows easier by enabling users to divide workload and by using Google Translate. In what aspects does this module differ from the Translation Management Tool (https://www.drupal.org/project/tmgmt) that we already have?

@fermicoding did you discuss the desire to see these features in tmgmt with that projects maintainers?

He can have a clear overview of what is happening, including all the states of all ongoing translations as well as the abbility to (re)assign translators as needed, all in one page, in a table.

This seems like a worthwhile UI improvement that could have been submitted to tmgmt or even provides as a module that provides an alternative UI.

especially because the translator list is role based, so one admin can setup everyone

I'm not following this at all. Both modules define permissions that are assign to roles...

http://cgit.drupalcode.org/sandbox-fermicoding-2322931/tree/translate_no...
http://cgit.drupalcode.org/tmgmt/tree/tmgmt.module#n216

I'm not seeing anything in this module that makes managing the translators any easier than tmgmt.

Well, i think you need to add "i18n" module to your dependencies list, as you are using the function "i18n_language_list" in file translate_nodes.module, line: 285.

Thanks.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

@fermicoding I am not able to install your module due to dependency i18n_language_list and also I cant find this module anywhere. I have Internationalization( i18n) module installed. Please add help or any link to dependency.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

@fermicoding still too many warnings and errors please do some important changes listed below.

1. Closing parenthesis of array declaration must be on a new line.
2. Array indentation error, expected 6 spaces but found 8.
3. Object operator not indented correctly; expected 6 spaces but found 10.
4. Inline comments must end in full-stops, exclamation marks, colons, question marks, or closing parentheses.
5. Expected 1 blank line after function.
6. Line must not exceeds 80 characters.

Thanks
Yogesh

1. Please use a personal user account instead of an organization account.
2. See https://pareview.sh/node/2672 for a list of errors and warnings that should be addressed regarding code style.
3. The unit test is not working. It uses the assertText function that is not present in DrupalUnitTestCase. It also seems to be dependant on an external service. You should try using a mockup service for testing.
4. admin/config/regional/translate-nodes/overview has not access check or page callback.
5. As mentioned in #5, use a dynamic query using addTag('node_access') to make sure translators can only see nodes they have access to.

Other than that, as mentioned above, it duplicates functionality from other modules. It should be better to use the functionality from e.g. gtranslate instead of reimplementing it here.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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