By krazykanuk on

I am fairly new to php and drupal. I am wanting to create a module that will insert the registration data into a second database. From what I have been able to read there is two ways to do this (http://drupal.org/node/18429), I am leaning towards doing this on the fly for not everyone who installs a drupal module will have access or permissions to edit there sites/default/settings.php file. What I have is a settings page that requests information about the second database (name of database, username, password, host and port) that is saved into a table of the drupal database. What I have so far is a call to the database for each bit of information (5 calls to the database) what I am wondering is there a way I could make one call to the database and save each bit of information into a variable?

Also is there a way that you could give a message at the top of the admin/build/modules page once someone enables the module informing them to go to the settings page for the module and setting database information? I was looking at hook_load but I don't think that is what I need.

⋅ Categories: Drupal 6.x

Comments

jordojuice’s picture

jordojuice commented

Yes and yes... there is a way to do everything! But, it sure helps to see what code you already have. It's just too hard to speculate on what you're working with and much easier for us to get it if we see what's going on. : )

jordojuice’s picture

jordojuice commented

But to answer your second question which is easier, create a MYMODULE.install file. In that file do something like this:

/**
 * Implementation of hook_install().
 */
function MY_MODULE_install() {
  drupal_set_message(t('The module was successfully installed. Please visit <a href="!thispage">This Page</a>.', array('!thispage' => url('admin/settings/my_module'))), 'status');
}

'status' refers to the type of message that is set.

krazykanuk’s picture

krazykanuk commented

I have not yet created a submit button or validate function to write the information to the database, the way I have it now there is 5 functions and 5 calls to the database, if I can I would like only one call to the database to get the 5 pieces of information required, don't matter if it in 1 function or 5 or more.

freeradius.module file


// $Id$

/*
 @file
 Main module file for FreeRadius module.
*/

/* Setup multiple databases on the fly (want to minimize this put so there is only 1 call to the database and save the proper information into a variable for each of the 5 bits of information required */
$dbname = 'freeradius_get_database';
$dbuser = 'freeradius_get_user';
$dbpass = 'freeradius_get_pass';
$dbhost = 'freeradius_get_host';
$dbport = 'freeradius_get_port';
global $db_url;     /* The internal variable that contains the database link */
if (!is_array($db_url)) {
    $default_db = $db_url;
    $db_url = array('default' => $default_db);
    }
    /* Setup the new database value */
$db_url['freeradius'] = 'mysql://$dbuser:$dbpass@$dbhost/$dbname';

/* function to get the database name from the drupal database (freeradius table) */
function freeradius_get_database() {
    $result = db_query("SELECT dbname FROM {freeradius} WHERE dbname = '%s'", $dbname);
    return db_result($result);
}

/* function to get the username from the drupal database (freeradius table) */ 
function freeradius_get_user() {
    $result = db_query("SELECT dbuser FROM {freeradius} WHERE dbuser = '%s'", $dbuser);
    return db_result($result);
}

/* function to get the password from the drupal database (freeradius table) */
function freeradius_get_pass() {
    $result = db_query("SELECT dbpass FROM {freeradius} WHERE dbpass = '%s'", $dbpass);
    return db_result($result);
}

/* function to get the hostname from the drupal database (freeradius table) localhost is assumed if left blank */
function freeradius_get_host() {
    if ($result = db_query("SELECT dbhost FROM {freeradius} WHERE dbhost = '%s' AND != ''", $dbhost));
    else {
        $dbhost = localhost;
    }
    return db_result($result);
}

/* function to get the port from the drupal database (freeradius table) 3306 is assumed if left blank */
function freeradius_get_port() {
    if ($result = db_query("SELECT dbport FROM {freeradius} WHERE dbport = %d AND != ''", $dbport));
    else {
        $dbport = 3306;
    }
    return (int) db_result($result);
}

/*
 Implementation of hook_perm().

 This function takes 0 arguments, and returns an array of permissions
 defined by our module. Our permissions are automatically made available
 under admin/user/permissions.
*/

function freeradius_perm() {
    return array('administer freeradius','view freeradius');
}

/*
 Implementation of hook_user().
 This is where the logged in user will see data and time stats.
*/

/*function freeradius_user($op, &$edit, &$account, $category = NULL) {
    global $user;
    switch ($op) {
        case 'view':
            
            break;
        
    }
}*/

/*
 Implementation of hook_menu
*/
function freeradius_menu() {
    $items = array();
    
    $items['admin/settings/freeradius'] = array(
        'title' => 'FreeRadius Settings',
        'description' => 'Configuration options for the FreeRadius module.',
        'page callback' => 'freeradius_settings',
        'access callback' => 'user_access',
        'access arguments' => array('administer site configuration'),
    );
    $items['user/%user/usage-stats'] = array(
        'title' => t('Time & Data Usage'),
        'page callback' => 'freeradius_user_page',
        'page arguments' => array(1),
        'access callback' => 'freeradius_user_access',
        'access arguments' => array(1),
        'file' => 'freeradius.pages.inc',
        'type' => MENU_LOCAL_TASK,
        'weight' => -10,
    );
    return $items;
}

/* function that loads the settings form */
function freeradius_settings() {
    return drupal_get_form('freeradius_settings_form');
}

function freeradius_user_access($account) {
    global $user;
    if ($user->uid == $account->uid || user_access('view freeradius')) {
        return TRUE;
    }
    return FALSE;
}

/* function to load the settings form */
function freeradius_settings_form($form_state) {
    $form = array();
    
    $form['dbsettings'] = array(
        '#type' => 'fieldset',
        '#title' => 'Database Settings',
    );
    $form['dbsettings']['dbname'] = array(
        '#type' => 'textfield',
        '#title' => t('Database name'),
        '#description' => t('The database name that the FreeRadius will use. This database needs to already be created and set up by someone with permissions to do so.'),
        '#size' => 25,
    );
    $form['dbsettings']['dbuser'] = array(
        '#type' => 'textfield',
        '#title' => t('Database username'),
        '#description' => t('The database username that is required to connect to the FreeRadius database.'),
        '#size' => 25,
    );
    $form['dbsettings']['dbpass'] = array(
        '#type' => 'textfield',
        '#title' => t('Database password'),
        '#description' => t('The database password that is required to connect to the FreeRadius database.'),
        '#size' => 25,
    );
    $form['dbsettings']['dbhost'] = array(
        '#type' => 'textfield',
        '#title' => t('Database host'),
        '#description' => t('The database hostname that is required to connect to the FreeRadius database. The hostname localhost will be assumed if left blank.'),
        '#size' => 50,
    );
    $form['dbsettings']['dbport'] = array(
        '#type' => 'textfield',
        '#title' => t('Database Port'),
        '#description' => t('The database port that is required to connect to the FreeRadius database. The port 3306 will be assumed if left blank.'),
        '#size' => 50,
    );
    $form['radiussettings'] = array(
        '#type' => 'fieldset',
        '#title' => 'Radius/FreeRadius Settings',
    );
    $form['radiussettings']['enctype']['decision'] = array(
        '#type' => 'select',
        '#title' => t('Select the Encryption type you plan on using in your Radius/FreeRadius Server.'),
        '#options' => array('Cleartext-Passwords', 'MD5'),
        '#default_value' => 'Cleartext-Passwords',
    );
    $form['groupname'] = array(
        '#type' => 'fieldset',
        '#title' => 'GroupName',
    );
    $form['groupname']['grpname'] = array(
        '#type' => 'textfield',
        '#title' => t('GroupName'),
        '#description' => t('GroupName to be inserted into the database you defined above.'),
        '#size' => 25,
    );
    $form['radcheck'] = array(
        '#type' => 'fieldset',
        '#title' => 'Radcheck Values',
    );
    $form['radcheck']['attribute'] = array(
        '#type' => 'textfield',
        '#title' => t('Attribute'),
        '#size' => 25,
    );
    $form['radcheck']['radvalue'] = array(
        '#type' => 'textfield',
        '#title' => t('Value'),
        '#description' => t('The value can be either character,numeric or drupal variables to return information like $user-host.'),
        '#size' => 25,
    );
    $form['radcheck']['radop'] = array(
        '#type' => 'textfield',
        '#title' => t('Op'),
        '#description' => t('The operator used'),
        '#size' => 4,
    );
    return $form;

}

freeradius.install, the freeradius_schema was generated from the schema module, and checked if uninstall and install works, still need to edit description fields in the schema. Just recently added the database port and have not yet added it to the schema.


function freeradius_schema() {
    $schema = array();
    
$schema['freeradius'] = array(
  'description' => t('TODO: please describe this table!'),
  'fields' => array(
    'dbname' => array(
      'description' => t('TODO: please describe this field!'),
      'type' => 'varchar',
      'length' => '25',
      'not null' => TRUE,
    ),
    'dbuser' => array(
      'description' => t('TODO: please describe this field!'),
      'type' => 'varchar',
      'length' => '25',
      'not null' => TRUE,
    ),
    'dbpass' => array(
      'description' => t('TODO: please describe this field!'),
      'type' => 'varchar',
      'length' => '25',
      'not null' => TRUE,
    ),
    'dbhost' => array(
      'description' => t('TODO: please describe this field!'),
      'type' => 'varchar',
      'length' => '50',
      'not null' => TRUE,
    ),
    'dbport' => array(
      'description' => t('TODO: please describe this field!'),
      'type' => 'int',
      'not null' => TRUE,
    ),
  ),
  'primary key' => array('dbname'),
  );
  return $schema;
}

function freeradius_install() {
    drupal_install_schema('freeradius');
}

function freeradius_uninstall() {
    drupal_uninstall_schema('freeradius');
}
jordojuice’s picture

jordojuice commented

Okay, thanks.
So, generally when working with databases you want to have a Primary Key field that always has a unique value so you can reference a single row and get all the data from that row. Normally primary keys are integers and auto-increment. In your schema you can set it up as type 'serial' and that will auto-increment the field. Basically, any time you're planning to fetch specific data from a table you want to have a primary key or a single field with which you can retrieve all the records you need in one query. And even in cases where you need data from two tables you can use JOIN queries, though they're a little tough to pick up on at first. Anyways, with a primary key it would look something like:

function mymodule_get_settings($my_id) {
  $result = db_query("SELECT * FROM {mymodule} WHERE my_id = %d", $my_id);
  while ($setting = db_fetch_object($result)) {
    $settings[$setting->my_id] = $setting;
  }
  return $settings;
}

This simple function will pass you an array of the setting object where the Primary Key is the array key and the object with all of the fields is the value. If you know you're only fetching one row then you can use:

  $settings = db_fetch_object($result);

...instead of the while loop. Then, to use the data you just use the object like $settings->db_user, $settings->db_pass, etc. If you populate the array of several rows from the database as in the first example then you can use a foreach loop to output the data:

foreach ($settings as $setting) {
  $foo = $setting->bar;
}

etc.
Lastly, if you are using this form as a settings form have you consider system_settings_form()? It automatically stores variables based on field names, but considering what you are doing with the module it might not work for your purpose.
I hope this helps and I didn't miss what you were asking about! : ) If so then set me straight.

jordojuice’s picture

jordojuice commented 
/**
 * Get an array of global or default settings.
 *
 * @param $type
 *   A string identifying which array of settings to return.
 */
function autolink_get_settings($type) {
  switch ($type) {
    case 'global':
      $settings->title = variable_get('autolink_title', 1);
      $settings->content_types = variable_get('autolink_content_types', array());
      $settings->remove_links = variable_get('autolink_remove_links', 0);
      $settings->link_limit = variable_get('autolink_link_limit', 'all');
      $settings->link_to_self = variable_get('autolink_link_to_self', 0);
      return $settings;
    case 'default':
      $settings->case_sensitivity = variable_get('autolink_case_sensitivity', 1);
      $settings->default_content_types = variable_get('autolink_default_content_types', array());
      $settings->style = variable_get('autolink_style', '');
      $settings->class = variable_get('autolink_class', '');
      $settings->target = variable_get('autolink_target', 'none');
      return $settings;
  }
}

/**
 * Return a link object matching the link ID.
 *
 * @param $lid
 *   The link ID
 *
 * @return
 *   The link object with all of its metadata, if exists, FALSE otherwise.
 */
function autolink_link_load($lid) {
  $result = db_query('SELECT * FROM {autolink} a WHERE a.lid = %d', $lid);
  if ($link = db_fetch_object($result)) {
    $result = db_query('SELECT n.type, n.name FROM {node_type} n INNER JOIN {autolink_types} t ON n.type = t.node_type WHERE t.lid = %d', $lid);
    $link->types = array();
    while ($type = db_fetch_object($result)) {
      $link->types[$type->type] = $type->name;
    }
  }
  else {
    $link = FALSE;
  }
  return $link;
}

/**
 * Display form for adding and editing links.
 *
 * @ingroup forms
 * @see autolink_form_link_submit()
 */
function autolink_form_link(&$form_state, $edit = array()) {
  $keyword_disabled = (isset($edit['keyword']) ? TRUE : FALSE);
  $settings = autolink_get_settings('default');
  $globals = autolink_get_settings('global');
  $types = autolink_get_node_type_settings();
  if (isset($edit['lid'])) {
    $link = autolink_link_load($edit['lid']);
  }

  $edit += array(
    'link' => '',
    'description' => '',
  );
  $form['definition'] = array(
    '#type' => 'fieldset',
    '#title' => t('Definition'),
    '#collapsible' => TRUE,
  );
  $form['definition']['status'] = array(
    '#type' => 'select',
    '#title' => t('Status'),
    '#default_value' => isset($edit['status']) ? $edit['status'] : 1,
    '#options' => array(
      '0' => 'Blocked',
      '1' => 'Active',
    ),
  );

...

 $form['definition']['case_sensitive'] = array(
    '#type' => 'checkbox',
    '#title' => t('Case-sensitive'),
    '#default_value' => isset($edit['lid']) ? $edit['case_sensitive'] : $settings->case_sensitivity,
    '#description' => t('Keywords will only be replaced with links if they match the case of the keyword.'),
  );
  $form['nodes'] = array(
    '#type' => 'fieldset',
    '#title' => t('Content types'),
    '#collapsible' => TRUE,
    '#collapsed' => FALSE,
  );
  $form['nodes']['content_types'] = array(
    '#type' => 'checkboxes',
    '#title' => t('Select content types'),
    '#required' => TRUE,
    '#default_value' => isset($edit['lid']) ? array_keys($link->types) : $settings->default_content_types,
    '#options' => $types,
    '#description' => ($types == NULL ? t('You must enable automatic link ' .
                       'generation for at least one content type.') : NULL),
  );
  $form['submit'] = array('#type' => 'submit', '#value' => t('Save'));
  if (isset($edit['lid'])) {
    $form['delete'] = array('#type' => 'submit', '#value' => t('Delete'));
    $form['lid'] = array('#type' => 'value', '#value' => $edit['lid']);
  }

  return $form;
}

...and here is a system_settings_form...

/**
 * Get an array of node types for Autolink bulk operations.
 *
 * @return
 *   An associative array of node types where type is the key and type name is the value.
 */
function autolink_node_types() {
  $result = db_query('SELECT * FROM {node_type} ORDER BY name');
  while ($type = db_fetch_object($result)) {
    $types[$type->type] = $type->name;
  }
  return $types;
}

/**
 * Settings form for autolink.
 *
 * @ingroup forms
 * @see autolink_form_settings_submit()
 */
function autolink_form_settings(&$form_state, $edit = array()) {
  $types = autolink_get_node_type_settings();

  $form['global'] = array(
    '#type' => 'fieldset',
    '#title' => t('Global settings'),
    '#collapsible' => TRUE,
    '#description' => t('Global settings are used during the link building process. These options are ' .
                        'provided to allow site administrators more control over the location and ' .
                        'frequency of link generation.'),
  );
  $form['global']['autolink_title'] = array(
    '#type' => 'checkbox',
    '#title' => t('Generate title attribute from keywords'),
    '#default_value' => variable_get('autolink_title', 1),
    '#description' => t('If the title field of a link definition is empty then the title will be generated ' .
                        'from the link keywords.'),
  );
  $form['global']['autolink_content_types'] = array(
    '#type' => 'checkboxes',
    '#title' => t('Enable automatic link generation by content type'),
    '#options' => autolink_node_types(),
    '#default_value' => variable_get('autolink_content_types', array()),
    '#description' => t('If no content types are selected then all content types will be enabled.'),
  );

...

    
$form['default']['autolink_auto_generate'] = array(
    '#type' => 'checkbox',
    '#title' => t('Automatically generate links on save'),
    '#default_value' => variable_get('autolink_auto_generate', 1),
    '#description' => t('Links may be automatically generated when a link definition is saved.'),
  );

  return system_settings_form($form);
}

You can see that I use some functions to grab form data to populate the form. autolink_link_load is used to grab a single row from the database (and the query in that statement is a JOIN that grabs data from two tables simultaneously, so if we can do that five queries is definitely too much!), while autolink_node_types is used to get an array of node types from the nodes table. The system settings form, as shown, is very easy to use and will automatically set your 'variables' in the variables table which can be retrieved through variable_get() as I did in the first function.

Edit: Of course, this isn't all the code from that file so there may be some pieces missing but it's 700 lines and contains some irrelevant code so let me know if you have any questions.

krazykanuk’s picture

krazykanuk commented

I may be over thinking this (I tend to do this quite often), but to make sure I understanding this I will go through a few things. This started out to be a small project but the longer I work on this (and get to thinking) the more I think it be better if I did this.. how about adding this.. but this is getting dangerous for everything I think about (other then the simple stuff like adding it to a form (it added to form but the functions may not be written to make it work)) sends me either to google or drupal to figure out how to do it in php/drupal because of the inexperience I have in php and drupal, but I am getting off subject here. Things didn't go as planed and didn't get to work on this today but went through the post and making sure I understanding this before I continue.

Basically I have right under the opening php tags the following:


// $Id$

/*
 @file
 Main module file for FreeRadius module.
*/

/* Setup multiple databases on the fly */
$dbtype = '';
$dbname = '';
$dbuser = '';
$dbpass = '';
$dbhost = '';
$dbport = '';
global $db_url;     /* The internal variable that contains the database link */
if (!is_array($db_url)) {
    $default_db = $db_url;
    $db_url = array('default' => $default_db);
    }
    /* Setup the new database value */
$db_url['freeradius'] = 'mysql://$dbuser:$dbpass@$dbhost/$dbname';

this basically gets read when the module gets loaded and because the six $db* variables have not been set or saved on the settings page they all would return blank or NULL. So basically what I would need to do is create a variable (using your settings example) and add this above the other variables that would retrieve the whole table row for the freeradius table located in my drupal database

$settings = $result(db_query('SELECT * FROM {freeradius} WHERE id = %d', id);

The table information is one table I don't see a use for adding more (so the auto increment would have only incremented to 1) doing this I could take out the $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport variables above then when I needed the information like in the form '#default_value I would do:

'#default_value' => '$settings['dbtype']', // for the database type 
'#default_value' => '$settings['dbname']', // for the database name
etc...

This would be called from the form for the appropriate setting.

jordojuice’s picture

jordojuice commented

By the way, you'll see my code and commenting are different than yours. I know you're probably not concerned about this now, but just an FYI, Drupal has strict coding standards and the more you practice them the easier they come. Check out the coder module (http://drupal.org/project/coder) it will review your code for standards compliance and help you learn a lot.

krazykanuk’s picture

krazykanuk commented

Had family over and didn't have much time to look at it, but I did change the database schema to now have an ID field also added a database type field so it can be selected between mysql and postgres, as well as manually added information to the database (from mysql command line) so there as at least some information that can be retrieved. Will be working on the function to call the database and retrieve information in the morning. I did download and enable coder (using drush). I need to more carefully go through the posts and make sure I am understanding how to do this before I get too involved. Most of the comments were only added to the code after it was requested to see what I had.I know I had to go back and better comment it, but it is better that I use the drupal coding standards, should help as I progress. I Thank You for your help thus far and will report back any progress/pitfalls when I work on this in the morning.

jordojuice’s picture

jordojuice commented

Sounds good! When I first started working with Drupal and even now I live on the Drupal API site. You'd be amazed at the type of information you can get by referencing the API. I just look up function names, see which core modules call them, and then look at those files to get a better understanding of parameters and what not. Drupal core generally follows the standards better than any contrib module, so learning there is best. But I guess I'm a bit of a self learner. Make good use of Drupal functions you can do a lot with them and the code is already there!

krazykanuk’s picture

krazykanuk commented

It seems I was basically putting the chicken before the egg. I wanted to have a way that I could switch to the freeradius database on the fly, using the values of the form fields. At this point in development I don't need to switch databases on the fly (at least not until I can validate, submit and have the values filled in after the submit function has executed). I removed the code required to switch on the fly, reduced the form so it is only dealing with things related to the database (id, dbtype, dbname, dbuser, dbpass, dbhost, dbport), and am working on the validate and submit functions and need a way to pass information between the two without adding all six fields as global variables. Right now if I don't validate it, it works fine but if I validate it when it moves to the submit function which writes to the database it loses some of the values. Once I fix this I be moving back to making single call to the database to retrieve all the table information required. I thank you for you he;p and responses thus far.

jordojuice’s picture

jordojuice commented

Sounds like you're making good progress and learning a lot!
Generally you want to avoid setting global variables wherever you can. They should definitely only be used in certain situations. If possible, you need to pass variables between functions using parameters.

Here are some simple but common form, validation, and submit functions that look like what I might use.

/**
 * Display form.
 *
 * @ingroup forms
 * @see mymodule_form_example_submit()
 */
function mymodule_form_example(&$form_state, $edit = array()) {
  // Collapsible form fieldset.
  $form['example'] = array(
    '#type' => 'fieldset',
    '#title' => t('Definition'),
    '#collapsible' => TRUE,
  );
  // Simple select form element for status settings.
  $form['example']['status'] = array(
    '#type' => 'select',
    '#title' => t('Status'),
    '#default_value' => isset($edit['status']) ? $edit['status'] : 1,
    '#options' => array(
      '0' => 'None',
      '1' => 'Blocked',
      '2' => 'Active',
    ),
  );
  // Simple textfield for entering a keyword.
  $form['example']['keyword'] = array(
    '#type' => 'textfield',
    '#title' => t('Keyword'),
    '#default_value' => $edit['keyword'],
    '#description' => t('Please enter a keyword.'),
  );
  $form['submit'] = array('#type' => 'submit', '#value' => t('Save'));
}

...and validation...

/**
 * Validation handler for the sample form.
 *
 * @see mymodule_form_example()
 */
function mymodule_form_example_validate($form, &$form_state) {
  // Check to make sure the status is not 'none'.
  if ($form_state['values']['status'] == 0) {
    form_set_error('status', t('You must select a status.'));
  }
  // Check to make sure the keyword doesn't already exist with a module function.
  elseif (mymodule_keyword_exists($form_state['values']['keyword'])) {
    form_set_error('keyword', t('The keyword \'%keyword\' already exists.', array('%keyword' => $form_state['values']['keyword'])));
  }
}

...and submit...

/**
 * Form submit handler for example form.
 *
 * @see mymodule_form_example()
 */
function mymodule_form_example_submit($form, &$form_state) {
  // Pass the form values array to the save function.
  switch (mymodule_example_save($form_state['values'])) {
    case SAVED_NEW:
      // If the save function reports this example as saved new then set records
      // appropriately.
      drupal_set_message(t('Created new example for keyword \'%keyword\'.', array('%keyword' => $form_state['values']['keyword'])));
      watchdog('My module', 'Created new example for keyword \'%keyword\'.', array('%keyword' => $form_state['values']['keyword']), WATCHDOG_NOTICE, l(t('edit'), 'admin/settings/mymodule/edit/example));
      break;
    case SAVED_UPDATED:
      drupal_set_message(t('Updated example for keyword \'%keyword\'.', array('%keyword' => $form_state['values']['keyword'])));
      watchdog('My module', 'Updated example for keyword %keyword\'.', array('%keyword' => $form_state['values']['stored_keyword']), WATCHDOG_NOTICE, l(t('edit'), 'admin/settings/mymodule/edit/example));
      break;
  }
}

...and another function called by the submit function to update the database...

/**
 * Save an example.
 *
 * This is a really simple example of how to pass parameters from a submit function
 * to a function that will insert the data into the database. This function uses
 * drupal_write_record() to insert the data. In that case, form element names must
 * match the field names in the database table. Otherwise, you could use db_query()
 * to insert the records.
 *
 * @param $edit
 *   An associative array of form values to be saved.
 */
function mymodule_example_save(&$edit) {
  drupal_write_record('mymodule', $edit);
  // or: db_query("INSERT INTO {mymodule_table} ('status', 'keyword') VALUES ('%s', '%s')", $edit['status'], $edit['keyword']);
  $status = SAVED_NEW;

  // This status message will get sent back to the submit function by return and the
  // submit function will subsequently write the proper message based on the
  // status. I've eliminated the SAVED_UPDATED status from this function, but you
  // only would have to add a simple if statement to determine if the example form is
  // new or updated. This is where the Primary Key field of your database table
  // comes in. You would use the primary key as a way to determine whether records
  // already exist and thus if this example form is new or updating. You would pass
  // that Primary Key to the form through the url with hook_menu(). If the form finds
  // it with something like:
  // if (isset($edit['eid'])) {
  //   This form is being edited
  // }
  // else {
  //   This form is new
  // }

  return $status;
}
krazykanuk’s picture

krazykanuk commented

I wouldn't set a global variable, unless it was a last resort and researched other options FIRST. This module has got me stuck I believe it is the submit function, I have tried it three different ways. My current validate funtion is now like the following:


/**
 * Implementation of hook_form_validate
 */
function freeradius_settings_form_validate(&$form, &$form_state) {
    // Check the select list, if the value is less then 1 set it to equal mysql
    if ($form_state['values']['dbtype'] < 1) {
        $form_state['values']['dbtype'] = 'mysql';
    }
    // If the value is equal to 1 or greater set it to equal postgres
    else {
        $form_state['values']['dbtype'] = 'postgres';
    }
    // If the value for dbname is empty give a error showwing it required
    if ($form_state['values']['dbname'] = '') {
        form_set_error('dbname', t('The database name is a REQUIRED field.'));
    }
    // If the value for dbuser is empty give an error showing it is required
    if ($form_state['values']['dbuser'] = '') {
        form_set_error('dbuser', t('The database username is a REQUIRED field.'));
    }
    // If the value of dbpass is empty give an error showing it is required
    if ($form_state['values']['dbpass'] = '') {
        form_set_error('dbpass', t('The database password is a REQUIRED field.'));
    }
    // If the value of dbhost is empty set it to localhost
    if ($form_state['values']['dbhost'] = '') {
        $form_state['values']['dbhost'] = 'localhost';
    }
    // Currently there is NO validate for the dbport, dbport if empty should be set to 3306.
    // Would like to check that if there was a value entered it was numeric (!is_numeric)
    // but doing so will not allow the dbport to be left empty, need to think more on this.
}

This all seems to work, but with problems with the submit function included for completeness. The submit function I have tried three different ways the first would be:

submit function #1


/**
 * Submission function for freeradius_admin_settings.
 */
function freeradius_settings_form_submit(&$form, &$form_state) {
    $dbtype = ($form_state['values']['dbtype']);
    $dbname = ($form_state['values']['dbname']);
    $dbuser = ($form_state['values']['dbuser']);
    $dbpass = ($form_state['values']['dbpass']);
    $dbhost = ($form_state['values']['dbhost']);
    $dbport = ($form_state['values']['dbport']);
    $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d"
                       , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    if (db_affected_rows() == 0) {
        $result = db_query("INSERT INTO {freeradius}
                           (dbtype, dbname, dbuser, dbpass, dbhost, dbport)
                           VALUES('%s', '%s', '%s', '%s', '%s', %d)"
                           , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    }
}

Submit #1 produces NO ERROR either in the admin page or error log. But does not produce the required result in the freeradius table of the drupal database.

mysql> use moduledev;
Database changed
mysql> select * from freeradius;
+----+--------+--------+--------+--------+--------+--------+
| id | dbtype | dbname | dbuser | dbpass | dbhost | dbport |
+----+--------+--------+--------+--------+--------+--------+
|  0 |        |        |        |        | 3306   |      0 |
+----+--------+--------+--------+--------+--------+--------+
1 row in set (0.00 sec)

Submit #2


/**
 * Submission function for freeradius_admin_settings.
 */
function freeradius_settings_form_submit(&$form, &$form_state) {
    $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d"
                       , $form_state['values']['dbtype'], $form_state['values']['dbname'], $form_state['values']['dbuser'], $form_state['values']['dbpass'], $form_state['values']['dbhost'], $form_state['values']['dbport']);
    if (db_affected_rows() == 0) {
        $result = db_query("INSERT INTO {freeradius}
                           (dbtype, dbname, dbuser, dbpass, dbhost, dbport)
                           VALUES('%s', '%s', '%s', '%s', '%s', %d)"
                           , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    }
}

Submit #2 produces NO ERROR either in the admin page or error log. But does not produce the required result in the freeradius table of the drupal database.

mysql> use moduledev;
Database changed
mysql> select * from freeradius;
+----+--------+--------+--------+--------+--------+--------+
| id | dbtype | dbname | dbuser | dbpass | dbhost | dbport |
+----+--------+--------+--------+--------+--------+--------+
|  0 |        |        |        |        | 3306   |      0 |
+----+--------+--------+--------+--------+--------+--------+
1 row in set (0.00 sec)

Submit #3 


/**
 * Submission function for freeradius_admin_settings.
 */
function freeradius_settings_form_submit(&$form, &$form_state) {
    //$dbtype = ($form_state['values']['dbtype']);
    //$dbname = ($form_state['values']['dbname']);
    //$dbuser = ($form_state['values']['dbuser']);
    //$dbpass = ($form_state['values']['dbpass']);
    //$dbhost = ($form_state['values']['dbhost']);
    //$dbport = ($form_state['values']['dbport']);
    $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d"
                       , 1, mysql, radius_test, radius_user, radius_pass, localhost, 3306);
    if (db_affected_rows() == 0) {
        $result = db_query("INSERT INTO {freeradius}
                           (dbtype, dbname, dbuser, dbpass, dbhost, dbport)
                           VALUES('%s', '%s', '%s', '%s', '%s', %d)"
                           , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    }
}

Submit #3 works, does produce errors on the page but it basically because I am manually inserting the information into the module filling out the page and submitting it. This one is UPDATING the database as I expected the other 2 submit functions to do, it is almost as if it is not getting the $form_state['values']['whatever'] from the form.

mysql> select * from freeradius;
+----+--------+-------------+-------------+-------------+-----------+--------+
| id | dbtype | dbname      | dbuser      | dbpass      | dbhost    | dbport |
+----+--------+-------------+-------------+-------------+-----------+--------+
|  1 | mysql  | radius_test | radius_user | radius_pass | localhost |   3306 |
+----+--------+-------------+-------------+-------------+-----------+--------+
1 row in set (0.00 sec)
krazykanuk’s picture

krazykanuk commented

Using a dsm($dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport); both before and after the $result = db_query (for the update) only produces a mysql value so it is losing the rest of the values prior to the submit function being run. Is it my code or something I am doing that is loosing the values. I am working in Komodo IDE remotely I make changes save to the server then use devel module's empty cache before I enter my values and submit it. But still values are gone before it updates/inserts the values into the database.

jordojuice’s picture

jordojuice commented

The problem that I see with this is setting form values in your validate function.

Like here:

  // Check the select list, if the value is less then 1 set it to equal mysql
  if ($form_state['values']['dbtype'] < 1) {
    $form_state['values']['dbtype'] = 'mysql';
  }
  // If the value is equal to 1 or greater set it to equal postgres
  else {
    $form_state['values']['dbtype'] = 'postgres';
  }

Generally, I would recommend moving the if statements that set form values to your submit function and indeed would do that myself. Alternatively, if you want to set form values from your validate function you have to use form_set_value() (http://api.drupal.org/api/drupal/includes--form.inc/function/form_set_va...). Basically, the form_set_value() function is specifically made for setting values during the validate function because altered data in $form_state['values'] does not normally get passed on to the submit function, but form_set_value() is what gives you that option.

So you could try:

  // Check the select list, if the value is less then 1 set it to equal mysql
  if ($form_state['values']['dbtype'] < 1) {
    form_set_value('db_type', 'mysql', $form_state);
  }
  // If the value is equal to 1 or greater set it to equal postgres
  else {
    form_set_value('db_type', 'postgres', $form_state);
  }

... or just move that check to the submit function.
If form_set_value() doesn't work then I have seen that some people say you have to set the #parent property or something, but I haven't used form_set_value() enough myself. Check out the API that I linked for more information.

If I'm understanding you right, then I think this could be he solution to your problem!

krazykanuk’s picture

krazykanuk commented

I tried form_set_value in the validate function and gave me an error in the log:

[Thu May 19 22:25:44 2011] [error] [client xxx.xxx.xxx.xxx] PHP Fatal error:  Maximum execution time of 30 seconds exceeded in /var/www/devuser/htdocs/includes/common.inc on line 3845, referer: http://moduledev.dyndns.info/admin/settings/freeradius

I tried putting it in the submit function and then everything after the dbtype is blank. The way I was able to get it to work was comment out the whole validate function without the the checking in the submit function. The only thing is that it is put in database as 0 and 1 (because there only 2 values in the select list). But it works this way, BUT there is no checking that we are receiving information that we want. It is an admin page so only TRUSTED users are given this role, but I would still rather check. What I could try is leave it in the database as 0 and 1 and then convert it to either mysql or postgres once it is retrieved from the database? I am now at a point that I am being ordered to put this away for the night and move on to something else, so if I can't sneak back before I will pick it up in the morning. I do believe with your guidance I am making extreme progress as well as learning a lot.

jordojuice’s picture

jordojuice commented

That's great!

With select lists, which I think you are probably using for the options between postgres and MySQL right?
...with select lists or checkboxes or any non textfield, the insert function will store the KEY and not the VALUE of that field. So, if I have select options like:
'0' => 'postgres',
'1' => 'mysql',
... It will store either 0 or 1. Similarly, to set the default value of that field it would be based on the key, '0' or '1', not the value.

Alternatively you can make the key/value pair like 'postgres' => 'POSTGRES', 'mysql' => 'MySQL', and it will store 'postgres' or 'mysql', not the capital lettered versions. This allows you to store the unique keys for the selection rather than the proper values (which may have spaces and not underscores), making it often easier to work with the data later in your application.

So, if you pulled those zeros and ones out of the database, you could use them just like that as the default value for the database type field and it would work properly. Only when you want to display the data 'Postgres' or 'MySQL' will you need to convert it from zeros and ones to proper values.

This is similar to how it is done with content types. Usually the actually node type is the key, and the proper name is the value. However, the KEY or TYPE is what is unique in content types, NAMES are not properly formatted for PHP (with underscores) and so the TYPE is used to identify the content type while the NAME makes it easier for the user to make a selection. The TYPE is stored in the database table and used for programmatically identifying the content type. Only When data needs to be displayed to the user is the proper NAME of the content TYPE retrieved. Hope that makes sense I ramble too much.

About the form_set_value thing, I think that's the same thing that happened to me last time I tried to use it! I saw something about needing to set a #parent value on the form to get it to work properly, but I still have not tried it and generally have avoided it for now. The major Drupal module development book appears to have it wrong and it has caused some confusion.

jordojuice’s picture

jordojuice commented

Submit #1

/**
* Submission function for freeradius_admin_settings.
*/
function freeradius_settings_form_submit(&$form, &$form_state) {
  $dbtype = ($form_state['values']['dbtype']);
  $dbname = ($form_state['values']['dbname']);
  $dbuser = ($form_state['values']['dbuser']);
  $dbpass = ($form_state['values']['dbpass']);
  $dbhost = ($form_state['values']['dbhost']);
  $dbport = ($form_state['values']['dbport']);
  // In this first query you SET id = %d which is good, but that should mean that the first parameter will be an integer for the field ID. With no $id being set for the first parameter the function will likely confuse the next record ($dbtype) as the ID. What this needs is a method to check if the record is being UPDATED or INSERTED by checking if you have the ID in a variable and then react accordingly. Otherwise, you are running two queries unnecessarily as well. The way you would check whether it's updated is by setting a value field "id" on your form if it is being edited, but that requires some more hook_menu() elements as well.
  // What you could do also is use drupal_write_record('freeradius', $form_state['values']);
  // This will insert the data into the proper table fields based on the names of your form elements. But since your form elements already have the same names as your database fields this would likely work well.
  // If you were to UPDATE using this method it would be:
  // drupal_write_record('freeradius', $form_state['values'], 'id');
  $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d", $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
  if (db_affected_rows() == 0) {
  // This insert query looks good to me. Are you sure that your database fields are properly set up in hook_schema()? Have you been able to successfully insert data into those fields that are failing before?
  $result = db_query("INSERT INTO {freeradius} (dbtype, dbname, dbuser, dbpass, dbhost, dbport) VALUES ('%s', '%s', '%s', '%s', '%s', %d)", $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
  }
}
jordojuice’s picture

jordojuice commented

First, in submit #2 in your INSERT query, the placeholder values have not been defined. I don't know if it got cut off, but these variables:

$dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport

were not defined that I could see. That would definitely cause data loss.

Anyways, I rewrote submit #2 to how it might look if I were using it. This is much simpler and it only performs one query and supports the form being edited or being saved new.

/**
 * Submission function for freeradius_admin_settings.
 * Rewrite for how it might look with checking for an ID first.
 */
function freeradius_settings_form_submit($form, &$form_state) {
  if (isset($form_state['values']['id'])) {
    // This is an edited form.
    // 'freeradius' is the name of the table.
    // $form_state['values'] is the array that contains the values to be inserted:
    // Records are inserted where the array key is the same as the field name in the table.
    // 'id' is the name of the auto-increment field in the database. This is only used on updates.
    drupal_write_record('freeradius', $form_state['values'], 'id');
  }
  else {
    // This is a new record, react accordingly. See above for explanation.
    drupal_write_record('freeradius', $form_state['values']);
  }
}

And this would go at the top of the form for editing:

function freeradius_setting_form($form, $edit = array()) {
  // This is what would be added to the top of the form. This checks if an ID was passed
  // to the form by hook_menu(). You would still need to set up hook_menu() to pass
  // this value though.
  if (isset($edit['id'])) {
    $form['id'] = array('#type' => 'value', '#value' => $edit['id']);
  }
}

On top of this, you would need to set up a different hook_menu() entry that would load the data and populate the form if the settings are being edited and write a function to load the form values of the current edit.

However, this is only best if your database table is going to have multiple records. Is that the case? Or is this just a single settings form that will only insert one record in the database? If that is the case then it would be best to use variable_set()/variable_get() in my opinion.

krazykanuk’s picture

krazykanuk commented

I can change my form or database schema and php code to anything that would allow this to work. What my plan is that once I have the id (only needed for inserting and updating information to the database) dbtype, dbname, dbuser, dbpass (will be encrypted once I have this working), dbhost (localhost or external database), dbport (want to default to 3306 if none given). There is only the need for 1 row on the freeradius table of the drupal database. Once I have this information written into the freeradius table I can retrieve it to set up the second database on the fly. So this information will be used to set $db_uri. I don't even mind if I have to start over from scratch as long as I am using drupal/php best practices, learn something new on the way to final achievement and I eventually get there. I am sure there is many things I will learn doing this because of what I want it to do once the sections I am working are are complete and working properly, some of what I don't even know where to begin to look (like having an add button to add a set of fields and once the add has been pressed you then have an add and a delete). Not sure if this be considered a multistep form or multiple submit buttons but at this point that is over my head, I having enough trouble getting a single validate submit button to work and do as I expected it to do.

krazykanuk’s picture

krazykanuk commented

I have a few thoughts. I have been looking at the stalker module (basically because it is the module used as an example in the Lullabot Module Development video) and this module as well as some of your examples use variable_get/variable_set. This module uses variable_set values in the submit function and it gets or sends it's information in two functions either stalker_set_count or stalker_get_count which has the SELECT and INSERT/UPDATE sql commands. The problem I am seeing with this is they are setting a count on a page view (hook_user), with mine it would be when the submit button was pressed. With that said it seems silly (to me) to have the submit function call a freeradius_save_settings which would have the sql commands currently in the submit function and then when that done continue on using variable_set. The only thing I see as doing it this way I could set a '#default_value' => variable_get('dbname', '$dbname');. I might have to play with it so I get the desired results but that would get all my results with 1 database call wouldn't it or is there a database call for every variable_get('value1', 'value2')?

jordojuice’s picture

jordojuice commented

An advantage to setting Drupal variables is that variable_get() and variable_set() are not actually database queries from what I have read. The values are indeed stored in the database but I think they are cached or something, so calling variable_get() several times in a single form is not a performance issue. Indeed, it is often standard in dealing with variables to call the function numerous times on each page load and I have not seen any performance issues from it.

Alternatively, you can set default values in the same way by loading the data from the database when your form is loaded. This requires a separate menu callback that would need to call a function like freeradius_record_load() or something similar to retrieve the data from the database. I could provide an example of how this would be done if you need it. But, in my opinion if your table will only have one record in it then it would be much better to go the variable route. Anything more than one record and you will need to have a callback like this.

Edit: remember that you can create a form that will automatically set variables with system_setting_form(). This type of form needs no submit function or anything and it will set variables with the name of the fields. So if your field is named 'dbuser' then it will variable_set('dbuser', $form_state['values']['dbuser']) automatically. Considering this, you have to prefix your field names with your module name like 'freeradius_dbuser' so your variable does not conflict with another variable. In fact, anything that has a chance to conflict with other modules should be prefixed in this way, which I know you understand from looking at your code.

Another great website I've seen for Drupal tutorials is gotdrupal.com. I've recommended it to a lot of people for learning to use advanced Drupal modules, but I'm not sure how much into development it gets.

krazykanuk’s picture

krazykanuk commented

I have pretty much got this section completed. I used my own validate and submit functions switched to variable_get and variable_set, no idea what database table drupal uses when you use this but it wasn't the one I set up, but got it working so it is using mine. Everything seems to work even uninstalled the module and reinstalled it (using devel's reinstall module) and everything I am suppose to have in the database table is there. I even have the values filled in when you are returned to the form (other then the password (figured I would get the password characters) but this could be a security issue). Only minor thing left to do is have some text displayed to the user about what happened when they hit the save button and go back and heavily comment the module. The finished validate, submit and load functions follows:


/**
 * Implementation of hook_form_validate
 */
function freeradius_dbsettings_form_validate(&$form, &$form_state) {
    // If the value for dbname is empty give a error showwing it required
    if (is_null($form_state['values']['dbname'])) {
        form_set_error('dbname', t('The database name is a REQUIRED field.'));
    }

    // If the value for dbuser is empty give an error showing it is required
    if (is_null($form_state['values']['dbuser'])) {
        form_set_error('dbuser', t('The database username is a REQUIRED field.'));
    }
    // If the value of dbpass is empty give an error showing it is required
    if (is_null($form_state['values']['dbpass'])) {
        form_set_error('dbpass', t('The database password is a REQUIRED field.'));
    }
    // If the value of dbhost is not empty and it is numeric give an error saying it shouldn't be numeric
    if (!is_null($form_state['values']['dbhost']) && is_numeric($form_state['values']['dbhost'])) {
        form_set_error('dbhost', t('The value entered here should not be numeric.'));
    }
    // If the value of dbhost is left blank set it to localhost
    else {
        $form_state['values']['dbhost'] = 'localhost';
    }
    // Currently there is NO validate for the dbport, dbport if empty should be set to 3306.
    // Would like to check that if there was a value entered it was numeric (!is_numeric)
    // but doing so will not allow the dbport to be left empty, need to think more on this.
    if (!is_null($form_state['values']['dbport']) && !is_numeric($form_state['values']['dbport'])) {
        form_set_error('dbport', t('The value entered here should be numeric'));
    }
    else {
        $form_state['values']['dbport'] = '3306';
    }
}
 

/**
 * Submission function for freeradius_admin_settings.
 */
function freeradius_dbsettings_form_submit(&$form, &$form_state) {
    $dbtype = $form_state['values']['dbtype'];
    $dbname = $form_state['values']['dbname'];
    $dbuser = $form_state['values']['dbuser'];
    $dbpass = $form_state['values']['dbpass'];
    $dbhost = $form_state['values']['dbhost'];
    $dbport = $form_state['values']['dbport'];
    $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d"
                       , 1, $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    if (db_affected_rows() == 0) {
        $result = db_query("INSERT INTO {freeradius}
                           (dbtype, dbname, dbuser, dbpass, dbhost, dbport)
                           VALUES('%s', '%s', '%s', '%s', '%s', %d)"
                           , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    }
    variable_set('dbtype', $form_state['values']['dbtype']);
    variable_set('dbname', $form_state['values']['dbname']);
    variable_set('dbuser', $form_state['values']['dbuser']);
    variable_set('dbpass', $form_state['values']['dbpass']);
    variable_set('dbhost', $form_state['values']['dbhost']);
    variable_set('dbport', $form_state['values']['dbport']);
}

function freeradius_record_load() {
    variable_get('dbtype', 'dbtype');
    variable_get('dbname', 'dbname');
    variable_get('dbuser', 'dbuser');
    variable_get('dbpass', 'dbpass');
    variable_get('dbhost', 'dbhost');
    variable_get('dbport', 'dbport');
    $result = db_query("SELECT * FROM {freeradius} WHERE id = %d", 1);
}
jordojuice’s picture

jordojuice commented

That's great! I think I can say you've learned a lot on just this thread. It becomes so much easier once you learn the Drupal API too.

Just as an FYI, the data stored with variable_set() is serialized and stored in the 'variable' table. Serializing means you can store arrays with variable_set() as well, but it also means you can't just pull data straight from the variable table and display it. Besides, variable_get() is faster than a query like I said.

What you'll want to do to display a message is in your submit handler after data is done doing its thing it is normal to do something like:

  drupal_set_message(t('Did something with %something.', array('%something' => $form_state['values']['field_something'])));
  watchdog('Mymodule', 'Did something with %something.', array('%something' => $form_state['values']['field_something']), WATCHDOG_NOTICE);
  $form_state['redirect'] = 'admin/mymodule';

So, first you set the message that will be displayed to the user, "Did something with 'something'." This message also uses the % placeholder to display one of the form values in the message as you can see. That doesn't have to be done though.

Secondly, it updates the system records by telling watchdog what your module did. "Mymodule" should have the proper name of your module so it can record which module performed the action, and then this message similarly uses a placeholder to use some form data.

Lastly, it redirects the user after the form has been submitted and the records have been updated. Even with this redirection after the drupal_set_message() the user will still see the message on the page that they are redirected to.

I'm curious, in your validate function are the values that you set being saved through the submit function?
i.e.:

  // If the value of dbhost is left blank set it to localhost
  else {
    $form_state['values']['dbhost'] = 'localhost';
  }

I'm interested to better know the limitations on setting form values in validate functions. Originally, I would have thought that using an ampersand (&) in front of $form_state would allow you to set values (&$variable_name normally means the values set on that variable should be usable outside that function), but it didn't work for me.

It looks like you're doing great overall though! I'm glad I got to help.
Remember, with setting variables you can set default values in your form with those variables as well.

  $form['my_field'] = array(
    '#type' => 'textfield',
    '#title' => t('My textfield'),
    '#default_value' => variable_get('my_field', 'some default text'),
  );
  return $form;

Considering that, you don't need a load function to populate form defaults. But you likely will need it for other tasks.

krazykanuk’s picture

krazykanuk commented

What I was thinking on doing to display a message to user after they submitted the information. Like an if statement on the $result, if true set drupal_set_message('save_msg', t('Your configuration settings have been saved')); if false do same thing except say there was a problem saving the information and use drupal_set_error instead of drupal_set_message.

  // If the value of dbhost is left blank set it to localhost
  else {
    $form_state['values']['dbhost'] = 'localhost';
  }

As far as this, I would think it is being saved, but the only proof I have is the data is being saved to the drupal freeradius table. With default_value I basically set it to be blank if it first time on the form otherwise set to your previous value (with the load function).

I even got the database switching to work on the fly, I had to alter it but it working.

/**
 * Custom function to set up multiple databases on the fly
 * as explained at http://drupal.org/node/18429
 */
function freeradius_onthefly() {
    // The internal variable that contains the database link
    global $db_url;
    if (!is_array($db_url)) {
        $default_db = $db_url;
        $db_url = array('default' => $default_db);
    }
    // Set up the new database value
    $dbtype = variable_get('dbtype', 'dbtype');
    $dbname = variable_get('dbname', 'dbname');
    $dbuser= variable_get('dbuser', 'dbuser');
    $dbpass = variable_get('dbpass', 'dbpass');
    $dbhost = variable_get('dbhost', 'dbhost');
    $dbport = variable_get('dbport', 'dbport');
    if (is_null ($dbtype)) {
        form_set_error('dbtype', st('The database type is not configured. Please visit the <a href="!freeradius">Database Settings </a>page and configure the database settings.', array('!freeradius' => url('admin/freeradius/database'))), 'status');
    }
    elseif ($dbtype < 1) {
        $dbtype = 'mysql';
        }
        else {
            $dbtype = 'postgres';
        }
        
        $freeradiusdb = ("$dbtype://$dbuser:$dbpass@$dbhost/$dbname:$dbport");
        // With the $db_url array that sets up 'default' & 'freeradius' I can probably remove this array
        // that only sets the array as a single value.
        $db_url = array('freeradius' => $freeradiusdb);

         // What I had to add to get it to work without saying my site was offline!
        $db_url = array('default' => $default_db,'freefadius' => $freeradiusdb);
        // Set up the new database as the active database
        //db_set_active('freeradius');
        // perform sql queries on this database, from other functions
        // set it back to db_set_active('default'); when done.
}

// function to return the radcheck form
function freeradius_radreply_form() {
    global $db_url;
    freeradius_onthefly();
    db_set_active('freeradius');
    $form['dbsettings'] = array(
        '#type' => 'fieldset',
        '#title' => 'RadReply Settings',
    );
    dsm($db_url);
    db_set_active('default');
    /**$sql = 'SELECT Username, Attribute, op, value FROM {radcheck}';
    $limit = 20;
    $header = array(
        array('data' => t('Username'), 'field' => 'UserName', 'sort' => 'asc'),
        array('data' => t('Attribute'), 'field' => 'Attribute'),
        array('data' => t('Operator'), 'field' => 'op'),
        array('data' => t('Value'), 'field' => 'value'),
    );
    $tablesort = tablesort_sql(&$header);
    $result = pager_query($sql . $tablesort, $limit);
    $rows = array();
    while ($radiusdb = db_fetch_object($result)) {
        $rows[] = array(l($radiusdb->UserName), ($radiusdb->Attribute), ($radiusdb->op), ($radiusdb->value));
    }
    if (!rows) {
        $rows[] = array(array('data' => t('No user accounts created yet.'), 'colspan' =>4));
    }
    $output .= theme('table', $header, $rows);
    $output .= theme('pager', NULL, $limit, 0);
    return $output;
    */
    
    return $form;
}

What I should also mention here is both now and previously all forms were in freeradius.admin.inc and the other functions are in freeradius.module. Basically what is happening with this it is just basically showing the dsm($db_url) with 2 elements and a fieldset. I know the tablesort_sql is not working, that is what I am attempting to tackle to learn next. I needed to set up the database that it will use to be able to attempt it. I have an example link and a pdf file on my system that goes through this.

jordojuice’s picture

jordojuice commented

I'd say using a conditional to determine if the update was successful and displaying a message based on that is the right thing to do. Much better than just displaying success regardless because that may not be the case.

What I do to accomplish this is call another function from my submit like:

switch ($status = my_insert_function($form_state['values'])) {
  case 'success':
    // do something and show success message based on value of $status.
    break;
  case 'failed':
    // do something and display insert failed message if $status = 'failed'.
    break;
}

This is a good way to separate your functions into single purposes as they should be. If my_submit_function fails to insert records it returns 'failed' and the message is displayed accordingly.

krazykanuk’s picture

krazykanuk commented

I did something to really mess things up, in process of going through and commenting everything and get the insert function in it's own function. Trying to figure out what I broke (it writing dbtype, dbname, dbuser, dbpass, dbhost and 0 for dbport to the database). I thought I had it so it would switch databases on the fly (because dsm($db_url); had values for default and freeradius array) but when I try and switch and get a value from the second database it either gives an error that site is off line or produces error about the database (because values I retrieving are not in the drupal database). I thought I was doing good because I have 2 functions that gets a value 1 from drupal database and 1 from the database I want to switch to and puts the results in a select list, but since I broke things it doesn't retrieve the table information from the database. Guess this will teach me to do what I should be doing from the start and comment things as I go.

krazykanuk’s picture

krazykanuk commented

If I split the submit function up into 2 a submit and a insert function I get an error about parimeter 2 but if I put it all in one it works with no errors.


/**
* Submission function for freeradius_admin_settings.
*/

function freeradius_dbsettings_form_submit(&$form, &$form_state) {
    variable_get('values', 'dbsettings');
    $dbtype = $form_state['values']['dbtype'];
    $dbname = $form_state['values']['dbname'];
    $dbuser = $form_state['values']['dbuser'];
    $dbpass = $form_state['values']['dbpass'];
    $dbhost = $form_state['values']['dbhost'];
    $dbport = $form_state['values']['dbport'];
    $result = db_query("UPDATE {freeradius} SET id = %d, dbtype = '%s', dbname = '%s', dbuser = '%s', dbpass = '%s', dbhost = '%s', dbport = %d"
                       , 1, $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    if (db_affected_rows() == 0) {
        $result = db_query("INSERT INTO {freeradius}
                           (dbtype, dbname, dbuser, dbpass, dbhost, dbport)
                           VALUES('%s', '%s', '%s', '%s', '%s', %d)"
                           , $dbtype, $dbname, $dbuser, $dbpass, $dbhost, $dbport);
    }
    if ($result != TRUE) {
        $status = 'failed';
    }
    else {
        $status = 'success';
    }
    switch ($status) {
        case 'success':
            drupal_set_message(t('Your configuration has been saved.'));
            break;
        case 'failed':
            form_set_error('dbsettings', 'Error: Failed to save check that the database server is running.');
            break;
    }
    variable_set('dbtype', $form_state['values']['dbtype']);
    variable_set('dbname', $form_state['values']['dbname']);
    variable_set('dbuser', $form_state['values']['dbuser']);
    variable_set('dbpass', $form_state['values']['dbpass']);
    variable_set('dbhost', $form_state['values']['dbhost']);
    variable_set('dbport', $form_state['values']['dbport']);
}

Now that this is working I can try and figure out why it is taking my site off line if I try and use the $db_url in an array.

jordojuice’s picture

jordojuice commented

Awesome! Let me know if there's anything else you could use some help on. It looks like you're picking this up very quickly and learning by doing has always worked for me!

krazykanuk’s picture

krazykanuk commented

OK I think I figured out why database switching was not working. I put some code in to get the headers of a database (the non drupal database) and when I was trying to execute it, it would say my site was off line but I could back out clear cache and go to any other page. At first I thought it was how my module was connecting to the database but if so that was only partly true. I commented out all the code used to display information from the database, then used the $db_url('mydatabase'); and a drupal_set_message('using mydatabase'); under the $db_url array. I manually put my information into the $db_url array and it worked (I didn't use port like my module was) I put it back how it was and it gave an error again, took the port off and it is working, I just have to learn how to display field header and tablesort and so on to display the information I require.

krazykanuk’s picture

krazykanuk commented

Just a quick note on why the database switching wasn't working. Your normal $db_url in your settings.php file looks like this.

mysql | mysqli | pgsql://username:password@hostname/database

When I was adding the port I was adding it to the end after the database name. I not tested it, but I would hazzard a guess putting it after hostname might work.

mysql | mysqli | pgsql://username:password@hostname:port/database
jordojuice’s picture

jordojuice commented

Ahh yes!

krazykanuk’s picture

krazykanuk commented

One last quick question. I have some special characters in a select list ( := and +: ) When I hit save on the form tells me something about illegal operation, I tried escaping it with a \ but still same thing. I not sure if it php that complaining about it or mysql any ideas?

jordojuice’s picture

jordojuice commented

You should be able to use those as the values but they cannot be used as array keys I think.
So you can do:

$form['field'] = array(
  '#type' => 'select',
  '#options' => array(
    'equals' => '=',
    'plus' => '+',
  ),
);

but I don't think you can do '+' => '+' because '+' is not a valid array key, but #options is an array. That's what I have done in those situations.

krazykanuk’s picture

krazykanuk commented

The values would actually be := (colon equals) and :+ (colon plus) there others but using this to test it

jordojuice’s picture

jordojuice commented

Yeah but I'm just sayin regardless I don't think you can use those symbols as array keys that's all. You should be able to use anything as a value, but not a key. Which is why I put 'equals' => '=' instead of '=' => '=' or ':=' => ':=' or anything else where the key is just some symbols. Just pretty sure array keys cannot contain operators.

krazykanuk’s picture

krazykanuk commented

With more thought I may be able to come up with something more meaningful, but for now I have it like this and it seems to save the proper 0 or 1 value to the database and not give me an illegal operation error when I click the save button, so looks like I good as far as that goes.

>
$form['settings']['defaultop'] = array(
        '#type' => 'select',
        '#title' => 'Default Password Encryption Type',
        '#description' => 'This is the default operatorhing  used by your radius server not by drupal',
        '#options' => array('1 :=', '2 :+'),
        '#default_value' => '1 :=',

    );
jordojuice’s picture

jordojuice commented

Actually, what you have here will indeed save 0 or 1, but that is simply because you did not give the #options array any keys at all, resulting in numerical keys starting at zero. You have only given them values, '0 :=' and '1 :+'. Return values from select form elements will be keys. So you can force the return value as I showed above by setting the keys for the array. Arrays are like 'key' => 'value', 'key' => 'value'. If you just set 'value', 'value', 'value' then those three options will get the keys '0', '1', and '2'.
For instance:

$form['field'] = array(
  '#type' => 'select',
  '#options' => array(
    'equals' => '=',
    'plus' => '+',
  ),
);

This will return 'equals' if '=' is selected, and 'plus' if '+' is selected. Alternatively, if I just put:

'#options' => array(
  '=',
  '+',
),

Then it would return '0' if '=' is selected and '1' if '+' is selected.