The Zoho Campaigns plugin for Drupal is an exclusive integration to create, import, and manage
sign-up forms, allowing subscribers to receive newsletters. Here, Cron - the session traffic
manager perfectly fits into pages and blocks so that it maintains the streamlined flow. Also,
additional fields and design patterns contribute to the ease of sign-up form creation.

Project Page -> https://www.drupal.org/sandbox/zohocampaigns/2545074
Clone Command -> git clone --branch 7.x-1.x https://git.drupal.org/sandbox/ZohoCampaigns/2545074.git

CommentFileSizeAuthor
#4 coder.txt456.16 KBjlicht

Comments

ZohoCampaigns created an issue. See original summary.

PA robot’s picture

Status: Needs review » Needs work

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

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.

ZohoCampaigns’s picture

Status: Needs work » Needs review

Issue fixed

jlicht’s picture

Status: Needs review » Needs work
StatusFileSize
new456.16 KB

Manual Review

Individual user account
No: Does not follow the guidelines for individual user accounts.

It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please note that organization accounts cannot be approved for git commit access. See https://drupal.org/node/1966218 and https://drupal.org/node/1863498 for details on what is/isn't allowed. Please update your user profile so that we don't have to assume that this is a group account.

No duplication
Yes: Does not cause module duplication and/or fragmentation.

Master Branch
Yes: Follows the guidelines for master branch.

Licensing
I’m not sure if it follows the licensing requirements (see note re: 3rd party assets below)

3rd party assets/code
No: Does not follow the guidelines for 3rd party assets/code.

[*] It looks like javascript files have been copied from Drupal core (tabledrag.js) and the slider field module (alter1.js) and included in this project
[*] The jquery_nicescroll module includes the jQuery.nicescroll.js javascript file which has been included in this module. Why not depend on that module instead?

README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.

- Please take a moment to make your README.txt follow the guidelines for in-project documentation.
- List all required dependencies (e.g. modules that are listed as dependencies in the .info file).
- Note that CURL is required
- There is no need to provide instructions for installing Drupal or generic instructions for installing the module - you can link to standard Drupal documentation and just provide information about installation steps specific to your module

Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.

Secure code
Difficult to review this until other issues addressed.

Coding style & Drupal API usage
General
- There are many issues reported by the automated review (see attached file)
- There is a great deal of CSS embedded in the PHP code. This should be pulled out into CSS files.
- Use descriptive variable names, and include some comments in the code.
- The CSS in the code, and lack of consistent formatting and indenting make this code very difficult to review (as well as the lack of comments).
- Consider splitting the .admin.inc into separate files (it is over 5500 lines of code)

ZohoCampaigns.admin.inc
- Do not assume the module will be installed in a specific location when constructing image paths (example: L3428)

ZohoCampaigns.install
[+] Namespace your variable names and table names by prefixing with the module name to avoid collisions with other modules.

ZohoCampaigns.module
[+] Do not set the global error_reporting value (L4)
[+] Use {table} syntax when running database queries (example: L856)

PA robot’s picture

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

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.

ZohoCampaigns’s picture

Issues have been fixed.
We are modifying the CSS property dynamically , hence it is not possible to pull it out to a seperate CSS file.

ZohoCampaigns’s picture

Status: Closed (won't fix) » Needs review
warped’s picture

Assigned: ZohoCampaigns » Unassigned

Please do not assign this issue to yourself. Then it looks like changes are in progress.

avpaderno’s picture

Priority: Normal » Critical

To the reviewers: Please set the priority to Normal after reviewing the project.

sleitner’s picture

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

Automated Review

Pareview details: https://pareview.sh/pareview/https-git.drupal.org-sandbox-zohocampaigns-...

Review of the 7.x-1.x branch (commit 5fa0439):

  • Your README.txt does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
    • The INTRODUCTION section is missing.
    • The REQUIREMENTS section is missing.
    • The INSTALLATION section is missing.
    • The CONFIGURATION section is missing.
  • ./zohocampaigns.admin.inc: All functions should be prefixed with your module/theme name (zohocampaigns) to avoid name clashes. See https://www.drupal.org/node/318#naming
    function back_to_edit($form, &$form_state) {
    function dont_delete($form, &$form_state) {
    function create_new_form($form, &$form_state) {
    
  • The zohocampaigns.module does not implement hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
  • ESLint has found some issues with your code (please check the JavaScript coding standards). See attachment.
  • DrupalPractice has found some issues with your code, but could be false positives.

    Manual Review

    Individual user account
    No: Does not follow the guidelines for individual user accounts: It is a company account
    No duplication
    No: Causes module duplication and/or fragmentation.
    Master Branch
    Yes: Follows the guidelines for master branch.
    Licensing
    Yes: Follows the licensing requirements.
    3rd party assets/code
    No: Does not follow the guidelines for 3rd party assets/code: jquery.nicescroll.js, alter1.js and possibly others
    README.txt/README.md
    No: Does not follow the guidelines for in-project documentation and/or the README Template. See pareview
    Code long/complex enough for review
    Yes: Follows the guidelines for project length and complexity.
    Secure code
    No
    Coding style & Drupal API usage
    1. (*) Use $form/$form_state instead of $_POST in zohocampaigns.admin.inc
    2. (*) Repeating points of @jlicht :
      • There is a great deal of CSS embedded in the PHP code. This should be pulled out into CSS files.
      • Use descriptive variable names, and include some comments in the code.
      • The CSS in the code, and lack of consistent formatting and indenting make this code very difficult to review (as well as the lack of comments). Most of your CSS is static, so there is no need for inline code for these
      • Consider splitting the .admin.inc into separate files (it is over 5500 lines of code)
      • ZohoCampaigns.admin.inc

        Do not assume the module will be installed in a specific location when constructing image paths (example: L3428)
      • ZohoCampaigns.install

        Namespace your variable names and table names by prefixing with the module name to avoid collisions with other modules.
    3. (+) compress images in images folder, 300kB for each 30x30 pixel file is to big by far
    4. (*) delete commented code, it does make the code more clear
    5. (*) you are using a company account and many commits are from another user @dhivya-3115. This application leads in granting a individual person the vetted role, therefore your knowledge in secure coding, using Drupal API and so on is tested in a project which is should reflect the work of one person (not a company or a group). Open a new application with an individual account with a project in which you commited the most code

    The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

    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.

avpaderno’s picture

Priority: Critical » Normal
Status: Closed (won't fix) » Needs work
sleitner’s picture

Issue tags: +PAreview: security
avpaderno’s picture

27 commits have been done using dhivya-3115 as Git username, against 35 done from the user who is applying here. In this case, the project used to apply needs to be another one.
Since the account seems to be company account, though, the application cannot be accepted.

avpaderno’s picture

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

I am closing this application as per my previous comment, and the lack of activity.