Kyle,

Many thanks for sharing this plugin with the Moodle community.

In order to review this plugin, the version(s) of Moodle for which it is intended needs to be specified. You can find this by navigating to the plugin, going to the Download versions tab, and clicking on the Edit details link for the most recent version. Under the Supported and/or required software section, select the Moodle versions for which that particular version of the plugin is designed to work with and then Save changes.

Please provide a documentation URL. You are welcome to create your documentation in Moodle Docs, See http://docs.moodle.org/dev/Plugin_documentation for more information.

You should remove the __MACOSX folder from the zip before uploading it. You may want to try selecting the "Auto remove system files" option and see if that gits rid of it. Similarly, I would recommend removing the .DS_Store files in all of the folders.

I noticed that some of the copyrights reference a previous author. It is preferred that all of the files that you have modified for this plugin have your name listed (or added) as the copyright holder.

The js folder seemed to be empty and should probably be removed.

I noticed that your plugin is storing its settings in the mdl_config table rather that the mdl_config_plugins. In order to avoid what I call $CFG bloat, it is recommended to use get_config to pull the data out of mdl_config_plugins.

The admin/settings.php contains the line:
$PAGE->set_url('/blocks/cmanager/manage.php');

Is the cmanager block a dependency? If so, it should be declared in the version.php file. See http://docs.moodle.org/dev/version.php for more information.

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. Once you have addressed these issues upload a new version and then reschedule the plugin for review. Please do not hesitate to let me know if there is anything I can do to be supportive of your efforts. Peace - Anthony

New version submitted! Thank you for the comments.

Hi Kyle,

thanks for updating the block code and the plugin record in this directory.
It will need some fixes yet though.

The settings.php throws fatal database error on PostgreSQL due to incorrect type of
the $d parameter (PARAM_TEXT) that is then used as the placeholder for the
$DB->delete_records().

Worse than that, the overall code flow based on if(isset($d)) condition in
scripts like admin/manage.php and admin/settings.php is wrong. As this
parameter is coming via optional_param(), it is always set. So every time you
visit these scripts, $DB->delete_records() is executed.

With regards to your repository at
https://github.com/Kylegoslin/moodle-block_socialbookmark it is strongly
recommended that plugin repositories have their root directly inside the
plugins repository. In other words, there should not be the folder
'socialbookmark'. This effectively prevents users from using your repository as
the source of the plugin because they can't easily clone it into their Moodle
source code tree (your .git folder would be located in /blocks folder,
possibly conflicting with other Git clones there).

There are some security related issues in your code that should raise your
attention - such as not using the parameter placeholders in SQL queries. And
again, things like $selectQuery = "courseid = '$cid'"; make the code
completely failing on PostgreSQL databases.

Your coding style is far away from the standards we like to promote in Moodle
devs community. It is encouraged to follow Moodle’s coding style as outlined
in: http://docs.moodle.org/dev/Coding_style and
 http://docs.moodle.org/dev/Coding Tthe code checker plugin can be quite
helpful in fine tuning your code and can be found at:
 https://moodle.org/plugins/view.php?plugin=local_codechecker You may wish to
consider using that tool to further improve your plugin

I noticed the uploaded ZIP still contains the __MACOSX root folder and
socialbookmark/.DS_Store file. These are valid for your local dev environment
and should not be part of the plugin package.

Let me suggest to go through the all code and clean it up. Redundant empty
lines, debugging code used for development (like in the
block_socialbookmark::cron() method) makes the code harder to review.

Note that in Moodle, the policy is not to use the Capitalised Words for titles
in English strings. Instead of 'Tags for This Course', the string should read
'Tags for this course' to look and feel consistent with other areas of Moodle.

I'm sorry but this needs significant amount of work yet to be approved. 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.

Hi Kyle. Firstly, thanks for all the improvements you made to the code since the last review. I am sorry, but I am still
experiencing fatal errors when trying to use your block on my notebook. Some of your database queries use MySQL specific syntax
(backticks) that make your block failing badly on other databases that Moodle supports.

I can still see some serious issues with your code. For example, total absence of sesskey checks make your code vulnerable against
CSRF attacks that can lead to data loss, or worse. Also, you probably want to introduce a new capability that would control who can
actually add bookmarks to your block. Things like this can easily become an easy target for spammers who could fill your users'
courses with links to malicious contents.

Overall user input processing is pretty weak in the code. I can see you are using the Moodle forms for generating forms - let me
strongly suggest to follow the whole forms API then. When processing data submitted from Moodle forms, there is no need for things
like $_POST or optional_param(). Not using the expected API introduces potential vulnerabilities to all sites that would use your
block. Please do not underestimate it.

I noticed that you create functions in the global PHP scope without the valid frankenstyle prefix - such as get_filter() or
get_bookmarks(). This is strongly discouraged in order to prevent collisions with (current and/or future) core code or some other
plugin. See http://docs.moodle.org/dev/Coding_style#Functions_and_Methods for details.

Please refer to the DML API for how to specify LIMIT and OFFSET in SQL queries in Moodle. Having them written like you have makes
again the code MySQL specific.

I would really like to see these issues resolved before we make the plugin available in the directory. Thanks for understanding.

New version added! Thank you for the feedback

Hi Kyle. Thanks for updating the code. I can confirm it works now as expected at my testing environment and I can see the critical parts of the code were fixed, too. I am going to approve your plugin now. There are still areas to be improved (as in any software) and I am sure you will re-act of the community inputs as they will come. Good luck with maintaining your plugin.

You are cleared to land, welcome to the plugins directory.

Thank you kindly for the feedback, great to see Social Bookmark live!

We are active on github, any issues can be reported to our tracker:

https://github.com/Kylegoslin/moodle-block_socialbookmark

so we can ensure to get on top of them for the next release!

Hello,

I am using older Moodle version (Moodle 2.4) for research purposes. I would like to integrate Social Bookmark plugin to it but the download of older plugin versions is not available. Where could I find Social Bookmark 1.5 version for older Moodle?

Hi Reda,
Version 1.5 worked with older versions but was buggy. We hid it. I've made it visible for a while so you can try it
https://moodle.org/plugins/download.php/6161/block_socialbookmark_moodle27_2014052815.zip

Hopefully it works for you

Hi!

I installed this plugin, and it works good for me as an administrator, but when I log in as a participant, it gives "error/Sorry you cannot view bookmarks" when I try to use it as a course participant.