Selfi allows to capture image using WebRTC, which can be used to set profile picture.
WebRTC is a free, open project that provides browsers and mobile applications with Real-Time Communications (RTC) capabilities via simple APIs.

Selfi module allows the user to specify how the user wants to upload a picture, if user selects to take a picture, camera is opened and an image is taken.

Project page: Selfi

Clone Repository:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/das.gautam27/2437741.git selfi

PAREVIEW:
http://pareview.sh/pareview/httpgitdrupalorgsandboxdasgautam272437741git

Manual reviews of other projects:
https://www.drupal.org/node/2511228#comment-10056842
https://www.drupal.org/node/2510074#comment-10056768
https://www.drupal.org/node/2445625#comment-9684039

Comments

kandy-io’s picture

Please add the below points in this issue about project description.

1. Git clone command: Please follow instructions:
You can find the correct git clone command for your sandbox by clicking on the Version control tab, removing the checkbox in front of "Maintainer", and clicking Show. You can then copy-paste the git clone command from the codeblock below "Setting up repository for the first time".
2. Please attach the zip file as feasible.

das.gautam27’s picture

Issue summary: View changes
das.gautam27’s picture

Hi @kandy-io,
I have updated the issue documentation, mentioned in #1,
related to #2, can you please provide more details, because if you want the project you can clone it using the git clone command mentioned above.

Thanks

naveenvalecha’s picture

Assigned: Unassigned » naveenvalecha

Assigned to myself for next review that may be tonight.
Please add more information on your project page.
As all reviewers 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).

das.gautam27’s picture

Issue summary: View changes
das.gautam27’s picture

Issue summary: View changes
naveenvalecha’s picture

Assigned: naveenvalecha » Unassigned
Status: Needs review » Needs work
Issue tags: +PAReview: security

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder are fine http://pareview.sh/pareview/httpgitdrupalorgsandboxdasgautam272437741git

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
No: List of security issues identified.
  1. (+)selfi_theme : you are passing the variables without sanitization in template.you need to sanitize it using check_plain.
  2. (+)selfi_form_alter : you are passing the variables in drupal settings js without sanitization.you need to sanitize it using check_plain.
Coding style & Drupal API usage
  1. Readme.txt is nice.
  2. selfi_help : Use single quotes instead of double quotes as per drupal coding standards.It will also give some performance hits as well.Add some more information in hook_help
  3. Move the admin configuration settings callback functions in seperate selfie.admin.inc file.It will give some performance hits as well.
  4. selfi_theme : Change the template 'take-selfi' name with module name prefix.
  5. (*)selfi_user_profile_submit: After viewing this module code it looks that module will work only for public file system ? if it is then please add a comment and also add on the project page regarding the same.
  6. selfi_upload_pic : Please add comments above at various lines in code.
  7. selfi_upload_pic :
      $status = array(
        'status' => ($success) ? array(
          'msg' => 'File saved successfully.',
          'value' => 1,
          'file' => $filename,
        ) : array('msg' => 'Problem while saving the file.', 'value' => 0),
      );

    Need to translate the message 'File saved successfully'

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.

das.gautam27’s picture

Hi @naveenvalecha,
Thanks for reviewing the project.
I have solved the items mentioned with "*", regarding the item mentioned with "+" the images are stored in the public file system and i have updated the project page with the same.

Thanks.

das.gautam27’s picture

Status: Needs work » Needs review
naveenvalecha’s picture

Assigned: Unassigned » naveenvalecha

Assigned to myself for next review that may be tonight.
I would recommend you, please help to review other project applications to get a review bonus back. This will put you on the high priority list, then git administrators will take a look at your project right away :)

naveenvalecha’s picture

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

Read (commit 215f909):

  1. selfi_help: Avoid use of html tags in translatable string.
  2. selfi_upload_pic : Don't use time() rather user REQUEST_TIME https://api.drupal.org/api/drupal/includes!bootstrap.inc/constant/REQUES...
  3. selfi_webrtc_settings : Add a validation function as well to check the writeable permissions on that particular folder.Else add a hook_requirements to check that whether the selfi_clicks directory exits.
  4. Please highlight on your project page that module only works with public filesystem.
  5. selfi.tpl.php : Add the available variables in the above comments that will be benificial for all the users.

As all reviewers are currently quite busy 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 :-)

mpdonadio’s picture

Assigned: Unassigned » mpdonadio

Assigning to myself for final review.

mpdonadio’s picture

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

Automated Review

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

  • 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

Classes, namespaced to your module are better than generic CSS IDs.

Spacing in JS doesn't follow Drupal coding standards.

(+) Your JS behavior should take into account the context and settings that are passed in.

It looks like your JS could have most/all of the internal functions in the closure, and not on the Drupal object itself.

You should note that thse get stored in the public filesystem, and are therefore publically accessible.

You are plaining the width and height, but look into using drupal_attributes() instead.

No need to call exit() in selfi_upload_pic. One drupal_exit() is better, two the page callback system will do this for you.

(+) You should really have configurable filesize and dimension checks. You should probably also validate that it is actually a PNG.

(*) You have a menu callback that access $_POST directly w/o checking for a valid token. This is a security problem. See http://epiqo.com/en/all-your-pants-are-danger-csrf-explained and https://docs.acquia.com/articles/protecting-your-drupal-module-against-c...

(*) With the CRSF, a user could fill the disk with images, and crash the server.

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.

das.gautam27’s picture

Status: Needs work » Needs review

Hi mpdonadio,
Thanks for your valuable review, i have fixed the above mentioned issues.

Thanks.

aneek’s picture

Hello @das.gautam27,

Its a great module to upload pictures to Drupal's profile. Reminds me of popular social media sites. Great Job!
So, I had a high level look at your module's code and site functionality. My observations are as below,

  1. You are creating a directory as "selfi_clicks" in the public files section. To do this you are using drupal_mkdir(). But why not removing the directory while the module is uninstalled? Since you are also removing all the variables from database keeping the files to server location doesn't make sense. To do this you might have to use drupal_rmdir() function.
  2. Initially the directory is created by the name of "selfi_clicks" but then I tried to change the directory name from configuration. But even I changed the name it was not changed in sites/default/files directory. It remained as "selfi_clicks". Is it that I have to manually create the folder and then change the name? That wouldn't be good UX right? Even if you tell users to create the upload directory manually and then change the location what will happen if the previous directory had some images stored?
  3. In selfi.module you have implemented hook_form_alter() but from the code I can see you have only altered a single form. So why not use hook_form_FORM_ID_alter? It's always better to use this one if you are not altering multiple forms in your module.

I hope these makes sense, I'd love to hear your thoughts on these above.
Thanks!

aneek’s picture

Status: Needs review » Needs work
das.gautam27’s picture

Status: Needs work » Needs review

Hi @aneek,
Thanks for your valuable review, i have fixed the issue's #3 and #2.
Regarding #1 , if the selfi_clicks folder is removed all the images in that directory will be lost and the user's who have used selfi to set profile picture will see a broken image, as the image path was set to public://selfi_clicks/.png in the database.

Thanks.

joachim’s picture

Wow, what a cool module!

Jut a few nitpicks:

  $items['upload-pic'] = array(

Menu paths should best be prefixed with the module name, especially one like this which AFAICT is purely internal.

$output .= '<p>' . t('It provide falicity to save user picture in public folder.<p> We are also working on providing facility to the user for their previous clicks clollection as a gallaery.') . '</p>';

A few typos in this line.

      'title' => t('Take Selfi'),
      'description' => t('User who can take selfi'),

Both strings could be a little clearer. Also, while the module is called 'selfi', the noun is 'a selfie' -- eg http://en.wikipedia.org/wiki/Selfie.

> * Implements hook_form_alter().

It's not this hook any more, it's hook_form_FORM_ID_alter() now.

das.gautam27’s picture

Thank you @joachim,
I have corrected the above issues.

Initially i decided to give module name as selfie itself but there was already a project named as selfie so i went on using with the word selfi.

joachim’s picture

I don't see a project at https://www.drupal.org/project/selfie ...

One thing I noticed is that the photo I take with the module looks squashed. Is that to do with the browser though?

das.gautam27’s picture

Hi joachim,
Please check this link https://www.drupal.org/sandbox/brockfanning/2273975.

Regarding the photo, can you please attach a screenshot for it.

Thanks.

das.gautam27’s picture

Priority: Normal » Major
das.gautam27’s picture

Priority: Major » Critical
omkar06’s picture

Status: Needs review » Reviewed & tested by the community

Hi @Gautam,

Very unique idea you got. Great. Looking forward to use this in my projects. Please keep contributing with such cool stuff.

Automated review result:

FILE: /var/www/drupal-7-pareview/pareview_temp/selfi.module
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
148 | ERROR | [x] Closing parenthesis of array declaration must be on a
| | new line
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Can you please check and fix this?

Manual review result:
Individual user account
Yes
No duplication
Yes
Master Branch
Yes
Licensing
Yes
3rd party assets/code
Yes
README.txt/README.md
Yes
Code long/complex enough for review
Yes : Code is long enough for review.
Secure code
Yes : Looks secured
Coding style & Drupal API usage
Yes : Looks good and follows drupal coding standards.

Thanks,
Omkar

das.gautam27’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus

Adding Review Bonus Tag

Ayesh’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed
Issue tags: -PAReview: review bonus

Hi Gautam,
I installed, tested and ran through your code. Everything looks great!

Thanks for your contribution, Gautam!

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.

das.gautam27’s picture

Thanks Ayesh.

Status: Fixed » Closed (fixed)

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