Comments

• 

  David Mudrák

  Thu, 13 Nov 2014, 6:00 AM

  Hi Igor. Thanks for sharing this plugin with the community. It provides a functionality that many Moodle sites call for, indeed. I am happy to see detailed documentation of the plugin at your site.
  
  It's evident your coding style is slightly different from the official Moodle one in some syntax details. Yet your style is consistent and the code is well structured and easy to follow - so it's not a problem.
  
  One thing I spotted during the code review is MySQL specific syntax in $DB->get_record_sql() query - single quotes wrapping the field names. Please make sure your plugin works with at least PostgreSQL, too (as these two are most commonly used DB systems Moodle sites run on). Also, please check the Moodle dev documentation for `$DB->sql_like()` that should be probably used in your case.
  
  Also note, when using set_config() and get_config() for the particular plugin, I believe the correct syntax is to use https://docs.moodle.org/dev/Frankenstyle for the full component (plugin) name. I know that some old auth plugins out there violate this but it became pretty standard convention.
  
  The code throws `Notice: Undefined property` at couple of places in out.php file. Please make sure to have development debugging on during development.
  
  Please note that the file view_admin_config.php does not seem to perform any security/access control related checks. Even if it currently relies on being included by the `config_form()` method only, it might make sense to perform some extra protection to make sure it does not reveal sensitive configuration data.
  
  For now, I am going to mark this plugin as needing more work until we get these issues resolved. Thanks for your patience with the review and approval process.
• 

  David Mudrák

  Fri, 21 Nov 2014, 5:35 AM

  Hi Igor. Thanks for the new version that fixes some of the issues raised during the last review. I can see you decided to follow the auth/lenauth style of the component identification in accessing the plugin configuration. As it is something that even legacy core auth methods do (there was no Frankenstyle when they were written), I can't really insist on it. Given that and the assumption there are probably sites using your plugin in production already, I am ok with letting it as it is now.
  
  I still experience these annoying PHP notices when trying to configure the plugin: https://gist.github.com/mudrd8mz/d99a33ce1a445e29b8c2 Can you look at those yet please?
• 

  David Mudrák

  Tue, 25 Nov 2014, 6:21 AM

  Thanks Igor for providing the new version with the fixes. The reason why the new version 1.0.6 did not appear as the most recent/current one was that there was no Moodle version selected as the supported one. I just edited the version record and selected Moodle 2.6 and 2.7 as supported (consistently with other versions).
  
  I am happy to go and approve this plugin now. Good luck with further development and maintenance of it. You are cleared to land, welcome to the Plugins directory!
• 

  Igor Sazonov

  Tue, 25 Nov 2014, 4:39 PM

  Thanks, David for your issues, really helpful to make the code more better! Released all of fixes/improves! Current version is 1.0.7
• 

  Bas Brands

  Tue, 25 Nov 2014, 5:39 PM

  Thanks Igor!
  
  It's great to have a new social media login plugin. And I think providing the HTML / PHP code for adding the buttons is great.
  
  I have just installed and tried this using the Facebook and Google login options on my Moodle site and both work fine using Moodle version 2.8. For Google I had to enable the Google+ api for it to work using
  Api's & Auths -> APIs
  Search for Google plus
  Enable it.
  
  It would be cool to get the user's picture from Facebook/Google/Twitter into the Moodle user profile to
  
  Thanks for sharing your awesome plugin!
• 

  Igor Sazonov

  Tue, 25 Nov 2014, 6:15 PM

  Hi, Bas!
  Thanks for good words!
  Yes, i need to add to documentation to enable Google+ API in developer console.
  Good idea about avatar, but not all socials returns this data, for example, Twitter provides only social id and username, so user need to complete his profile...
  PS: Yahoo and Twitter was much difficult about developing this plugin, because they has very difficult OAuth rules..
  So enjoy the plugin, be sure it has been tested much to be right working. Thanks again, have a good day!
• 

  Craig Simmons

  Wed, 28 Jan 2015, 4:29 AM

  Does this work with Google or only Google Plus? We are a K-12 school and so most of our students aren't old enough to legally have Google Plus.
• 

  Igor Sazonov

  Wed, 28 Jan 2015, 2:45 PM

  Hi, Craig! The plugin is only for Google+ in Google services, and what kind of service of Google you need except Google+?
• 

  Craig Simmons

  Tue, 3 Feb 2015, 6:36 AM

  Some of our students are below the age of 13 and are not allowed to have Google + accounts.