Dynamic Zendesk Forms.
=======================

This is a module that helps to integrate zendesk and
your drupal site for rendering support forms and submitting
user input back to zendesk. This module dynamically pulls in form fields
and field rules directly from zendesk admin configuration. Without almost zero
coding we can set up a support form with fields of your choice.

This modules uses the rest apis exposed by zendesk
for powering the support(contactus) form

The forms can be used programmatically or just be embedded via the
provided block(s).

Uses the Zendesk API V2.
Requirements
------------

Once the module is installed and enabled, browse to the Status Report
page (admin/reports/status) and confirm that the curl lib is installed.

Configuration Parameters
---------------------------------
Go to admin/tve/zendesk for configuring the zendesk.

Below are the details about the configurations

Zendesk API key-
Zendesk API Key. This will be used for making Zendesk api calls.

Zendesk username -
The username of your Zendesk agent.

Zendesk API url -
Zendesk API url. The url is where you access your Zendesk backend.

Zendesk Ticket Form -
Select the ticket form that need to be displayed in front end

Zendesk System to Custom Field Mapping Description -
Select proper fields for the description,name and email fields

Configure the Minimum cache lifetime.

Zendesk will provide with separate account and url to access the backend.
For ex:
https://xxxxx.zendesk.com/agent/. Then zendesk api url will be
https://xxxxx.zendesk.com/api/v2/

Steps in the configuration:

1) Once you get hold of the api key and user name and the zendesk url, please click on save.
And if you want to try out you might have to register for a free version of zendesk in https://www.zendesk.com/register#company. And for getting the api key https://support.zendesk.com/entries/21733618-api-key

2) Please make sure to create some ticket form and ticket fields in the zendesk admin section. Please as below
Create forms:
https://xxxx.zendesk.com/agent/#/admin/ticket_forms
Create fields:
https://xxxx.zendesk.com/agent/#/admin/ticket_fields

3) Once you have completed the step1 and step 2. All forms will be listed in the Ticket forms select box.Please select corresponding form.

4) Since email,name and description is mandatory for a ticket in zendesk. You will have to create custom fields for all the 3 in zendesk and configure the corresponding custom field in Zendesk System to Custom Field Mapping Description.

5) Once you are done with it a block will be created and you can configure that block to be displayed anywhere in the site.

Usage:
-------------------------------
Once all the configurations are completed.
A drupal block will be created by name "Zendesk support form".

Alternatively you can call drupal_get_form('zendesk_create_ticket_form')
which will return the form array so that
we can display as per your requirements.

Please visit "/admin/structure/block" and enable the zendesk block.

Useful Additional Information
--------------------------------

Zendesk has ability to create (https://xxxx.zendesk.com/agent/) form and fields.
And we also have option to create multiple forms in zendesk
and attached has many fields has possible.
It has settings like to make it required and also
add conditional field settings in zendesk admin.

Basically this module will pull in the forms
and also form fields from zendesk including the label
and settings and will create a drupal form for the user.

Here are some links to Zendesk API documentation:
General introduction
http://developer.zendesk.com/documentation/rest_api/introduction.html
Tickets
http://developer.zendesk.com/documentation/rest_api/tickets.html
Conditional fields
https://xxxx.zendesk.com/agent/#/apps/conditional-fields
Create forms:
https://xxxx.zendesk.com/agent/#/admin/ticket_forms
Create fields:
https://xxxx.zendesk.com/agent/#/admin/ticket_fields

Future Updates:
1) Also Planning to work on a option to allow for a editor to configure two different zendesk form in a single drupal site.
2) Currently Caching applied through Blocks to avoid unnecessary calls to zendesk. But will work to add additional custom caching and settings for cache flushing etc.
3) Work on providing more flexibility for overriding the default form theme.

Similar Modules:
https://www.drupal.org/project/zendesk_feedbacktab
This module just opens the zendesk support widget in a dailog and we do not have much control any of aspects of it like form skinning etc.

https://www.drupal.org/project/zendesk
This module is used for zendesk authentication purpose. It is not related to support forms. The main purpose of this module is for adding users into zendesk and providing functionality similar to connect with facebook.

https://www.drupal.org/project/zendesk_forms
Although the purpose of this module and my module is similar. I decided to create a new module because the module i created is more dynamic module and concept behind my module and this module is completely different, i mean the architecture wise and concept wise both are different. Some main different

1) The existing module is completely static module and if we need to add more fields we need to write code using form alter etc.
2) My module is generic enough that in case of multisite setup we can have create two different forms with different field set without any code and just configuring it in zendesk admin section and configuring the right form in drupal zendesk backend configuration.
3) Supports conditional fields also without any extra code.Validation etc everything is configured in zendesk admin.
4) All the form creation logic is dynamic and completely controlled through zendesk admin.

Git Clone command:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/pkamerakodi/2326903.git zendesk

dynamic_zendesk_forms

Reviews of other projects:
https://www.drupal.org/node/2326343#comment-9089051
https://www.drupal.org/node/2323567#comment-9089153
https://www.drupal.org/node/2325405#comment-9089157
https://www.drupal.org/node/2322009#comment-9089463
https://www.drupal.org/node/2314061#comment-9089537
https://www.drupal.org/node/2294899#comment-9089579
https://www.drupal.org/node/2300519#comment-9095603
https://www.drupal.org/node/2328863#comment-9101177
https://www.drupal.org/node/2327875#comment-9101447
https://www.drupal.org/node/2329241#comment-9102427
https://www.drupal.org/node/2328931#comment-9108385
https://www.drupal.org/node/2308975#comment-9108417
https://www.drupal.org/node/2330445#comment-9109767
https://www.drupal.org/node/2330561#comment-9109819
https://www.drupal.org/node/2329957#comment-9115557
https://www.drupal.org/node/2331299#comment-9115613

I have reviewd 16 projects.

Updated Link for pareview:
http://pareview.sh/pareview/httpgitdrupalorgsandboxpkamerakodi2326903git-0

Comments

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
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/httpgitdrupalorgsandboxpkamerakodi2326903git

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

pkamerakodi’s picture

Issue summary: View changes
Status: Needs work » Needs review
pkamerakodi’s picture

Hi,

I have updated the git clone command to avoid errors. Please do review the code and let me know in case of any changes needed.

I have review other 3 projects.
https://www.drupal.org/node/2326343#comment-9089051
https://www.drupal.org/node/2323567#comment-9089153
https://www.drupal.org/node/2325405#comment-9089157

Regards,
Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi,

I have run my code through the automatic review process and have fixed all the code review comments. Please do a git pull and please review the latest code.

Regards,
Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi,

I have reviewed 6 other projects and updated the links, could someone please review the code. I have ran the code to through pareview and fixed all the warnings and error it showed. Some which are left are actually not mentioned in the coding standards list.

Regards,
Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi,

Added some more updates to the code to handle conditional required fields capability with some custom caching.

Also i see some warning in the PAReview report stating "Type hint "array" missing for $form", but actually this type of standard is not followed by any contrib module in the drupal community.

Please let me know in case of any modification needed.

Prajwal

pkamerakodi’s picture

Issue summary: View changes
naveenvalecha’s picture

Status: Needs review » Needs work

@pkamerakodi, first of all thanks to you for contributing.

Automatic Review

I'll be great if you include a link to pareview:
http://pareview.sh/pareview/httpgitdrupalorgsandboxpkamerakodi2326903git-0

It says that the default branch is not set. Here's the documentation: https://www.drupal.org/node/1659588

Manual Review

No duplication

Yes I'm not really sure about what exatly this module do, so ... I think it's a new functionality.

Master Branch

No: It does not have a default branch defined.

README.txt

Yes: The readme and the project page seems clear.

Code long/complex enough for review

Yes: a bunch of lines and functions.

Specify extension

No. : Please specify the curl extension is required for the module to work.

Thanks for review other project applications. Keep up the efforts.

Thanks
Naveen Valecha

pkamerakodi’s picture

Hi,

Thanks Naveen for the feedback I sincerely appreciate the help. I will apply the updates suggested by you

1) Setting the default branch

2) And will implement a hook_requirements and check for curl extension presence.

Thanks,
Prajwal

pkamerakodi’s picture

Hi Naveen,

I have made both the fixes. I have implemented hook_requirements and have added a check for curl lib extension presence and i am displaying appropriate message.

Please find the link to updated pareview link for the updated code:

http://pareview.sh/pareview/httpgitdrupalorgsandboxpkamerakodi2326903git-0.

Please do review the latest code and let me know in case of any updates needed.

Regards,
Prajwal

pkamerakodi’s picture

Issue summary: View changes
Status: Needs work » Needs review
pkamerakodi’s picture

Issue summary: View changes
naveenvalecha’s picture

@pkamerakodi, .thanks for the updations.

Automatic Review

It says that remove the translation function 't' from the following lines. Please do so and remove the warning from there.

As I don't know more about the zendesk 3rd party too much so I have done with my suggestions regarding the coding style and else.

As I am not a git administrator, so I would recommend you to keep reviewing other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Thanks
Naveen Valecha

pkamerakodi’s picture

Hi Naveen,

Thanks for the feedback. I did check those warning, actually "Only string literals should be passed to t() where possible". But since i just trying to translate the label from zendesk i could not remove it.

Had a small query how will change the status to "Review and tested by community"

Regards,
Prajwal

naveenvalecha’s picture

Hi Prajwal,
I have given +1 bonus to set this to RTBC but let's hear the comment from other git admins.

Thanks
Naveen Valecha

pkamerakodi’s picture

Thanks Naveen

er.pushpinderrana’s picture

Status: Needs review » Needs work

@pkamerakodi, thankyou for your contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Yes, http://pareview.sh/pareview/httpgitdrupalorgsandboxpkamerakodi2326903git-0 reported some minor issues that need to be fix.

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
(*) May be: Does not cause module duplication and fragmentation. As I googled, found following two modules similar to your module.
  1. https://www.drupal.org/project/zendesk
  2. https://www.drupal.org/project/zendesk_forms

As I haven't used these modules not sure how your module is different from others. Also if this module contains some other functionality then can you follow the steps in the link and confirm that the functionality contained in this module should stay separate from above two? Also list out similar module on your project page and show the differences how your module is different from others.

Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements
3rd party code
Yes: Follows the guidelines for 3rd party code.
README.txt/README.md
(+) No: Follows the guidelines for in-project documentation and the README Template. Personally I would say it is not exactly clear from Readme.txt file what your module works and how user would configure at initial stage.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
No. If "no", list security issues identified.
  1. (+) _zendesk_perform_curl_request: this is vulnerable to XSS. zendesk_api_key, zendesk_api_url and zendesk_user_name variables are coming form user provided input, you should sanitize it before passing further, make sure to read https://www.drupal.org/node/28984 again.
  2. Did correction as per https://www.drupal.org/node/263002.

  3. (+) _zendesk_list_form_fields: $fields_list = _zendesk_perform_curl_request(ZENDESK_REQUEST_LIST_FIELDS); Now $fields_list holds untrusted third party source and must therefore be considered untrusted user input. Am I right that you are directly getting this information from the web service and storing it as is (which is correct)? Did correction as per https://www.drupal.org/node/263002.

    Great thanks to mpdonadio for correcting these points.

Coding style & Drupal API usage
  1. I am unable to find where proxy_server and proxy_port are set in your module.
      $http_proxy_server = variable_get('proxy_server', FALSE);
      $http_proxy_port = variable_get('proxy_port', 80);
    
  2. hook_help() is missing in your module.
  3. It would be better to keep admin settings form link admin/config/services/zendesk in .info file to make it directly accessible from module page. You can make it like this configure = admin/config/services/zendesk
  4. $json = json_encode($data);: Instead of using json_decode(), should use drupal_json_decode()
  5. _zendesk_perform_curl_request(): Instead of using curl directly, better to use drupal_http_request().

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.

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

I haven't tested your module functionality yet and just did manual review of this module. Hopefully, I will test this module functionality in next week.

Thanks again for your hard work.

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi Pushpinder,

Thanks for review. It was very helpful. I will take a lot at all the comments and fix them. Mean while for your question on module duplicaiton i have added a section in the ticket description pointing out the differences

No duplication
(*) May be: Does not cause module duplication and fragmentation. As I googled, found following two modules similar to your module.

https://www.drupal.org/project/zendesk
https://www.drupal.org/project/zendesk_forms

As I haven't used these modules not sure how your module is different from others. Also if this module contains some other functionality then can you follow the steps in the link and confirm that the functionality contained in this module should stay separate from above two? Also list out similar module on your project page and show the differences how your module is different from others.

[Prajwal] - Added a new section in the issue description pointing out the differences. Please let me know if you have any concerns.

pkamerakodi’s picture

Hi,

Thanks for the feedback updated to code to handle all your review comments. Please review the latest code and let me know in case of any further changes required.

Updated pareview link:
http://pareview.sh/pareview/httpgitdrupalorgsandboxpkamerakodi2326903git-0

README.txt/README.md
(+) No: Follows the guidelines for in-project documentation and the README Template. Personally I would say it is not exactly clear from Readme.txt file what your module works and how user would configure at initial stage.
[Prajwal] - updated the read me txt file

_zendesk_perform_curl_request: this is vulnerable to XSS. zendesk_api_key, zendesk_api_url and zendesk_user_name variables are coming form user provided input, you should sanitize it before passing further
[Prajwal] - updated the code to handle this.

_zendesk_list_form_fields: $fields_list = _zendesk_perform_curl_request(ZENDESK_REQUEST_LIST_FIELDS); Now $fields_list holds untrusted third party source and must therefore be considered untrusted user input.

I am not storing any data and moreover zendesk is trusted source so since only admins have access to it.

I am unable to find where proxy_server and proxy_port are set in your module.
$http_proxy_server = variable_get('proxy_server', FALSE);
$http_proxy_port = variable_get('proxy_port', 80);
[Prajwal] - Normally drupal standards is that proxy is set in settings.php file using $conf varialbe. That is the reason we dont have any input fields for it.

hook_help() is missing in your module.
Added a help page

It would be better to keep admin settings form link admin/config/services/zendesk in .info file to make it directly accessible from module page. You can make it like this configure = admin/config/services/zendesk
[Prajwal] - updated the code

$json = json_encode($data);: Instead of using json_decode(), should use drupal_json_decode()
[Prajwal] - Changed the code to use drupal_json_decode

_zendesk_perform_curl_request(): Instead of using curl directly, better to use drupal_http_request().
[Prajwal] - I cannot make this change since the zendesk authenticated rest api call does not work using drupal_http_request.

Regards,
Prajwal

pkamerakodi’s picture

Status: Needs work » Needs review
pkamerakodi’s picture

Issue summary: View changes
ram0108’s picture

Hi,

There must be some api validation for "Zendesk API key", "Zendesk username" & "Zendesk API url".
So that if user put some wrong entry then they must know whats wrong!!

Thanks!!

ram0108’s picture

Hello,

I installed this module on my local machine. but did not get how it works..
If possible plz mentioned some necessary so that user can know about work flow and necessary steps so that they can know how it works!!

Thanks!!

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi Friends,

I reviewed 2 more projects. So that my project gets some special attention :). I will keep helping other developers in reviewing there code. So my project gets reviewed and approved soon :).

Regards,
Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi,

Reviewed one more project

https://www.drupal.org/node/2329241#comment-9102427

Prajwal

pkamerakodi’s picture

Hi Ram,

Thanks for your valuable comment. Actually you are point is really valid about adding a validation. But unfortunately zendesk does not provide any option currently. I will keep looking for it. I will add it to list of future updates to provide a test configuration button where it will try to create a sample ticket and see if it works.

For your Second question on usage Please check the readme or the issue description where i have put together both configuration steps and usage. And if you want to try out you might have to register for a free version of zendesk in https://www.zendesk.com/register#company. And for getting the api key https://support.zendesk.com/entries/21733618-api-key

Please let me know if you need more help in any specific steps, i will be glad to help you with.

Thanks for taking time and interest to look into my module. I sincerely appreciate your efforts.

Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi Ram,

I have updated the issue description and project description page with some detailed steps with reference links for testing.

Please let me know if you have need clarifications on any point.

Thanks in advance,

Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Hi Friends,

Reviewed 2 more projects, so that i module reviewing gets speedend :)

https://www.drupal.org/node/2328931#comment-9108385
https://www.drupal.org/node/2308975#comment-9108417

Regards,
Prajwal

er.pushpinderrana’s picture

@pkamerakodi, its nice you are reviewing other applications and contributing to community, would encourage you to do the same in future as well. Regarding the review process, admins are working on clearing these applications out as quickly as they can (without compromising the review process). As your application is just one week old, this is a new application and older projects have priority. There are currently five RTBC applications and nine Need Review applications before you, so it is understandable it will take few days to review your application :)

pkamerakodi’s picture

Issue summary: View changes

Hi,

Sure i will continue my efforts reviewed 2 more projects.

Prajwal

pkamerakodi’s picture

Issue summary: View changes
pkamerakodi’s picture

Issue summary: View changes
mpdonadio’s picture

Status: Needs review » Postponed (maintainer needs more info)

@pkamerakodi, sorry for the delay, but the end if summer got us backed up a little. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition, and I am still a little confused about the differences between this and zendesk_forms. Are you basically saying that your features couldn't be merged into zendesk_forms? That though they are superficially similar, that under hood the approach is so different that you couldn't fold your improvements into zendesk_forms?

pkamerakodi’s picture

Hi,

Thanks for your time. Since the concept behind this module is completely different than what is available it is difficult to merge, the only way is to override that module functionality completely and use this functionality. Because here i am completely integrating with zendesk apis to populate the forms and also some other functionality like providing conditional fields, validation for the fields to make them required or not. Order of the fields everything can be controlled through zendesk admin and exposed through apis. Where as the zendesk_forms is a simple module that just displays basic static field set, rest everything if the module user has to override functionality and implement himself through writing custom code. I understand your concern completely i did check all the possibility and then decide to create a new one.

Please let me know if you need more info.

Prajwal

pkamerakodi’s picture

Status: Postponed (maintainer needs more info) » Needs review
pkamerakodi’s picture

In this module i am calling zendesk apis to get the configured form fields and all form field settings and prepare the drupal form and render it has a block, where in the old it is just couple of fields if we need to override them we need to write form alter and code ourselves.

The functionality/Concept in this module including admin settings and also drupal form block is completely different from what that module supports. The only similarity is that both submit tickets to zendesk.

Prajwal

mpdonadio’s picture

Assigned: Unassigned » mpdonadio

OK, thanks for the explanation. I am assigning to myself for next review.

pkamerakodi’s picture

Thanks for your time sir. Looking forward for your review comments.

Prajwal

mpdonadio’s picture

Status: Needs review » Needs work
Issue tags: +PAReview: security

Automated Review

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

  • DrupalPractice has found some issues with your code, but could be false positives.
    
    FILE: /home/matt/PAR/pareview_temp/zendesk.module
    --------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
    --------------------------------------------------------------------------------
     298 | WARNING | Potential security problem: SSL peer verification must not be
         |         | disabled
     381 | WARNING | Unused variable $key.
    --------------------------------------------------------------------------------
    
    Time: 255ms; Memory: 8.75Mb
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and fragmentation. Addressed.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements
3rd party code
Yes: Follows the guidelines for 3rd party code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation and the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
No. If "no", list security issues identified.

(*) In _zendesk_perform_curl_request, you disable peer verification. This needs to be fixed. See http://stackoverflow.com/q/13740933 You should really use drupal_http_request() anyway, instead of directly using cURL.

Coding style & Drupal API usage
(*) There is already a module called 'zendesk'. You will need to change the namespace for yours.

Using cURL directly is not the best idea. Drupal provides drupal_http_request(), which should be used instead and is also proxy aware.

$form['zendesk_cache_lifetime'] should have a comment that the options are in seconds.

zendesk_create_ticket_form() probably needs a default case to handle additions in the future.

(+) In zendesk_create_ticket_form_validate(), :field is not a valid placeholder for t(). See https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/form... This is not a security problem, as they will behave like %field.

In _zendesk_perform_curl_request(), you don't need to filter_xss() here, these are not going to HTML output.

(+) The check_plain() in _zendesk_list_fields_by_form_id() is wrong, too. This is not user output.

In zendesk_create_ticket_form_validate(), instead of checking for empty fields, can you just make them required?

(+) In zendesk_create_ticket_form_submit(), it looks like you are plaining things that aren't going to output, just cURL (ie, HTML). See https://www.drupal.org/node/263002

cache_set/cache_get should handle the expiration for you. No need to check yourself.

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.

I think the two security issues mentioned in #24 above are not correct. #1 is not use output. My trace of #2 does not lead to direct user output either.

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.

mpdonadio’s picture

Assigned: mpdonadio » Unassigned

Forgot to unassign myself.

pkamerakodi’s picture

Hi,

Thanks for the feedback. I actually tried initially to use drupal_http_request, but due the way drupal handle username and password and how zendesk excepts it does not match and the calls do not go through. Since drupal base 64 encode the auth string and also doesnot work if the username has a slash in it.

That is the reason i had to switch to curl, but will look into the disable peer verification issue.

Thanks,
Prajwal

mpdonadio’s picture

Not a problem. It would be best to add a comment about why cURL is needed.

pkamerakodi’s picture

Status: Needs work » Needs review

$form['zendesk_cache_lifetime'] should have a comment that the options are in seconds.
[Prajwal] - Added the check for a scenario where user changes the cache time to 0.Since the cache get rebuild only when the form is accessed not form the admin settings, i had to add that logic.

In zendesk_create_ticket_form_validate(), instead of checking for empty fields, can you just make them required?

[Prajwal] - I could not add the required since those fields for which i am checking empty are conditional required validations not always required. And drupal states has a bug that it does not allow required using states.

In _zendesk_perform_curl_request, you disable peer verification. This needs to be fixed. See http://stackoverflow.com/q/13740933 You should really use drupal_http_request() anyway, instead of directly using cURL.

[Prajwal] - Removed the peer verification disabling. I had added it for testing the module in my local machines where SSL verification will not be set up. I have removed it from the production ready code, but added a note in readme.txt for people how want to try the module in local.

I actually tried initially to use drupal_http_request, but due the way drupal handle username and password and how zendesk excepts it does not match and the calls do not go through. Since drupal base 64 encode the auth string and also doesnot work if the username has a slash in it.

It is not possible to use drupal_http_request the only way around it is curl.

I have fixed all your review comments and some notes on thing that i could not update.

The security issue and starred items are handled.

Prajwal

mpdonadio’s picture

Assigned: Unassigned » mpdonadio

Assigning to myself for next review, which will hopefully be tonight.

mpdonadio’s picture

Assigned: mpdonadio » Unassigned
Status: Needs review » Reviewed & tested by the community

Automated Review

Review of the 7.x-1.x branch (commit 4f196eb):

  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

Manual Review

Looks like all of my (*) and (+) issues were taken care of.

You don't normally need to manually check $cache->expire. The Drupal cache keeps itself clean, and does garbage collection at the beginning of the call to cache_get().

Not seeing anything to prevent RTBC.

pkamerakodi’s picture

Hi Sir,

Thanks for all the help. Lot of good points learned from the reviews.

Regards,
Prajwal

er.pushpinderrana’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, pkamerakodi!

I have gone through 7.x-1.x branch (commit 4f196eb) and did a manual scan, and didn't see anything major. Not seeing any blocking issues, and also issues mentioned by other admins have been addressed.

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.

pkamerakodi’s picture

Hi,

Thanks everyone for the support you have given through the project. It was good learning experience.

Prajwal

Status: Fixed » Closed (fixed)

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