? .cvsignore
? add_edit_moderate.patch
Index: userpoints.admin.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/userpoints/Attic/userpoints.admin.inc,v
retrieving revision 1.1.2.2
diff -u -p -r1.1.2.2 userpoints.admin.inc
--- userpoints.admin.inc	10 Aug 2010 08:26:26 -0000	1.1.2.2
+++ userpoints.admin.inc	30 Sep 2010 20:29:13 -0000
@@ -38,19 +38,36 @@ function userpoints_confirm_approve_subm
   $form_state['redirect'] = 'admin/config/people/userpoints/moderate';
 }
 
-
-function userpoints_admin_txn($form, &$form_state, $mode, $txn_id) {
+/**
+ * Form builder for add/edit userpoints transaction form.
+ */
+function userpoints_admin_txn($form, &$form_state, $mode, $txn_id = NULL) {
   global $user;
 
   $timestamp = format_date(REQUEST_TIME, 'custom', 'Y-m-d H:i O');
-
   if ($mode == 'edit' && $txn_id) {
     $txn = db_query('SELECT * FROM {userpoints_txn} WHERE txn_id = :txn_id', array(':txn_id' => $txn_id))->fetch();
+
+    if (!$txn) {
+      // Failed to load transaction, display not found page.
+      drupal_not_found();
+      drupal_exit();
+    }
+
     $timestamp = format_date($txn->time_stamp, 'custom', 'Y-m-d H:i O');
     $txn_user = user_load($txn->uid);
+
+    $form['txn'] = array(
+      '#type' => 'value',
+      '#value' => $txn,
+    );
   }
   elseif ($mode == 'add' && $txn_id) {
     $txn_user = user_load($txn_id);
+  } elseif ($mode == 'edit') {
+    // Edit but now transaction id given, display not found page.
+    drupal_not_found();
+    drupal_exit();
   }
 
   $form['txn_user'] = array(
@@ -59,9 +76,11 @@ function userpoints_admin_txn($form, &$f
       '#size' => 30,
       '#maxlength' => 60,
       '#default_value' => isset($txn_user) ? $txn_user->name : '',
-      '#autocomplete_path' => 'user/autocomplete',
-      '#description' => t('User Name for the user you want the !points to affect', userpoints_translation()),
+      '#autocomplete_path' => $mode == 'edit' ? NULL : 'user/autocomplete',
+      '#description' => t('The name of the user who should gain or lose !points.', userpoints_translation()),
       '#required' => TRUE,
+      '#weight' => -20,
+      '#disabled' => $mode == 'edit',
   );
 
   $form['points'] = array(
@@ -70,17 +89,43 @@ function userpoints_admin_txn($form, &$f
       '#size' => 10,
       '#maxlength' => 10,
       '#default_value' => isset($txn->points) ? $txn->points : 0,
-      '#description' => t('Number of !points to add/subtract from the user. For example, 25 (to add !points) or -25 (to subtract !points).', userpoints_translation()),
+      '#description' => t('The number of !points to add or subtract.  For example, enter %positive to add !points or %negative to deduct !points.', array('%positive' => 25, '%negative' => -25) + userpoints_translation()),
       '#required' => TRUE,
+      '#weight' => -15,
   );
 
+  if ($mode == 'add') {
+    $form['moderate'] = array(
+      '#title' => t('Moderated'),
+      '#type' => 'checkbox',
+      '#description' => t('If checked, points need to be moderated.'),
+      '#default_value' => variable_get(USERPOINTS_POINTS_MODERATION, 0),
+      '#access' => userpoints_admin_access('moderate'),
+      '#weight' => -10,
+    );
+  }
+  else {
+    $form['status'] = array(
+      '#title' => t('Approval status'),
+      '#type' => 'radios',
+      '#options' => userpoints_txn_status(),
+      '#description' => t('Approval status of the transaction.'),
+      '#default_value' => $txn->status,
+      '#access' => userpoints_admin_access('moderate'),
+      '#weight' => -10,
+    );
+  }
+
   $form['time_stamp'] = array(
-      '#type' => 'textfield',
-      '#title' => t('Date/Time'),
-      '#default_value' => $timestamp,
-      '#size' => 30,
-      '#maxlength' => 30,
-      '#description' => t('Date and time of this transaction, in the form YYYY-MM-DD HH:MM +ZZZZ'),
+    '#type' => 'textfield',
+    '#title' => t('Date/Time'),
+    '#default_value' => $timestamp,
+    '#size' => 30,
+    '#maxlength' => 30,
+    '#description' => t('The date and time recorded for this transaction. Use this format: YYYY-MM-DD HH:MM +ZZZZ.'),
+    '#weight' => -5,
+    // Do not show this if it is not allowed to change the timestamp anyway.
+    '#access' => !variable_get(USERPOINTS_TRANSACTION_TIMESTAMP, 1),
   );
 
   $expirydate = 0;
@@ -99,103 +144,68 @@ function userpoints_admin_txn($form, &$f
   $form['expirydate'] = array(
       '#type' => 'textfield',
       '#title' => t('Expiration date'),
-      '#default_value' => $expirydate,
+      '#default_value' => $expirydate ? $expirydate : '',
       '#size' => 30,
       '#maxlength' => 30,
-      '#description' => t('Date and time to expire these !points, in the form YYYY-MM-DD HH:MM +ZZZZ', userpoints_translation()) .
-      '<br />' . t('Leave blank for non-expiring !points', userpoints_translation()),
+      '#description' => t('The date and time that the !points should expire. Use this format: YYYY-MM-DD HH:MM +ZZZZ. Leave this field blank if the !points should never expire.', userpoints_translation()),
+      '#weight' => 25,
   );
   if (module_exists('taxonomy')) {
+    $options = userpoints_get_categories();
     $form['tid'] = array(
-        '#type' => 'select',
-        '#title' => t('Category'),
-        '#default_value' => variable_get(USERPOINTS_CATEGORY_DEFAULT_TID, 0),
-        '#options' => userpoints_get_categories(),
-        '#description' => t('Category to apply these !points to', userpoints_translation()),
+      '#type' => 'select',
+      '#title' => t('Category'),
+      '#default_value' => userpoints_get_default_tid(),
+      '#options' => $options,
+      '#description' => t('The !points category that should apply to this transaction.', userpoints_translation()),
+      '#weight' => 0,
+      // Only show the category if there are actually categories to choose from.
+      '#access' => count($options) > 1,
     );
   }
-
-  $form['reference'] = array(
+  
+  $form['operation'] = array(
       '#type' => 'textfield',
-      '#title' => t('Reference'),
-      '#default_value' => isset($txn->reference) ? $txn->reference : '',
-      '#size' => 30,
-      '#maxlength' => 128,
-      '#description' => t('Enter optional reference for this transaction. This field will be indexed and searchable.'),
+      '#title' => t('Operation'),
+      '#default_value' => isset($txn->operation) ? $txn->operation : t('admin'),
+      '#size' => 20,
+      '#maxlength' => 20,
+      '#description' => t('The operation type for this transaction (default is %admin).', array('%admin' => t('admin'))),
+      '#weight' => 5,
+      '#required' => FALSE,
   );
 
   $form['description'] = array(
       '#type' => 'textarea',
       '#title' => t('Description'),
       '#default_value' => isset($txn->description) ? $txn->description : '',
-      '#width' => 70,
-      '#lines' => 5,
+      '#rows' => 7,
+      '#cols' => 40,
       '#description' => t('Enter an optional description for this transaction, such as the reason it is created.'),
+      '#weight' => 10,
   );
 
+  $form['reference'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Reference'),
+      '#default_value' => isset($txn->reference) ? $txn->reference : '',
+      '#size' => 30,
+      '#maxlength' => 128,
+      '#description' => t('Enter an optional reference for this transaction.  This is for internal tracking and is not shown to the end user.'),
+      '#weight' => 15,
+  );
 
-  switch ($mode) {
-    case 'add':
-      $form['approver_uid'] = array(
-          '#type' => 'hidden',
-          '#value' => $user->uid,
-      );
-
-      $form['operation'] = array(
-          '#type' => 'hidden',
-          '#value' => 'admin',
-      );
-
-      $form['status'] = array(
-          '#type' => 'hidden',
-          '#value' => USERPOINTS_TXN_STATUS_PENDING,
-      );
-
-      $form['mode'] = array(
-          '#type' => 'hidden',
-          '#value' => $mode,
-      );
-      break;
-
-    case 'edit':
-
-      $form['txn_user']['#disabled'] = TRUE;
-      unset($form['txn_user']['#autocomplete_path']);
-
-      $form['txn_uid'] = array(
-          '#type' => 'value',
-          '#value' => $txn->uid,
-      );
-      $form['txn_id'] = array(
-          '#type' => 'value',
-          '#value' => $txn_id,
-      );
-      $form['approver_uid'] = array(
-          '#type' => 'textfield',
-          '#description' => t('Approver ID'),
-          '#default_value' => $txn->approver_uid,
-          '#size' => 10,
-          '#maxlength' => 7,
-          '#description' => t('The user ID of the person who approved this transaction. 0 means not yet approved.'),
-      );
-
-      $form['operation'] = array(
-          '#type' => 'textfield',
-          '#description' => t('Operation'),
-          '#default_value' => $txn->operation,
-          '#size' => 20,
-          '#maxlength' => 20,
-          '#description' => t('The operation type for this transaction. Normally, it is "admin".'),
-      );
-
-      $form['status'] = array(
-          '#title' => t('Approval status'),
-          '#type' => 'radios',
-          '#options' => userpoints_txn_status(),
-          '#description' => t('Approval status of the transaction.'),
-          '#default_value' => $txn->status,
-      );
-      break;
+  $approved_by = !empty($txn->approver_uid) ? user_load($txn->approver_uid) : NULL;
+  if ($approved_by) {
+    $form['approver'] = array(
+      '#type' => 'textfield',
+      '#title' => t('Approved by'),
+      '#default_value' => $approved_by->name,
+      '#size' => 30,
+      '#maxlength' => 30,
+      '#description' => t('The user ID of the person who approved this transaction. Empty means not yet approved.'),
+      '#weight' => 30,
+    );
   }
 
   $form['mode'] = array(
@@ -206,57 +216,87 @@ function userpoints_admin_txn($form, &$f
   $form['submit'] = array(
       '#type' => 'submit',
       '#value' => t('Save'),
+      '#weight' => 50,
   );
-
   return $form;
 }
 
-function userpoints_admin_txn_submit($form, &$form_state) {
-  if ($form_state['values']['form_id'] != 'userpoints_admin_txn') {
-    return;
-  }
-
+/**
+ * Validate function for userpoints transaction form.
+ */
+function userpoints_admin_txn_validate($form, &$form_state) {
   $txn_user = user_load_by_name($form_state['values']['txn_user']);
   if (!is_object($txn_user)) {
-    return;
+    form_set_error('txn_user', t('Specified user does not exist.'));
+  }
+  else {
+    form_set_value($form['txn_user'], $txn_user, $form_state);
   }
-  switch ($form_state['values']['mode']) {
-    case 'add':
-      $params = array(
-          'points' => $form_state['values']['points'],
-          'uid' => $txn_user->uid,
-          'operation' => 'admin',
-          'description' => $form_state['values']['description'],
-          'reference' => $form_state['values']['reference'],
-          'tid' => $form_state['values']['tid'],
-          'time_stamp' => strtotime($form_state['values']['time_stamp']),
-      );
-      if ($form_state['values']['expirydate']) {
-        // Check for the existence of an expirydate.
-        $params['expirydate'] = strtotime($form_state['values']['expirydate']);
-      }
-      userpoints_userpointsapi($params);
-      break;
 
-    case 'edit':
-      $expirydate = 0;
-      if (!empty($form_state['values']['expirydate'])) {
-        $expirydate = strtotime($form_state['values']['expirydate']);
+  if ((int)$form_state['values']['points'] == 0) {
+    form_set_error('points', t('Amount of !points must be a positive or negative number.', userpoints_translation()));
+  }
+
+  if (!strtotime($form_state['values']['time_stamp'])) {
+    form_set_error('time_stamp', t('The provided timestamp is not a valid date.'));
+  }
+
+}
+
+/**
+ * Submit function for userpoints transaction form.
+ */
+function userpoints_admin_txn_submit($form, &$form_state) {
+  global $user;
+  if ($form_state['values']['mode'] == 'add') {
+    $params = array(
+      'points' => $form_state['values']['points'],
+      'uid' => $form_state['values']['txn_user']->uid,
+      'operation' => $form_state['values']['operation'],
+      'description' => $form_state['values']['description'],
+      'reference' => $form_state['values']['reference'],
+      'tid' => $form_state['values']['tid'],
+      'time_stamp' => strtotime($form_state['values']['time_stamp']),
+      'moderate' => (bool)$form_state['values']['moderate'],
+      'approver_uid' => !$form_state['values']['moderate'] ? $user->uid : NULL,
+    );
+    if ($form_state['values']['expirydate']) {
+      // Check for the existence of an expirydate.
+      $params['expirydate'] = strtotime($form_state['values']['expirydate']);
+    }
+  }
+  else {
+    $expirydate = 0;
+    if (!empty($form_state['values']['expirydate'])) {
+      $expirydate = strtotime($form_state['values']['expirydate']);
+    }
+
+    $approver_uid = 0;
+    if ($form_state['values']['status'] == USERPOINTS_TXN_STATUS_APPROVED) {
+      // Use existing value if transaction already was approved, use new value is
+      // it wasn't.
+      if ($form_state['values']['txn']->status == USERPOINTS_TXN_STATUS_APPROVED) {
+        $approver_uid = $form_state['values']['txn']->approver_uid;
       }
-      $params = array(
-          'uid' => $form_state['values']['txn_uid'],
-          'approver_id' => $form_state['values']['approver_uid'],
-          'points' => $form_state['values']['points'],
-          'time_stamp' => strtotime($form_state['values']['time_stamp']),
-          'operation' => $form_state['values']['operation'],
-          'description' => $form_state['values']['description'],
-          'reference' => $form_state['values']['reference'],
-          'status' => $form_state['values']['status'],
-          'expirydate' => $expirydate,
-          'txn_id' => $form_state['values']['txn_id']
-      );
-      userpoints_userpointsapi($params);
+      else {
+        $approver_uid = $user->uid;
+      }
+    }
+
+    $params = array(
+      'uid' => $form_state['values']['txn']->uid,
+      'approver_uid' => $approver_uid,
+      'points' => $form_state['values']['points'],
+      'time_stamp' => strtotime($form_state['values']['time_stamp']),
+      'operation' => $form_state['values']['operation'],
+      'description' => $form_state['values']['description'],
+      'reference' => $form_state['values']['reference'],
+      'status' => $form_state['values']['status'],
+      'expirydate' => $expirydate,
+      'txn_id' => $form_state['values']['txn']->txn_id,
+    );
   }
+  userpoints_userpointsapi($params);
 
   $form_state['redirect'] = 'admin/config/people/userpoints';
 }
Index: userpoints.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/userpoints/userpoints.module,v
retrieving revision 1.68.2.9
diff -u -p -r1.68.2.9 userpoints.module
--- userpoints.module	16 Sep 2010 18:28:36 -0000	1.68.2.9
+++ userpoints.module	30 Sep 2010 20:29:17 -0000
@@ -75,6 +75,60 @@ function userpoints_help($path, $arg) {
 }
 
 /**
+ * Checks access for administrative functionality.
+ *
+ * Provides simplified access checks for the administrative permissions:
+ * - administer userpoints
+ * - add userpoints
+ * - edit userpoints
+ * - moderate userpoints
+ *
+ * @param $type
+ *   The access type to check. The administer permission has access to all of
+ *   them. Supported strings:
+ *   - list: Access to the userpoints list, default local task. All
+ *           administrative permissions have access to this.
+ *   - add: Permission to add new userpoint transactions.
+ *   - edit: Permission to edit existing userpoint transactions.
+ *   - moderate: Permission to approve/decline pending transactions.
+ *   - administer: Unlimited userpoints permissions, used for settings page.
+ *
+ * @return
+ *   TRUE if the current user has access, FALSE if not.
+ */
+function userpoints_admin_access($type = 'list') {
+  // Administer userpoints permission has full access.
+  if (user_access('administer userpoints')) {
+    return TRUE;
+  }
+
+  switch ($type) {
+    // All admin permissions have access to the list page.
+    case 'list':
+      return user_access('add userpoints') || user_access('edit userpoints') || user_access('moderate userpoints');
+      break;
+
+    case 'add':
+      return user_access('add userpoints');
+      break;
+
+    case 'edit':
+      return user_access('edit userpoints');
+      break;
+
+    case 'moderate':
+      return user_access('moderate userpoints');
+      break;
+
+    case 'administer':
+      // administer permission was already checked, this exists for
+      // documentation purposes only.
+      break;
+  }
+  return FALSE;
+}
+
+/**
  * Implements hook_menu().
  */
 function userpoints_menu() {
@@ -84,14 +138,16 @@ function userpoints_menu() {
       'title arguments' => userpoints_translation(),
       'description' => strtr('Manage !points', userpoints_translation()),
       'page callback' => 'userpoints_admin_points',
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('list'),
       'file' => 'userpoints.admin.inc',
   );
   $items['admin/config/people/userpoints/list'] = array(
       'title' => 'List',
       'title arguments' => userpoints_translation(),
       'description' => strtr('List users by !points', userpoints_translation()),
-      'access arguments' => array('view userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('list'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_DEFAULT_LOCAL_TASK,
       'weight' => -2,
@@ -101,7 +157,8 @@ function userpoints_menu() {
       'title arguments' => userpoints_translation(),
       'description' => strtr('Review !points in moderation', userpoints_translation()),
       'page callback' => 'userpoints_admin_manage',
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('moderate'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_LOCAL_TASK,
       'weight' => -1,
@@ -111,7 +168,8 @@ function userpoints_menu() {
       'description' => 'Admin add/delete userpoints',
       'page callback' => 'drupal_get_form',
       'page arguments' => array('userpoints_admin_txn', 4, 5),
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('add'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_LOCAL_TASK,
       'weight' => 0,
@@ -120,7 +178,8 @@ function userpoints_menu() {
       'title' => 'Edit',
       'page callback' => 'drupal_get_form',
       'page arguments' => array('userpoints_admin_txn', 4, 5),
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('edit'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_CALLBACK
   );
@@ -129,7 +188,8 @@ function userpoints_menu() {
       'title' => 'Approve Userpoints',
       'page callback' => 'userpoints_admin_approve',
       'page arguments' => array(4, 5),
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('moderate'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_CALLBACK
   );
@@ -138,7 +198,8 @@ function userpoints_menu() {
       'title arguments' => userpoints_translation(),
       'page callback' => 'userpoints_admin_approve',
       'page arguments' => array(4, 5),
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('moderate'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_CALLBACK
   );
@@ -149,7 +210,8 @@ function userpoints_menu() {
       'title arguments' => userpoints_translation(),
       'page callback' => 'drupal_get_form',
       'page arguments' => array('userpoints_admin_settings'),
-      'access arguments' => array('administer userpoints'),
+      'access callback' => 'userpoints_admin_access',
+      'access arguments' => array('administer'),
       'file' => 'userpoints.admin.inc',
       'type' => MENU_LOCAL_TASK,
       'weight' => 10,
@@ -188,25 +250,41 @@ function userpoints_menu() {
 function userpoints_access_my_points($uid = NULL) {
   global $user;
   if ($user->uid != $uid) {
-    return user_access('administer userpoints');
+    return user_admin_access('edit');
   }
   return (user_access('view userpoints') && user_is_logged_in()) || user_access('view own userpoints');
 }
 
 /**
- * Implements hook_perm().
+ * Implements hook_permission().
  */
 function userpoints_permission() {
   return array(
+    'view own userpoints' => array(
+      'title' => t('View own !points', userpoints_translation()),
+      'description' => t('Allows to view own !points, including own !point transactions.', userpoints_translation()),
+    ),
     'view userpoints' => array(
-      'title' => t('View all UserPoints'),
+      'title' => t('View all !points', userpoints_translation()),
+      'description' => t('Allows to view the !points of other users, but not the transactions.', userpoints_translation()),
     ),
-    'view own userpoints' => array(
-      'title' => t('View own UserPoints'),
+    'add userpoints' => array(
+      'title' => t('Add new !point transactions', userpoints_translation()),
+      'description' => t('Allows to create new !point transactions.', userpoints_translation()),
+    ),
+    'edit userpoints' => array(
+      'title' => t('Edit !point transactions', userpoints_translation()),
+      'description' => t('Allows to change existing !point transactions.', userpoints_translation()),
+    ),
+    'moderate userpoints' => array(
+      'title' => t('Moderate !point transactions', userpoints_translation()),
+      'description' => t('Allows to approve or disapprove !point transactions.', userpoints_translation()),
     ),
     'administer userpoints' => array(
-      'title' => t('Administer Userpoints')
-  ));
+      'title' => t('Administer Userpoints'),
+      'description' => t('Allows to configure the settings and includes full read and write access of all !point transactions.', userpoints_translation()),
+    ),
+  );
 }
 
 /**