closed site [Drupal 7]

Welcome,

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

This short and sweet module works as advertised: a single on/off checkbox acts as a killswitch for making the site content unavailable to anonymous traffic, and turns the homepage into a user login page with a custom title. Here's a short list of issues from my review:

• Some basic Drupal coding standards violations (spacing issues -- indentation looks correct). See http://drupal.org/coding-standards.
• In closed_site.module, line 52, a call to watchdog() uses 'access denied' as the $type argument. Consider using your module's name as this argument to make it clear which module threw this warning. Something like this would work:
  

  <?php
  
    watchdog('closed_site', t('Access denied for "!path"', array('!path' => check_plain($_GET['q'])), NULL, WATCHDOG_WARNING);
  
  ?>
  

• Installing the Drupal standards for CodeSniffer is a great way to generate an itemized list of standards violations. Here is a link to the output of CodeSniffer set with Drupal standards: http://pastebin.com/tYcUevuy

How does this module differ from standard access content permission and http://drupal.org/project/r4032login module working together?

Automatic review

An automated review through coder found 11 normal and 1 minor warnings.
closed_site.module

Line 36: Comment should be read "Implements hook_foo()."
 * Implementation of hook_init()
Line 38: use a space between the closing parenthesis and the open bracket
function closed_site_init(){
Line 40: Control statements should have one space between the control keyword and opening parenthesis
  if(!user_is_logged_in() && request_path() != 'access-denied-page' && variable_get('closed_site_close', 0) == 1){
Line 40: use a space between the closing parenthesis and the open bracket
  if(!user_is_logged_in() && request_path() != 'access-denied-page' && variable_get('closed_site_close', 0) == 1){
Line 42: Control statements should have one space between the control keyword and opening parenthesis
  }else if(request_path() == 'access-denied-page' && variable_get('closed_site_close', 0) == 0){
Line 42: use a space between the closing parenthesis and the open bracket
  }else if(request_path() == 'access-denied-page' && variable_get('closed_site_close', 0) == 0){
Line 42: else statements should begin on a new line
  }else if(request_path() == 'access-denied-page' && variable_get('closed_site_close', 0) == 0){
Line 50: use a space between the closing parenthesis and the open bracket
function closed_site_drupal_access_denied(){
Line 55: Control statements should have one space between the control keyword and opening parenthesis
  if($destination['destination'] == variable_get('site_frontpage', 'node')){
Line 55: use a space between the closing parenthesis and the open bracket
  if($destination['destination'] == variable_get('site_frontpage', 'node')){
Line 57: else statements should begin on a new line
  }else{

close_site.install

Line 14: Arrays should be formatted with a space separating each element and assignment operator
  db_query($sql, array(':module_name'=>'closed_site'));

Manual review

You are working in the master branch in git. You should move your code to a version specific branch.

drupal_add_http_header($_SERVER['SERVER_PROTOCOL'], ' 403 Forbidden');
Why not use drupal_access_denied()?

'access arguments' => array('access content'),
What if user will not have access content permission? He or she will not be able to log in. Otherwise user will always stay as logged out user even if have an active account. Very nasty situation, especially for unexperienced users who does not know structure of Drupal database to fix it.
This menu item setting should always be TRUE!

About an .install file...
Imagine I wrote a module called closed_site_backport not dependent on your module, just something new. When I will uninstall your module all of my variables will probably be uninstalled too because of the similar module name we share and Drupal naming conventions. variable_del() is a safer choice considering you use only three variables.

I believe this module is an overkill. It provide similar functionality to modules that already exists.

@ColonelForbinX
I'm sorry for getting into your way in reviewing this module. Just didn't notice while posting :)

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