blisstering [blisstering]

1.             '#description' => t('Should this group appear on the '. l('list of groups page', 'og') .' (requires OG Views module)? Disabled if the group is set to <em>private group</em>.'),
   

   l() is not used together t(); that is reported in the documentation page for t(), which reports the following text:

   Here is an example of incorrect usage of t():

   $output .= t('<p>Go to the @contact-page.</p>', array('@contact-page' => l(t('contact page'), 'contact')));
   

   Here is an example of t() used correctly:

   $output .= '<p>'. t('Go to the <a href="@contact-page">contact page</a>.', array('@contact-page' => url('contact'))) .'</p>';
   

2.   drupal_set_message('Your settings have been saved.'. $output);
   

   The string passed to the function is not translatable; if it would be translatable, then it should also use placeholders.

3.     form_set_error($element['#name'], t($element['#name'] .'Listing to directory and private group cannot exist simultaneously.'));
   

   The first argument of t() should be a literal string, not a dynamic value; differently, the script for extracting the strings to translate is not able to extract the string.

4.   foreach ($form_state['values'] as $key => $value) {
       if (strpos($key, '_og_') !== FALSE) {
         variable_set($key, $value);
       }
     }
     drupal_set_message('Your settings have been saved.');
   

   Code should use Drupal Unicode functions, which are able to handle multi-bytes strings.

5.                   $group_node = new stdClass();
                     $group_node->status = 1;
                     $group_node->uid = $node->uid;
                     $group_node->type = $group;
   
                     // assign node title using token replacement
                     $title = variable_get($node->type .'_'. $group .'_autocreategroup_og_title', '');
                     $title = str_replace('[node-title]', $node->title, $title);
                     $group_node->title = $title;
   
                     // assign description using token replacement
                     $description = variable_get($node->type .'_'. $group .'_autocreategroup_og_description', '');
                     $description = str_replace('[node-title]', $node->title, $description);
                     $group_node->og_description = $description;
   
                     // assign membership type
                     $membership_type = variable_get($node->type .'_'. $group .'_autocreategroup_og_selective', '');
                     $group_node->og_selective = $membership_type;
   
                     // assign directory listing
                     $directory_listing = variable_get($node->type .'_'. $group .'_autocreategroup_og_directory', '');
                     $group_node->og_directory = $directory_listing;
   
                     // enable group on registration, if necessary
                     $registration = variable_get($node->type .'_'. $group .'_autocreategroup_og_register', '');
                     $group_node->og_register = $registration;
   
                     // assign private groups, if necessary
                     $private_group = variable_get($node->type .'_'. $group .'_autocreategroup_og_private', '');
                     $group_node->og_private = $private_group;
   
                     $group_node->picture = $node->picture;
   
                     $group_node->data = $node->data;
   
                     // save it and give it the rest of the attributes
                     node_save($group_node);
   

   The code creates a node object, and saves it, but it doesn't allow to other modules to attach their own properties to the module being saved.

6.         $exists = ($value[$group] !== 0)? 1 : 0;
           switch ($exists) {
             case 1:
               variable_set($og_variables .'_autocreategroup_og_title', '[node-title] '. $value[$group]);
               variable_set($og_variables .'_autocreategroup_og_description', 'This '. $value[$group] .' is dedicated to [node-title].');
               variable_set($og_variables .'_autocreategroup_og_directory', 0);
               variable_set($og_variables .'_autocreategroup_og_selective', '0');
               variable_set($og_variables .'_autocreategroup_og_private', 0);
               variable_set($og_variables .'_autocreategroup_og_register', 0);
               break;
             case 0:
               variable_del($og_variables .'_autocreategroup_og_title');
               variable_del($og_variables .'_autocreategroup_og_description');
               variable_del($og_variables .'_autocreategroup_og_directory');
               variable_del($og_variables .'_autocreategroup_og_selective');
               variable_del($og_variables .'_autocreategroup_og_private');
               variable_del($og_variables .'_autocreategroup_og_register');
               break;
           }
   

   There is no need to use a SWITCH-statement when the possible values are always, and just two.\

7. function autocreategroup_settings($settings = 'general') {
     $content_types = node_get_types('types');
     $types = array_keys($content_types);
     if ($types !== FALSE) {
       if (is_array($types)) {
         $groups = 0;
         $omitted = 0;
         foreach ($types as $type) {
           // Calculate number of active and group content types
           $group_type = variable_get('og_content_type_usage_'. $type, '');
           switch ($group_type) {
             case '':
             case 'omitted':
               $omitted++;
               break;
             case 'group':
               $groups++;
               break;
           }
         }
   
         // If there are active and group content types create and render the general and advanced settings accordingly
         if ($groups > 0 && $omitted > 0) {
           switch ($settings) {
             case 'general':
               $output = drupal_get_form('autocreategroup_settings_general');
               break;
             case 'advanced':
               $output = drupal_get_form('autocreategroup_settings_advanced');
               break;
           }
         }
   
         // If there are no group content types display a message
         else if ($groups == 0) {
           $output = 'There are no group content types that can be automatically created. Click '. l('here', 'admin/content/types/add') .' to add a group content type. Click '. l('here', 'admin/content/types') .' to select from an existing list of content types and make it a group content type.';
         }
   
         //If there are no active content types, display a message
         else if ($omitted == 0) {
           $output = 'There are no content types for which you can automatically create groups. The content types that are neither group type nor can be posted in a group are the ones for which you can automatically create groups. Click '. l('here', 'admin/content/types/add') .' to add a content type. Click '. l('here', 'admin/content/types') .' to select from an existing list of content types.';
         }
       }
   
       // If there is only one content type, display a message
       else {
         $output = 'There are not enough content types to create groups automatically. To add more content types click '. l('here', 'admin/content/types/add') .'. Make sure you have at least one group node before trying to configure automatic creation of groups per content type.';
       }
     }
   
     // If there are no content types, display a message
     else {
       $output = 'Currently, there are no content types. Click '. l('here', 'admin/content/types/add') .' to add some content types. Make sure you have at least one group node before trying to configure automatic creation of groups per content type.';
     }
   
     return $output;
   }
   

   In case of errors, the function returns a not translatable string; it would be better to pass the error string to drupal_set_message().

Can You Elaborate More on Point No 5 & 7.Give Me an Exampleto understand it.

For point #5: Before to save the node, the code should invoke the implementations of hook_nodeapi() made from other modules (passing 'save' as first argument). It would then better if the code would invoke hook_nodeapi('load'), and hook_nodeapi('save').

For point #7: $output = 'There are not enough content types to create groups automatically. To add more content types click '. l('here', 'admin/content/types/add') .'. Make sure you have at least one group node before trying to configure automatic creation of groups per content type.'. The string is not translatable as it is not passed to t(); if then you are passing the return value of your function to t(), then it doesn't work because the first argument of t() must be a literal string, not a concatenation of strings, nor the value of a variable.

Please change only the status, when you upload new code; other metadata are not though to be changed by the applicant.

1.     'title' => 'AutoCreateGroup',
   

   A menu callback title should be something that communicate something to the user, see the menu titles used from Drupal, which are Content management, Site building, etc…

2.     'title' => 'General Settings',
   

   Strings used in user interface should have the first word in capital case, and the other words in lower case (with the exception of proper nouns, adjectives derived from proper nouns, and acronyms).

3.   if ($types !== FALSE) {
       if (is_array($types)) {
   

   It should enough to check if the variable contains an array, as a boolean value is not an array.

4.           case 'general':
               return $output = drupal_get_form('autocreategroup_settings_general');
               break;
             case 'advanced':
               return $output = drupal_get_form('autocreategroup_settings_advanced');
               break;
   

   The code saves in a variable the result of a function, which is then immediately returned to the caller.

5.       elseif ($groups == 0) {
            
   	 drupal_set_message('There are no group content types that can be automatically created. Click '. l('here', 'admin/content/types/add') .' to add a group content type. Click '. l('here', 'admin/content/types') .' to select from an existing list of content types and make it a group content type.', 'error', FALSE);
   	 return $output ;
         }
   
         //If there are no active content types, display a message
         elseif ($omitted == 0) {
           drupal_set_message('There are no content types for which you can automatically create groups. The content types that are neither group type nor can be posted in a group are the ones for which you can automatically create groups. Click '. l('here', 'admin/content/types/add') .' to add a content type. Click '. l('here', 'admin/content/types') .' to select from an existing list of content types.', 'error', FALSE);
   	return $output ;
         }
       }
   

   See the Drupal coding standards to understand how a module code should be written. In particular, see how the code should be formatted, and the character to use when indenting code, and the part about the namespace respect, which is also valid for the Drupal variables defined from the module.

6.         '#collapsible' => TRUE,
           '#collapsed' => FALSE,
           '#description' => t('Please click '. l('here', 'admin/settings/autocreategroup/general') .' and proceed to select group types that need to be automatically created for the active content types.'),
   

   l() is not used together t(); that is reported in the documentation page for t(), which reports the following text:

   Here is an example of incorrect usage of t():

   $output .= t('<p>Go to the @contact-page.</p>', array('@contact-page' => l(t('contact page'), 'contact')));
   

   Here is an example of t() used correctly:

   $output .= '<p>'. t('Go to the <a href="@contact-page">contact page</a>.', array('@contact-page' => url('contact'))) .'</p>';
   

7.   drupal_set_message(t('Your settings have been saved.' . $output));
   

   The first argument of t() is a literal string, not a dynamic value; differently, the script to extract the strings to translate will not extract the string.

8. function _autocreategroup_get_name($type) {
     $name = db_result(db_query("SELECT name FROM {node_type} WHERE type = '%s'", $type));
     return $name;
   }
   

   I think there is a Drupal function to get the name associated with a content type.

There have not been replies from the OP in the past 7 days. I am marking this report as won't fix.