[D7] Age Verification Module

In the .info file

• 'core' is defined twice.
• The package 'other' is implied, so does not need to be defined.

In the .module file:

• The @file definition does not meet documentation practices - see https://drupal.org/coding-standards/docs#file (the description should immediately follow the @file declaration; each author requires a separate @author tag…
• Typo in hook_help: "addeding".
• In hook_menu, the path "age-verification" use the file age_verification.admin.inc but does not appear to be an admin page. Use age_verification.pages.inc instead.
• In the docblock for age_verification_url_inbound_alter(), remove the @function declaration - it's not needed. In the same docblock, the Implements syntax is as follows: Implements hook_foo().: add parenthesis after the hook-name.
• The check is carried out in hook_url_inbound_alter(), which is called by drupal_path_initialize() in _drupal_bootstrap_full(). This means it won't run for cached pages. The kind of access-control is often handled in hook_boot() - consider moving it there. If it's OK for this to not run in cached pages, hook_init() may also be a better place.

In the .admin.inc file:

• The module stores information in the variables age_verification_urls and age_verification_description. You should add a .install file with hook_uninstall() implementations to clean those variables up when the module is removed.
• Consider adding @ingroup forms to the age_verification_admin_form() and age_verification_form() docblocks.
• In age_verification_form(), the dob form-element has a max-length attribute, which is not used by the date FAPI type.
• In age_verification_form(), consider adding a hint to the dob field, to show the format of the date (e.g. '#description' => t('Format: dd/mm/yyyy'),
• In age_verification_form(), the title in the confirmation form-element does not pass through t().
• In age_verification_form(), the request_uri form element is a hidden field: it's good practice that hidden fields are only used if they need to be changed on the front-end (for example, by JavaScript). A 'value' FAPI type can be used instead.
• In age_verification_form(), consider wrapping the submit button in an actions wrapper: see http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.ht...
• In age_verification_form_validate(), the form_set_error() syntax should be form_set_error($field_name, $error_message); - rather than using form_set_error and a separate drupal_set_message().
• The value '21' (years old) is hard-coded into the module. It would be great to make this flexible (for example, in the UK, many age-controls such as purchases of alcohol, tobacco, etc are set at 18 years old).

We 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.

Fix the issues in http://ventral.org/pareview/httpgitdrupalorgsandboxbiged2007914git

.install file is not available to unset the variables like 'age_verification_urls'.

Use 'administer age_verification' underscore is missing in " 'access arguments' => array('administer age verification'),"

Age verification form is coded in age_verification.admin.inc file which does not seems for admin configuration only make one more file or write your code in .module file which is not related with admins.

use hook_init() for preventing use to access the pages.

Hey @BigEd. Below are some points needs work.

1. age_verification_form($form, &$form_state, $request_uri) No use of $request_uri. just remove it. i hv checked the functionality after removing
2. Age limit should be configurable by admin. Do not hard code it.
3. I had mentioned in my previous comment, apart from admin configuration nothing functions should be in admin.inc file make another file with .pages.inc like or use .module itself for front end functionality.
4. Validation issue is their error comes if i got the "You need to be 21 or over to access the site." then click on another link it does not validate again.
5. Add a condition if visitor is on home page.

if (empty($user->uid) && empty($_SESSION['age_verified']) && $original_path != 'age-verification') {
    drupal_goto('age-verification', array('query' => array('destination' => $original_path)));
  }

Hey @BigEd. Below are some points needs work.

1. age_verification_form($form, &$form_state, $request_uri) No use of $request_uri. just remove it. i hv checked the functionality after removing
2. Age limit should be configurable by admin. Do not hard code it.
3. I had mentioned in my previous comment, apart from admin configuration nothing functions should be in admin.inc file make another file with .pages.inc like or use .module itself for front end functionality.
4. Validation issue is their error comes if i got the "You need to be 21 or over to access the site." then click on another link it does not validate again.
5. Add a condition if visitor is on home page.

if (empty($user->uid) && empty($_SESSION['age_verified']) && $original_path != 'age-verification') {
    drupal_goto('age-verification', array('query' => array('destination' => $original_path)));
  }

This is fine you are using hook_url_inbound_alter(). No need to use hook_init().

Awesome module, BigEd - I'll actually use something like this on a site I'm building for a brewery!
In you summary, you might change the git repo link to: git clone --branch 7.x-1.x http://git.drupal.org/sandbox/biged/2007914.git instead of your username-prefixed URI
lines 52 && 53 in age_verication_pages.inc are commented out ... just remove them
// kpr($form_state['values']['urls']);
// exit;

When I do a fresh install, and navigate to the home page, I get the verification page - awesome. Forces me to be 21 to enter... but when I go to the config page (admin/config/development/age-verification) the default is set to 16? Kinda strange.

Also, on config page the description, your comment about allowing HTML looks broken. I think I see why you coded it like that, but it looks a little funny.
... Can have html in example href="/cookie-policy" target="_blank">Cookie Policy ...

While I mostly agree with the default ages you supply, are there others that should be included? Or at least allow me to add my own. For example, drinking age in Haiti is 16, 19 in some parts of Canada, 20 in Paraguay. (source: http://en.wikipedia.org/wiki/Legal_drinking_age).

Totally going to use this module, thank you!
~R~

I've had a look at the new version - I think there's a misunderstanding about how foo.admin.inc and foo.pages.inc should be used.

foo.admin.inc should generally be used for the page-callbacks and supporting functions for admin pages. foo.pages.inc should generally be used for the page-callbacks and supporting functions for pages and tools that are available to regular users.

The admin callbacks were previously correctly found in age_verification.admin.inc. Those callbacks should be moved back to that file.

The /age-verification URL contains the callback age_verification_form(). The hook_menu() implementation specifies 'file' => 'age_verification.pages.inc', but the callback is actually found in the .module file.

In summary: you should:
- move the contents of 'age_verification.pages.inc' to 'age_verification.admin.inc'
- move the callbacks age_verification_form(), age_verification_form_validate() and age_verification_form_submit() to 'age_verification.pages.inc'.
- change the hook_menu() implementation to reference the files accordingly.

BigEd,

I took some time today to review this module. Nice work and something I'll like need to reach for some day. Here are some things to consider:

• Make README.txt formatting consistent. First item has a space after the number, other items don't. Nitpicky I know, but a nice touch. There are a few grammar errors as well.
• My cloned REPO contained a gitignore file, this should be removed.
• You may wish to add a "package" property to your .info file. I think something like "User Interface" would make sense for this.
• "Channel in bound URLs to age gate page if user is not verified" should be "Channel inbound URLs to age gate page if user is not verified"
• In age_verification.module: " * age_verification module renders form and allows user to enter if age entered meets minimum age value." Should probably be updated to better match other module descriptions.
• .module file has one extra trailing blank line. Should only be one.
• The description settings allows for HTML but the output is always wrapped in paragraph tags, is that intended?

Otherwise I didn't notice anything that jumped out at me. Module seems to work as intended.

Thanks,
Chris

Reading Chris' comments above, I realised that the description is vulnerable to an XSS attack, mitigated by the fact the a user needs the administer age_verification to carry out the attack.

You should either mark the permission as restricted:

/**
 * Implements hook_permission().
 */
function age_verification_permission() {
  return array(
    'administer age_verification' => array(
      'title' => t('Administer age_verification'),
      'description' => t('Configure Age Verification'),
      'restrict access' => TRUE,
    ),
  );
}

Or escape the markup through filter_xss (or similar):

$form['custom_age_verification_description'] = array(
  '#type' => 'markup',
  '#markup' => '<p>' . filter_xss(variable_get('age_verification_description')) . '</p>',
);

@Chris, I've not come across anything to suggest that .gitignore files should not be present in contrib modules, do you remember where you came across this advice? IMO it's OK to use them - although in this case, I'm not sure how much value it adds.
As for the package property in the .info file, I'd be happy to see it omitted. It's useful for grouping related modules (esi uses "Caching", so it's grouped with Varnish, Expire and similar modules), but "User Interface" may be a bit generic, I'm not sure that'd help with grouping. If a package property is omitted, the module simply uses "Other" which is a reasonable well-understood default.

The text "Configure Age Verification" comes from the permission's description, the text "Warning: Give to trusted roles only; this permission has security implications." is from the 'restrict access' => TRUE flag.

In this case, the title is enough to describe what the permission does, the description could be completely removed.

.gitignore file seems obsolete.

Codesniffer still finds some stuff http://ventral.org/pareview/httpgitdrupalorgsandboxbiged2007914git

You have to move the .project folder then, you can't have it because you need it, it's obsolete.

Hi there, I've taken a look over the module and here's what I've found:

1. Error on every page render: "Warning: strpos(): Empty delimiter in age_verification_url_inbound_alter() (line 90 of /var/www/html/d7/sites/all/modules/review/age_verification/age_verification.module)." Once I saved the configuration screen this went away. Something weird with the explode() on and empty string.
2. Caching: To my surprise everything worked well with Drupal's full page cache. The hook_url_inbound_alter() fires before Drupal checks page cache. For sites using Varnish or Boost for full page caching I expect this to break. You should probably add a warning to the project page and README noting this. I'm not sure how you would get around this with a Drupal-only solution. It would probably require some type of .htaccess check/redirect.
3. age_verification_admin_form(): Make sure to wrap the form element titles and the submit button's value with t().
4. age_verification_admin_form(): You're using system_settings_form() which uses the keys in $form to save variables and you're saving the variables in age_verification_admin_form_submit(). Right now the module is saving variables by the names of "age", "urls", etc which I believe is not intended. I recommend adjusting the keys in $form to match your intended variable names and completely removing age_verification_admin_form_submit().
5. $_SESSION['age_verified'] is a Boolean: It may be better to store the actual date and check it versus the limit instead of a TRUE/FALSE. For instance if the restriction was 18 and the user was validated, then the limit was later raised to 21 the user could access pages if they were 18, 19, 20. The session would eventually expire, but you've still got a loophole there.

Nice job. This looks useful and should be available as a project.

Forgot to change status.

I would like to commend BigEd on the patience he has had in this issue. It is my opinion that his application should already be approved at this point. He is very attentive to the requests, fixing issues, and seems to be interested in further development.

However, as maintainer of the Validate Age module, I want to extend an offer to create a D7 version of this module rather than creating a new project.

Yeah, I agree and I am truthfully okay with a complete divergence of the code for D7 assuming it is an improvement. Open an issue about this in the issue queue if you want to work out further details, or write me via my contact form. Best!

Hi nicholas,

I think @BigEd should get the full access for projects. I see since long time he has been working on module. I also agree that there is existing module available for Age Verification. So I too request to you for @BigEd to get full access so he can develop and publish another modules easily.

I appreciate to BigEd for his efforts.

Thanks,
@Ravindra Singh

Why not use the module load that seems to work for everybody?

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.

Happy New Year BigEd!

Hopefully these are minor issues. Everything works flawlessly but two items as far as I can see.

1) It seems that the math is slightly off. If I set it to 14 years and try to enter with a birthdate of Jan 1, 2000, I can't get in it tells me I must be 14 years old. Same for 16 years old and a birthdate of Jan 1, 1998. I am told I must have the corrects date of birth. I did check my Drupal settings and it shows that today is Thursday, January 9, 2014 - 06:11 (It's actually still Wednesday but I'll let the test site be out by a few hours.)

2) I am receiving a Theme Warning: Theme hook agil_list_form not found. I haven't got any idea what that is. The referrer appears to be: /age-verification?destination=node/xxx

Other than that it seems solid and easy to use. Awesome addition I'd say.

Tested on new vanilla D7 site with Bartik theme.

Cheers,
Andrew

Just a quicky. When you post Git links in the public. Make sure you post the public Git clone. In your issue here you have your D.O account git URL (biged@)

Use this one:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/biged/2007914.git age_verification

Module breaks drush.

I just discovered that with your module enabled, Drush terminates due to an "unrecoverable error". I confirmed this looking at the Drush debug log and tracing that core gets bootstrapped fully then crashes when moving to modules. Disabling your module makes the problem go away. I havent' looked at your code yet (about to) this is typically caused by a bug/problem in a hook_init() call.

Found the culprit. The drupal_goto is causing Drush to crash. Which is a typical reason for Drush to crash.

To fix this, add an additional condition to your IF statement on line 120. Add this as another check. This will not run the drupal_goto command if the caller is CLI (Drush).

&& php_sapi_name() != 'cli'

So line 120 becomes

if (empty($GLOBALS['user']->uid) && empty($_SESSION['age_verified']) && $original_path != 'age-verification' && php_sapi_name() != 'cli') {

@awasson In regards to #1. The math is correct. They are using the DateTime PHP library to do the calculations which is good. http://de1.php.net/manual/en/book.datetime.php

I cannot replicate your #2.

I'm most interested in this module because I am building a website for a brewery :-D

Not a blocker to project status but the custom form submit handler in age_verification.admin.inc is not needed if you key your form array with the variable names you want them to be. Drupal will automatically save the variables as the key name when using the return system_settings_form().

For instance:

  $form['age'] = array(
    '#type' => 'select',
    '#title' => t('Age Limit'),
    '#options' => array(
      21 => t('21'),
      20 => t('20'),
      19 => t('19'),
      18 => t('18'),
      17 => t('17'),
      16 => t('16'),
      15 => t('15'),
      14 => t('14'),
    ),
    '#default_value' => variable_get('age_verification_selected', ''),
    '#description' => t('Set this to the age limit you require.'),
  );

Should be:

  $form['age_verification_selected'] = array(
    '#type' => 'select',
    '#title' => t('Age Limit'),
    '#options' => array(
      21 => t('21'),
      20 => t('20'),
      19 => t('19'),
      18 => t('18'),
      17 => t('17'),
      16 => t('16'),
      15 => t('15'),
      14 => t('14'),
    ),
    '#default_value' => variable_get('age_verification_selected', ''),
    '#description' => t('Set this to the age limit you require.'),
  );

Then you can deleted the custom submit handler and save a little code.

Last comment. I have modified your code with the changes to fix drush and the recommendation for the admin pages and verified that it works. I'm actually needing this module for a site I am building like previously stated.

If you can, at a minimum, commit the fix for Drush then I would gladly say this module is ready for nomination as a project.

@Perignon: Yes I see that that code uses the Datetime library but are you able to properly verify ages? I can not.

If I were born Jan 1, 2000 then I would be 14 years old today. So if I enable the module on my site, set it to 14 years for the verification and upon entering the site tell it my birth date is Jan 1, 2000, I should be able to get in. I cannot. I have to tell it that I was born Jan 1, 1999 in order to gain access.

At first I thought maybe my server's date time was incorrect but glancing at recent logs I can see that my server's clock is slightly ahead of time but it still is set for 2014 (01/12/2014 - 21:57).

I will do a little looking into the #2 issue I noted and see if I can find the problem.

Andrew

Ok.... Issue #2.

If I run the module in a vanilla D7 site and I check my recent logs, I see an error: Theme hook agil_list_form not found.

This looks to me to be caused by the age verification form or more precisely in age_verification.pages.inc the function:

function age_verification_form($form, &$form_state) {
  $form['#theme'] = 'agil_list_form';
  $form['dob'] = array(
 

  . . . 

I believe the issue is that theme_agil_list_form() hasn't been defined.

I don't have access to my test site so I can't check but I think something like this is what is needed (I'll try this in the morning):

function theme_agil_list_form($vars) {
  $form = $vars['form'];
  // Do something agily or listy ; )
}

On further examination of issue #2, it looks like the line of code that is causing the php warning might be an erroneous piece of code in your module. I can't see where $form['#theme'] = 'agil_list_form'; makes sense in your module. I removed it and the warning goes away so maybe it was something that was left over from development. I've included a git patch if you decide to remove it. It just removed $form['#theme'] = 'agil_list_form'; from line 11.

The only thing left from my point of view other than the fix for drush from Perignon on comment #51 is that the math seems to be incorrect by one year. If that can be sorted out or we can figure out why it's out on my server then it's good to go.

Andrew

Hrm. I just confirmed the age issue myself. I'm going to try other ages to see what happens.

I found the problem. I'm working a fix. It's resolving the current date to just the year and basing the calculation off of that. What we need to do here I think is use EPOCH time to do the calculation - break it down to seconds and it's easy to do the math.

Rolled a patch against this to include all the changes.

Note that there are more changes than just the time calculation. I had already removed the submit handler and converted the admin page over to using Drupal's core functions for system settings forms. I also ran code styling/formatting over the files.

Nice work Perignon. I ran that patch against my local copy and it's working perfectly!

BigEd, if you apply the patches from #57 & #60 it'll satisfy the concerns I had and I'll be happy to vote this module gets promoted.

By the way, check patch #57 and my comments first before applying it in case you did intend to use the statement: $form['#theme'] = 'agil_list_form';

Cheers,
Andrew

Thanks BigEd, I'm out of town at the moment but I'll see if I can get around to reviewing your changes in the next day or two.

Looks great BigEd.
Thanks for the effort.
I'm all for this module being promoted!

Me too. Lets get this promoted!

Project page

Please take a moment to make your project page follow tips for a great project page. You

• You have a typo in your .info file, "in bound" should be one word without the space.
• Now is a good time to remove age_verification_init(). In general, when you want to update a module's settings from one version to the next, you should use hook_update_N(). And you don't actually need to set any defaults for variables - you're meant to provide a default when calling variable_get().
• Since users are free to modify their own user agent string at will, you should note on the project page that this module does not provide complete protection unless you remove all user agents from age_verification_useragents.
• What are the key values in the age_verification_selected select list? Can you use an integer field instead, so the admin can set whatever age they like?
• All user facing text should be run through t(), for ex "I confirm that this is my age".
• Instead of checking for $form_values['confirmation'] != 1, it's easier to make that form element #required when creating the form in age_verification_form().

Otherwise l think this module is ready to go! There's no major issues listed above, but it would be nice to reduce the number of issues.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Since I wrote this code:

"What are the key values in the age_verification_selected select list? Can you use an integer field instead, so the admin can set whatever age they like?"

This is where you pick your battles. Do you define it via a drop down or do you add more code to calculate the number of seconds based on the user input as well as validating the user input.

In this case I would let this go forward the way it is written and put the ability to put an arbitrary amount of time in the roadmap for future development of the module. Because this is asking the contributor/developer to do more work instead of fix problem with the existing code.

I fully would (and have) accepted the module with just a drop down. The #1 use of this module is for people 18 and 21 years of age.

I would agree that line 21 of age_verification.pages.inc needs changed to:

'#title' => t('I confirm that this is my age'),

In regards to:

"Instead of checking for $form_values['confirmation'] != 1, it's easier to make that form element #required when creating the form in age_verification_form()."

It is already required. Seems the code in the validation is a second check?

This still not been promoted to a project yet?

Well, I guess we're in the wait part of the process. All the hoops appear to have been jumped through and according to the process we are here:

Once all issues have been addressed:
The reviewer will change the status of the issue to 'RTBC'

We've done that. The next step is:
A 'git administrator' will validate the review, grant the applicant the 'Create Full Projects' role, and change the status of the application to 'fixed'.

Yeah I think we are at a loss of git admins..

You have to get the review bonus, or it won't be looked upon.

Yup, doing some reviews will speed things up for sure. Once you've done a review or three, update this thread noting that you've done some reviews and they'll be pretty quick.

Removing review bonus tag, you have not all manual reviews, you just copied the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

Hey BigEd,
Once you've done the manual reviews, update this issue and add the "PAReview: review bonus" tag so that the powers that be will be alerted of the status.

Cheers,
Andrew

Hello, BigEd.
I think that will be very good to apply this patch.

Also, there is a backslash before the php tag on the first line of the .module file, we definitely do not want a full project with such an obvious error.

Good job on the module though, will be handy in future.

I might have some feature requests which I need for a client, maybe some of them could be generally used. If so, I will post patches not dependent whether the module will be or will not be committed by then.

Automated Review.

FILE: /var/www/drupal-7-pareview/pareview_temp/age_verification.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
94 | WARNING | Unused variable $agent_lines.
97 | WARNING | Variable $agentlines is undefined.

These are leading to warnings and notices:

Notice: Undefined variable: agentlines in age_verification_url_inbound_alter() (line 97 of age_verification/age_verification.module).

Warning: Invalid argument supplied for foreach() in age_verification_url_inbound_alter() (line 97 of age_verification/age_verification.module).

This is a blocking issue.

Manual Review.

Module duplication/fragmentation is a problem on drupal.org. You need to elaborate on how this differs from other modules
and outline their differences, particularly with Legal. Put this in the description above, and on your project page.

You don't need to variable_set() in age_verification_install; use default values in your variable_get() calls.

I get warnings / notices:

age_verification_menu() should have a comment in it about why the age-verification page always needs to be accessible.

In age_verification_url_inbound_alter, use drupal_is_cli() instead of function_exists('drush_main')

age_verification_url_inbound_alter() shouldn't have explicit return values.

Use user_is_anonymous instead of empty($GLOBALS['user']->uid.

In age_verification_admin_form(), don't run HTML through t().

variable_get() should always have a default value.

Comment the magic numbers, like 662695992, to explain what they are.

Does the logic take into account leap lears properly?

In age_verification_form(), you don't need to do that with the default value on line 15.

Using the !age placeholder in age_verification_form_validate() is a a bad idea. What if things change with how you
store data? Use @age as a precaution.

If $form['age_verification_description'] is going to allow HTML, then don't use the paragraphs in age_verification_form(). Use
#prefix and #suffix to wrap it in a div w/ a class.

Your README should warn users that someone can set the user-agent and bypass the check.

The module description in the .info doesn't need to be quoted.

hook_inbound_url_alter may not be the best place for the meat of the logic. It gets invoked by drupal_get_normal_path(), so your check
will happen places other than at the beginning of a page reqeust. Really weird things may happen during search indexing when
cron.php is invoked from crontab. I really think this is a blocker (major API problem); it was mentioned early on in the
comments above, but I am not seeing an explanation as to why it has to be this hook.

I'm setting this back to needs review; the proper procedure is for others to set RBTC.

Of the things I mentioned, the duplication issue was the blocker. This is in the project application checklist (https://drupal.org/node/1587704), and is something that reviews are being explicitly asked to check for. Edit your info into the description above, so that it is available for final review.

If hook_boot() isn't suitable for all you need to do, that I won't use this as a blocker, but I think you need to test whether outside cron runs will still function (or you may need to add a test for cron.php, authorize.php, update.php, and xmlrpc.php in your logic). But make sure you document this.

I will likely review your changes tonight.

Two quick comments, though.

Even though many modules don't use the second parameter for variable_get(), it is best practice to use it.

With placeholders and which flavor to use, don't think of it as sanitization being needed or not. Think of it more as "I know I have HTML so I need this to go through unaltered and need !arg."

Guess I need to play catch up on this module. I am already using this (modified) on a production website. Being that I hacked it my code base has deviated of course. I honestly cannot believe this has been lingering now for almost a year.

This module has been in the queue for one year and 3 days. Duplication of other modules has long been addressed.

I would like to quote myself again:

...as maintainer of the Validate Age module, I want to extend an offer to create a D7 version of this module rather than creating a new project.

I also need to note my other comment:

I am truthfully okay with a complete divergence of the code for D7 assuming it is an improvement. Open an issue about this in the issue queue if you want to work out further details, or write me via my contact form.

In summary:

• I don't agree with creating a new project that offers duplicate functionality to Validate Age project.
• I have offered to allow BigEd to create a D7 branch/version of the module on the Validate Age project, which would be a complete rewrite, possibly even with no upgrade path.
• I already gave BigEd Maintainer access to the Validate Age project months ago, yet no contributions to the project as of yet.

I am not really sure why this thread keeps going under review when you have everything you need BigEd. I am marking this thread as Needs work since we should agree on whether this module really subtantiates creation of a new project. I don't believe it does.

In this case I totally agree with @nicholas.alipaz his statements here give free reign over the Validate Age module with no upgrade page, no rewriting code, and starting from a clean slate - go with it.

@Perignon, the project is over a year old, but it got a review bonus six days ago. Having prospective module maintainers review projects helps get everything reviewed faster, and also exposes new contributors to more code (both good and bad). Personally, I am working through the mentoring process to try to become a review administrator to help with the backlog of project applications. One of the things mentioned to me is the duplication/collaboration process. This is rather explicit in the project application checklist, and I have seen applications turned down as a result. It also needs to be part of the "application" which is the top description on this page. Having all of this in one place really does help admins. See also https://drupal.org/node/1187664

@mpdonadio Correct that the duplication/collaboration process is very important. I have also put my $0.02 cents in on the entire application process (I started some of the flamethrowing threads on G.D.O). However the duplication issue was resolved because there was not a module for D7 that answered the question of age verification. Now recent information has come to light about the collaboration issue. The details of which we do not know but we have the maintainer of the D6 module saying "go for it" and that changes things.

Like I said before, I needed a capability like this for a upstart brewery website so I used this module but modified it for my own means. Prior use should have some factor in the application process!

I wrote this 9 months ago:

I am truthfully okay with a complete divergence of the code for D7 assuming it is an improvement. Open an issue about this in the issue queue if you want to work out further details, or write me via my contact form.

That means you have free reign to write a D7 branch that is exactly the code you have already written, without any upgrade path. Just do it, you could've done it months ago when I gave you access to the project.

Hi BigEd,

One error still there "http://pareview.sh/pareview/httpgitdrupalorgsandboxbiged2007914git".

BigEd, maybe do this. Create your module as 7.x-2.x branch and edit the description of the project to say that 7.x-2.x is a complete rewrite and there will be no upgrade path. The 7.x-1.x branch would then be for a straight upgrade from 6.x if we ever tried, or someone provides one.

BigEd, you do have full project access. You have all permissions to the project except 'Administer maintainers' and have for quite some time.

Could you rename and commit your code to the validateage repository?

I think we can proceed with reviewing and granting the git vetted user role after that trust has been established.

Resetting status so one of the admins can take a look.

Review of the 7.x-2.x branch:

• ./validateage.install: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming
  

  function age_verification_uninstall() {
  function age_verification_install() {
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/validateage.module
  --------------------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  --------------------------------------------------------------------------------
   5 | ERROR | Doc comment short description must be on a single line, further
     |       | text should be a separate paragraph
  --------------------------------------------------------------------------------
  

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:

1. Please update the issue summary of this project application so that it is clear what git repository to review.
2. "'access arguments' => array('administer Validate Age'),": the permission should be all lower case, right?
3. validateage_url_inbound_alter(): doc comment is wrong, this is not a preprocess function?
4. "'#value' => 'Submit',": all user facing text must run through t() for translation.
5. validateage_url_inbound_alter(): the second function_exists('drush_main') is useless since you already checked that in the beginning.
6. validateage_url_inbound_alter(): "return TRUE;" is useless in this hook, since the return value is ignored. Just use "return;"?
7. Why do you even use validateage_url_inbound_alter() if you never change $path? Shouldn't you alter $path instead of using drupal_goto()? Please add a comment.

But that are not absolute critical application blockers, otherwise I think this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to mpdonadio as he might have time to take a final look at this.

Updated git link to refer to the project and not the sandbox.

That's what I get for not previewing...

Automated Review

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-2.x branch:

• ./validateage.install: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function age_verification_uninstall() {
  function age_verification_install() {
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• DrupalPractice has found some issues with your code, but could be false positives, and may be duplicate results from Coder Sniffer. See attachment.

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.

Not attaching the results, as they are the same as in #116.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

No: Follows the guidelines for master branch. Not a blocking issue in this case asn this is a legacy project.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt

Yes: Follows the guidelines for in-project documentation and the README.txt Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes.

Coding style & Drupal API usage

It looks like many/most of my comments in #94 didn't get addressed.

I concur with klausi's comments in #116.

This module needs TLC for prime time, but there are no critical application blockers.

Thanks for your contribution, BigEd!

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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.

i really need this module with a little tweak, only the question are you over 18 and two buttons with yes or no (or something like that) love this concept and it works great but i want to over my visitors a one click agegate.

can somebody help me to tweak this module to do just that? i'm not really a code wizard ;)