Opt-in Cookie Consent

Hi

See automated review here
http://ventral.org/pareview/httpgitdrupalorgsandboxbeljaako1862782git

http://drupal.org/project/cookiecontrol
http://drupal.org/project/eu-cookie-compliance

This sounds like a feature that should live in the existing cookiecontrol project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the cookiecontrol issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

@beljaako

I like the module. I'm new to the module review process but I did a review using codesniffer and it cam back great. In a manual review of the code I didn't see anything that stood out to me. The only thing that I think could be changed is the process of notifiying the admin of the required JS libraries. It's clear in the README that those need downloaded, but if they haven't even anonymous visitors are getting the error message. "The Rijksoverheid Cookie Opt-in library library cannot be loaded. Please make sure that it exists before using this module."

Maybe look at not allowing the module to be configured unless those JS files are correctly installed. That way the admin has to install the JS before being able to enable any features.

Otherwise this looks good, a very useful module for EU and Dutch privacy laws.

changing name

This application is not fixed? See http://drupal.org/node/532400

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and 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.

I have implemented this module successfully in one of my projects. Two small remarks though:
- it was not clear to me whether I should rename the library folder, turned out I had to but the README.txt wasn't clear on that
- I couldn't find the settings page 'admin/config/system/cookie-consent' anywhere in the admin navigation. I would expect it to be somewhere under 'settings/config' and I would also expect a link to the settings page on the module page 'admin/modules'

manual review:

1. please add a list of similar modules to your project page and describe the differences.
2. opt_in_cookie_consent.module: why do you include the log.inc file in the global module context? It does not contain hooks? You should only include files when actually need them in function bodies, or if they include hooks the must be loaded on every single page request.
3. opt_in_cookie_consent_node_view(): all user facing text must run through t() for translation, the variable defualt value too.
4. opt_in_cookie_consent_node_view(): why do you check_plain() the variable? You are not printing it to the user later anywhere? Make sure to read http://drupal.org/node/28984 again.
5. opt_in_cookie_consent_node_view(): What is going on here? On what special node is that added and why? Please add a comment.
6. "'#default_value' => filter_xss(variable_get('opt_in_cookie_consent_close', 'Close this menu bar')),": no need to filter_xss() here since #default_value will be sanitized by the form API for you. Again, please read the docs about user provided text filtering.
7. opt_in_cookie_consent_init(): no need to check_plain() $cookie_name as it is not printed to the user?
8. opt_in_cookie_consent_js_alter(): why do you call your own hook there without any modification? So hook_opt_in_cookie_consent_scripts_alter() is the same as hook_js_alter()? Hooks that are provided by a module should be documented in MODULENAME.api.php, see http://drupal.org/node/161085#api_php
9. opt_in_cookie_consent_modify_cookies(): same here, please document your hooks.

Setting back to "needs work", you need to know where to sanitize text and where not. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

There is a module that performce the same task.
ro_cookie_opt_in
So why build a other one?

regards
Dirk

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

Changing "Reviews of other projects"

Sounds like you should collaborate with ro_cookie_opt_in then?

Can open an issue and/or contact the maintainer(s) to discuss what you need in addition?

Sounds like a good idea klausi.

I am the maintainer of the beforementioned 'ro_cookie_opt_in' module that integrates the exact same JS library from the Dutch government. If I am correct the current code under review is a fork of an early version of my code (I see a big resemblance in code).

If I remember correctly beljaako contacted me months ago to see if I would handover the maintainership to him completely and I suggested that he could become co-maintainer to add the additional functionality that he has been working on. Actually I suggested that we could create options for his sub-modules to hook into so that he would still have his own projects on drupal.org that extend the ro_cookie_opt_in module.

After that I didn't get any response anymore. There has been no recent contact.

As soon as the Dutch government releases a functioning version of their opt-out JS library I will create a version of the ro_cookie_opt_in module that integrates that library in a new branch.

I see no reason not to combine the two modules.

I agree that there should be cooperation and not competition between two modules that are so closely related. You are both trying to solve the same problem, and one of the modules is already published, so I would think the author of the new project should work with the maintainer of the old project, and combine ideas and functionality to create one module that meets the needs.

You don't need to become a co-maintainer immediately, just provide a patch in the issue queue.

Then you should create a new sandbox that has the extension module(s). Please link that sandbox then in this application and change the sandbox link in the issue summary accordingly.

@Beljaako As Klausi says: you can provide the necessary changes in a patch in one or more issues in the issue queue together with an explanation of the added/changed functionality that I can review and test and apply to the dev branch.

I'd be happy to implement any patches that improve the module. I will give you all credits for your contributions to the module of course. Your extensions can then go into separate modules.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.