Description

This module provides integration with Nimble CRM. Each time someone submits a contact form response on your website, a new Contact will be added to your Nimble CRM account. It has integration with the core Contact module and Webform module.

Usefull links

Git clone command

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/ribel/2417261.git nimble_crm
cd nimble_crm
CommentFileSizeAuthor
nimble_crm_contact_config.png52.21 KBribel
nimble_crm_webform_component_config.png108.47 KBribel
nimble_crm_webform_config.png73.71 KBribel
nimble_crm_auth_config.png59.46 KBribel

Comments

PA robot’s picture

PA robot commented

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.

arijits.drush’s picture

arijits.drush
Primary language English
Location India
commented
Status: Needs review » Reviewed & tested by the community


Automated Review

No problem found @ http://pareview.sh/pareview/httpgitdrupalorgsandboxribel2417261git


Manual Review

Individual user account

Yes: Follows

No duplication

Yes: Does not cause

Master Branch

Yes: Follows


Licensing


Yes: Follows

3rd party assets/code

Yes: Follows

README.txt/README.md

Yes: Follows

Code long/complex enough for review

Yes: Follows

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

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.

oleh chemerys’s picture

oleh chemerys commented

Automated Review

No problem found @ http://pareview.sh/pareview/httpgitdrupalorgsandboxribel2417261git

Manual Review

Individual user account

Yes: Follows

No duplication

Yes: Does not cause

Master Branch

Yes: Follows

Licensing

Yes: Follows

3rd party assets/code

Yes: Follows

README.txt/README.md

Yes: Follows

Code long/complex enough for review

Yes: Follows

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

No issues found.

This review uses the Project Application Review Template.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Reviewed & tested by the community » Needs work 
throw new Exception("No client with name '$name' is defined.");

and

drupal_set_message($e->getMessage(), 'error');

Message text should be translatable with t().

/**
 * Class OAuth2\NimbleClient.
 */
class NimbleClient extends Client {}

Modules creating classes should place their code inside a custom namespace.

/**
 * Return the redirect_uri of oauth2_client.
 */
function nimble_crm_oauth2_client_get_redirect_path() {
  return NIMBLE_CRM_OAUTH_REDIRECT_PATH;
}

I guess that part is redundant.

$items['admin/structure/contact/nimble'] = array(

Would be ok to change nimble with nimble-crm (like in parent module).

ribel’s picture

ribel
Primary language Ukrainian
Location 🇺🇦Lviv
commented
Status: Needs work » Needs review

Thanks @zaporylie for the review!
I've fixed issues that you noted in commit d938b3f.

tkuldeep17’s picture

tkuldeep17 commented
Status: Needs review » Needs work

hi ribel,

Manual Review
1. You should also add install file in main module to remove variables which you have created in module.
2. Please explain me "why you have included all files in nimbile_crm module". IMO it should be included when it is required. So you should include these files conditionally.

inc files are created in drupal for two reasons:
1. Code readability and formatting.
2. Code is loaded in memory only when it is required.

ribel’s picture

ribel
Primary language Ukrainian
Location 🇺🇦Lviv
commented
Status: Needs work » Needs review

Hi @tkuldeep17,

Thanks for your notes.

  1. I've added removing variables in nimble_crm_uninstall() and nimble_crm_contact_uninstall().
  2. I've changed files including. As you said inc files was created for better code readability and formatting. Now module have:
    • -nimble_crm.module - Main module file. Here i prefer have only hooks implementations.
    • --includes/nimble_crm.admin.inc - Admin page callbacks. This file is included with hook_menu().
    • --includes/nimble_crm.core.inc - API callbacks, and other core functions.
    • --includes/nimble_crm.oauth2_client.inc - Contains class NimbleClient which extends Client class from oauth2_client module.

You can see changes in commit a663ddc.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs review » Reviewed & tested by the community

Seems pretty solid to me.

Setting RTBC.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.