minghui.yu [minghuiyu]

The previous application was #570056: minghui.yu [minghuiyu].

1. When a module needs to verify that some conditions are verified at runtime (which includes the value of a Drupal variable), it should implement hook_requirements()
2. The code uses <br /> in the description given for a form field when that is not necessary.
3. What I reported before about the functions to use is still valid. Is there a reason to not follow what said from the coding standards about the functions a module should use?

4.   $form['cwl_admin_cwlid'] = array(
       '#title' => t("Your site's super amdin's CWL ID"),
       '#description' => t("We suggest you to set this value to site super admin's CWL ID. Site super admin is the user whose user id is 1. Based on database record, this site's super admin's username is: %username and its CWL ID (if have) is: %adminid. Please make sure you enter the correct CWL ID from site super admin. Otherwise, someone whose CWL is the one you input here will gain unlimited access to your Drupal site", array('%username' => $user->name, '%adminid' => $userone_cwlid)),
       '#maxlength' => 8,
       '#size' => 10,
       '#type' => 'textfield',
       '#default_value' => $superadmin_cwlid,
     );
   

   The code is showing information about the user #1 without to verify if it is showing them to that user; I am not sure it is a good idea.

5.   $form['user_register'] = array(
       '#title' => t('User Registration Option'),
       '#type' => 'select',
       '#description' => t('Please choose one from the list. Please note: this option will disable "Require e-mail verification when a visitor creates an account" option. To adjust this setting, please go to admin/user/settings'),
       '#options' => array(2 => t('Visitors can create accounts but administrator approval is required.'), 0 => t('Only site administrators can create new user accounts'), 1 => t('Visitors can create accounts and no administrator approval is required')),
       '#default_value' => variable_get('user_register', 2),
     );
   

   The code is duplicating the settings already used by Drupal; should not the code just use the settings already set in a Drupal settings page?

6.   $uri = parse_url(check_url($form_state['values']['cwl_xmlrpc_path']));
   

   check_url(), which calls check_plain() should not be called, in this case; the function alters the URL, which could be not useful anymore.

See the section about Unicode string functions.
Generally speaking, a module should prefer a Drupal core function over PHP functions.

I think I have already said too much.

However, in university Drupal sites, it is not rare that user #1 is not the person who manages the site on a daily basis.

Bear in mind that a module present in Drupal.org needs to work for anybody.

To report the name of the user #1 is not a problem as that is available to anybody who has access to the user profiles (do you know who is the user #1 on Drupal.org? ;-))
I am not sure if it is a problem to show the other data, which is taken from the user profile data. The code should at least respect the settings for that field (which means that if I set that field to not be visible to anybody, the code should not show it to anybody).

Anyway, it is easier for a Drupal user to know if it is the super user, without any reminder. Just to make an example, do you think I am the super user?

If you think it's more correct to report to the currently logged user s/he is not the super user, why doesn't the code explicitly says to the user you are not the super user (or a similar sentence)?
A module should be able to verify if a user is the super user, should not it?

  if (xmlrpc_error()) {
    drupal_set_message('CWL communication error:'. t(xmlrpc_error_msg()), 'error');
    return FALSE;
  }

The string passed to drupal_set_message() is not translatable, and the first argument passed to t() needs to be a literal string, not a dynamic string.

You didn't upload the new code.

I would remove the phrase As of writing: using in the description of some form fields; it should add something more in the description.

The error messages shown from the validate function should end with a period.