This module integrates with rackspace webmail and provides a single sign-on for users.
It uses nusoap for Rackspace Webmail API integration.
It will create email account on rackspace webmail when new user register on Drupal site.
Example:--
if any user register with Drupal username: firstname.lastname
Rackspace webmail Inbox user will be: firstname.lastname@YOURSITENAME.COM
It will create a 'Mail information' tab on /user/* page, where you can see the total number of emails on your webmail and link to open your rackspace webmail.
Link to project (sandbox) page
https://www.drupal.org/sandbox/babusaheb.vikas/2508733
Git clone command
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/babusaheb.vikas/2508733.git rackspace_webmail_integration
cd rackspace_webmail_integration
Manual reviews of other projects:
- https://www.drupal.org/node/2509862#comment-10052526
- https://www.drupal.org/node/2502333#comment-10053034
- https://www.drupal.org/node/2507433#comment-10053404
- https://www.drupal.org/node/2477153#comment-10056532
- https://www.drupal.org/node/2507433#comment-10056808
- https://www.drupal.org/node/2505375#comment-10060588
- https://www.drupal.org/node/2505219#comment-10061004
- https://www.drupal.org/node/2513090#comment-10063586
Comment | File | Size | Author |
---|---|---|---|
#7 | mail_info_tab.png | 14.47 KB | babusaheb.vikas |
Comments
Comment #1
PA robot CreditAttribution: PA robot commentedWe 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.
Comment #2
darol100 CreditAttribution: darol100 as a volunteer and commentedAutomated Review
Pareview.sh - No errors, http://git.drupal.org/sandbox/babusaheb.vikas/2508733.git
Coder Review
rswebmail.module
Manual Review
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.
I did not see any blockers.
Comment #3
Ayesh CreditAttribution: Ayesh as a volunteer commentedDarryl: Sorry about setting the application back to Needs Work. But this module has some issues that I believe should be fixed first.
Thanks for submitting this, Vikas. I installed the module, but I could not review well without purchasing the rackspace service. But I reviewed the code, and unfortunately there are few changes that needs done.
- This module does not integrate with the libraries module, which is fine, but the instructions are not clear on how to install the third party library. You are encouraged to use the libraries module, but the module should not use a common name to refer to files in the libraries folder. sites/all/libraries folder contains all other libraries. General practice is to have a folder named after the library name to store the files. "lib" is not a unique name. Also, the README.txt instructions could be more precise.
- Also, module uses
require_once
which throws a fatal error. Since the README.txt instructions were not precise, it's easier for anyone to experience this error, reducing the reliability of the module.- The
rswebmail_details
function returns the last entered row in the database. Could you explain how it's supposed to work?- There is a reference to a file in sites/default/files folder. Please provide all assets with the module. You can use
drupal_get_path()
to get the path to the module folder.- The function name
rswebmail_form
is supposed to be a function fordrupal_get_form()
, but it also implementshook_form
. Using a different unique name should fix any possible issues.Comment #4
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks darol100 for your support and guidance.
I went through your points and fixed both Coder Review issues.
I have also fixed issues of Coding style & Drupal API usage
For this I have used libraries_get_path and added libraries module as a dependency in the .info file.
I have provided hook_help to allow site builders to find information about my module using Drupal UI.
Comment #5
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks Ayesh for your feedback and support. it is appreciated and I have fixed the issues.
I have added instruction about steps for install the third party library in README.txt file
which prevent anyone to experience this require_once error.
rswebmail_details function returns the last entered row in the database. === I have added a condition in rswebmail_create_form_submit() function.
if ($total_row == '0') then db_insert() will work otherwise db_update() will work
So, each time there will be only one row found in the database that will return using rswebmail_details().
I have used drupal_get_path() function and provide all assets with the module.
I have changed rswebmail_form name to rswebmail_create_form
Comment #6
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #7
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #8
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #9
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #10
BR0kENImplements hook_HOOK().
. So, change it herehook_enable()
needed here? You are not deleting the scheme inhook_uninstall()
, and installation hook will be performed before enabling. So, your schema will be available.Also, the comments for functions like this:
is very bad. Why we comment the code? We do this to help other developers work with our code. In your case, your comments only irritate me (IMHO).
Also, I cannot understand this loop. For what it needed?
You need to work a lot of time on this module before it will be published.
Good luck in your work!
Comment #11
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #12
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks BROkEN for contributing your valuable time to review my module.
I have made all the changes as pointed out by you.
Comment #13
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #14
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #15
BR0kENreturn
statement.@
for preventing warning. It takes the memory.looks not so good.
Also, I see the same code here. To follow the DRY principles we need to create additional function for this.
Comment #16
klausiThat minor issues are surely not application blockers, anything else that you found or should this be RTBC instead?
Comment #17
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks BROkEN for your support and guidance.
I have made all the changes as pointed out by you.
Comment #18
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedComment #19
babusaheb.vikas CreditAttribution: babusaheb.vikas commented@klausi, I hope this module is ready for RTBC status.
Comment #20
mpdonadioReviewing next.
Comment #21
mpdonadioComment #22
mpdonadioAutomated Review
Git errors:
Review of the 7.x-1.x branch (commit 8e2d7b1):
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
(*) rswebmail_block_content() is sending output from the webservice directly to the user with drupal_set_message(). One, this should
be part of a translated message. Two, the webservice is third-party data, and considered unsafe. Use t() with proper placeholders and some
context for the errors.
Use user_is_anonymous() instead of checking against $user->uid == 0.
There are a decent number of Drupal Coding standards that didn't get picked up by the PAReview process. Double check them when you get a chance.
Don't concatenate translated strings. Use placeholders.
rswebmail_block_content() should build up a render array and not markup directly.
You require the Libraries module, but you aren't really using the library API. Take a look at the docs for how to implement this properly.
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.
The blocking issues (*) are the unsanitized output of third-party data.
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.
Comment #23
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks mpdonadio for contributing your valuable time to review.
I have done the changes suggested by you:
1) Git default branch is set now.
2) rswebmail_block_content() is sending output from the webservice directly to the user with drupal_set_message(). this is now part of a translated message. I have used t() with proper placeholders and some context for the errors.
3) Added simple variables for database schema.
4) Used user_is_logged_in() instead of $user->uid != 0.
5) Added placeholders instead of concatenate translated strings.
6) Used render array instead of markup directly in rswebmail_block_content().
7) Used libraries_get_path() in correct way.
Comment #24
klausimanual review:
But otherwise looks good to me.
Assigning to mpdonadio since he might have time to take a final look at this.
Comment #25
mpdonadioAutomated Review
Review of the 7.x-1.x branch (commit 30918a4):
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
My earlier security concerns look addressed.
(*) I just noticed that the 'admin/config/services/rswebmail' path is only protected by the 'access Rswebmail' permission. This
means that all users who have access to the Rswebmail page/block can update the system settings and get the reseller username/password. If you add a new permission, say 'administer Rswebmail', and update the access on this path, I will approve the application.
Leaving myself assigned so it stays in my queue.
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.
Comment #26
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedThanks klausi for your review and support.
I have made all the changes as pointed out by you.
Comment #27
mpdonadioI will look at this tonight when I am on my primary Drupal dev machine.
Comment #28
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedHi mpdonadio,
Thanks for contributing your valuable time to review my module.
I have added a new permission 'administer Rswebmail' and update the access on this path.
Now, 'admin/config/services/rswebmail' path is protected by the 'administer Rswebmail' permission.
Comment #29
mpdonadioManual Review
Review of the 7.x-1.x branch (commit 2472e3e).
Read `git diff 30918a4`. My concern was fixed.
Comment #30
mpdonadioThanks for your contribution, babusaheb.vikas !
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.