Sandbox Page:
https://drupal.org/sandbox/jribeiro/2174305
Git Repository:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/jribeiro/2174305.git webform2sftp
Reviews of other projects
https://drupal.org/comment/8455627#comment-8455627
https://drupal.org/comment/8455427#comment-8455427
https://drupal.org/comment/8455197#comment-8455197
https://www.drupal.org/node/2390489#comment-9419867
https://www.drupal.org/node/2386767#comment-9419913
https://www.drupal.org/node/2373075#comment-9419951
Overview
This module provides the functionality to export webform submissions to a secure SFTP server.
Features
- Security: The module focuses on the integrity of the data to be exported, sending the data via secure connection (sFTP) and be possible to encrypt (PGP) the data file before sending.
- Multiple Integration: The module serves multiple webforms by separating the settings for each one.
- File data: Is basically a file containing a submission in each line, separating the field values for a character of your choice, usually pipe "|".
- Schedule and automation: You can automate and schedule data exports, the Job will be executed by system cron in the time you set.
- Notification: You can configure a email to be sent when the Job executes successfully.
- Encryption: Is possible to use PGP encryption in the data file before it is sent to a sFTP.
Requirements
- This module depends of webform module.
- IMPORTANT: The module uses the PHP extension SSH2 to connect to SFTP server and PHP GnuPG extension for file PGP encryption, so installing these extensions are REQUIRED to module works, otherwise errors will occur.
Comment | File | Size | Author |
---|---|---|---|
#22 | webform2sftp.txt | 6.86 KB | er.pushpinderrana |
Comments
Comment #1
jribeiro CreditAttribution: jribeiro commentedComment #2
jribeiro CreditAttribution: jribeiro commentedIssue Tags
Comment #3
PA robot CreditAttribution: PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxjribeiro2174305git
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #4
jribeiro CreditAttribution: jribeiro commentedThe errors and warnings are now fixed.
Comment #5
jribeiro CreditAttribution: jribeiro commentedThe errors and warnings are now fixed.
Comment #6
caiovlp CreditAttribution: caiovlp commentedThis looks really good, and I even looked for security issues, missing hook_requirements, missing dependencies, and all that stuff, but could not find any major issues. I just found some minor issues:
- Missing t() function for some exception messages @ webform2sftp/class/webform2sftp.classes.inc
- I couldn't find out how this module would escape line breaks when printing variables. I might have missed it, but if this is not being handled it could break your file.
Other than that the code looks spotless.
Comment #7
jribeiro CreditAttribution: jribeiro commentedThanks for your review @caiovlp,
- Related to security issues, hook_requirements is implemented, to check and warning about the required PHP extensions.
- Related to t() for exception messages, I fixed the call to the watchdog, so that it is translatable now.
Thank you again.
Comment #8
pijus CreditAttribution: pijus commentedI gone through the module and did not find any major issues. Just found some minor issues/improvements -
webform2sftp.install
Line 63 and 76: Comment should be read "Implements hook_install()." and "Implements hook_uninstall().".
webform2sftp.module
Line 83 : '=' is default value in the condition() method, If no operator is specified, = is assumed.
->condition('nid', $component['nid'])
See the documentation - https://drupal.org/node/310086
Comment #9
jribeiro CreditAttribution: jribeiro commentedThanks for your review @pijus,
I fixed the comments in .install file hooks.
About the Line 83, I'm actually using the operator "=", nothing to be changed in this case.
Comment #10
caiovlp CreditAttribution: caiovlp commentedGood work Jean!
Looked it over again and tested out functionality as well... Looks really good and certainly much better than most modules out there.
Comment #11
klausiReview of the 7.x-1.x branch:
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:
'#prefix' => t('<strong>Trigger File</strong>'),
: HTML tags should be outside of t() where possible. Also elsewhere.'#markup' => '<strong>Components Order</strong><br/>The order that components will be exported in file to sftp'
: all user facing text must run through t() for translation. Please check all your strings.So I think the gpg config files are a blocker right now. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #12
jribeiro CreditAttribution: jribeiro commented@klausi,
Thanks for your detailed review, helped to improve the quality of the module and my learning.
Follow the responses:
1 - I modified to file be inserted only when necessary.
2 - Fixed. Implemented hook_form_FORM_ID_alter().
3 - The problem is that when I add this function in admin.inc, an error is displayed, that the function can not be located. For this reason, I kept on .module file.
4 - I not had enough time to Implement test cases, maybe in the next release.
5 - Changed to db_query() when was possible.
6 - Fixed. Changed the field type to password.
7 - Fixed.
8 - Fixed all watchdog() functions.
9 - Fixed: all strings checked.
10 - Actually this folder was not necessary. Removed from the project, function also reviewed.
11 - This line was removed.
12 - Fixed to set GNUPGHOME to temporary path.
13 - All supressed warnings were removed.
Comment #13
gabrielmachadosantos CreditAttribution: gabrielmachadosantos commentedModule looks great with the new fixes, moving to reviewed.
Comment #14
dixon_I've had a look through the code and it looks good. Would be nice to see this move through :)
+1 RTBC
Comment #15
jribeiro CreditAttribution: jribeiro commentedComment #16
jribeiro CreditAttribution: jribeiro commentedAnother battery of reviews to make this request "up":
https://www.drupal.org/node/2390489#comment-9419867
https://www.drupal.org/node/2386767#comment-9419913
https://www.drupal.org/node/2373075#comment-9419951
Comment #17
PA robot CreditAttribution: PA robot commentedProject 1: https://www.drupal.org/node/2390607
Project 2: https://www.drupal.org/node/2390569
Project 3: https://www.drupal.org/node/2190225
As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).
If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #18
jribeiro CreditAttribution: jribeiro commentedOk, chosen that application to be the chosen one.
Again, reviews list:
https://www.drupal.org/node/2390489#comment-9419867
https://www.drupal.org/node/2386767#comment-9419913
https://www.drupal.org/node/2373075#comment-9419951
Comment #19
naveenvalecha@jribeiro,
Thanks for your contribution.I am setting the issue status "needs review". you need to wait some one else to review the project application and make it RTBC.
Comment #20
natanmoraesRTBC +1
Comment #21
er.pushpinderrana CreditAttribution: er.pushpinderrana commentedAssigning to myself for next review, which will hopefully be soon.
Comment #22
er.pushpinderrana CreditAttribution: er.pushpinderrana commentedAutomated Review
(+) Best practice issues identified by pareview.sh / drupalcs / coder. Yes, See attached (
webform2sftp.txt
).Manual Review
(+) When I access
webform/sftp-export
url, it produces following warning that need to be fix.Warning: Creating default object from empty value in _webform2sftp_get_components() (line 179 of C:\xampp\htdocs\drupal\sites\all\modules\webform2sftp\inc\webform2sftp.webform.inc).
(+) webform2sftp_configure_form(): When using explicitly set HTML id attribute prefix it with module name to avoid name collisions. Alternatively, HTML id attributes can be passed through drupal_html_id().
A hook_help() would be nice.
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.
But these are not blocking issues. I also tested module functionality w.r.t XSS, CSRF and Sql Injection and it worked as expected. Good Job!
Also, Blocking issues from #11 have been addressed and after reading through `git diff bc7a3f2..HEAD` didn't see anything major so...
Thanks for your contribution, Jean Ribeiro!
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.
Comment #23
jribeiro CreditAttribution: jribeiro commented@er.pushpinderrana , Thanks for your support!!
I fixed the errors and code standards issues and promoted the module :)
Thanks again! Keep pushing!