? .cvsignore
? fix_edit_max_with_tests.patch
? userpoints.test
Index: userpoints.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/userpoints/userpoints.module,v
retrieving revision 1.68.2.23
diff -u -p -r1.68.2.23 userpoints.module
--- userpoints.module	22 Jan 2011 22:12:29 -0000	1.68.2.23
+++ userpoints.module	27 Jan 2011 18:44:04 -0000
@@ -450,7 +450,7 @@ function userpoints_get_max_points($uid 
   }
 
   // Check if a term id is passed as a parameter.
-  if (!$tid) {
+  if (!isset($tid)) {
     // It is not, so get the default term id.
     $tid = userpoints_get_default_tid();
   }
@@ -458,7 +458,7 @@ function userpoints_get_max_points($uid 
   // Check if we have already cached the maximum for the user/term combination on previous calls.
   if (!isset($max[$uid][$tid])) {
     // We did not cache it.
-    if ($tid == 'all') {
+    if ($tid === 'all') {
       // There is no term id, so we use "all".
       $max[$uid][$tid] = db_query('SELECT SUM(max_points) FROM {userpoints} WHERE uid = :uid', array(':uid' => $uid))->fetchField();
     }
@@ -656,7 +656,8 @@ function userpoints_userpointsapi($param
   // Call the _userpoints hook to allow modules to act after points are awarded.
   userpoints_invoke_all('points after', $params);
   return array(
-      'status' => TRUE,
+    'status' => TRUE,
+    'transaction' => $params,
   );
 }
 
@@ -728,14 +729,13 @@ function _userpoints_transaction(&$param
     $ret = drupal_write_record('userpoints_txn', $params, array('txn_id'));
     // Only update if the record has been successfully updated.
     if ($ret != FALSE) {
-      _userpoints_update_cache($params);
+      _userpoints_update_cache($params, $txn);
     }
   }
   else {
     // Create new transaction record.
     $ret = drupal_write_record('userpoints_txn', $params);
-    // Don't update cache if it's pending.
-    if ($params['status'] != USERPOINTS_TXN_STATUS_PENDING && $ret != FALSE) {
+    if ($ret != FALSE) {
       _userpoints_update_cache($params);
     }
   }
@@ -743,40 +743,95 @@ function _userpoints_transaction(&$param
 }
 
 /**
- * Update the caching table
- */
-function _userpoints_update_cache(&$params) {
-  if ($params['status'] != USERPOINTS_TXN_STATUS_APPROVED || $params['expired'] == 1) {
-    // Only update the cache for fully approved non-expired points.
-    return FALSE;
+ * Update the caching table.
+ *
+ * @param $params
+ *   Array with the transaction params.
+ * @param $txn
+ *   The original transaction, if this is an update.
+ */
+function _userpoints_update_cache($txn, $old_txn = NULL) {
+  // Store eventual updates in this array.
+  $updates = array();
+  if (!$old_txn) {
+    // For new transactions, only update the cache for fully approved non-expired
+    // points.
+    if ($txn['status'] == USERPOINTS_TXN_STATUS_APPROVED && $txn['expired'] != 1) {
+      // Calculate the current points based upon the tid.
+      $updates['points'] = $txn['points'] + userpoints_get_current_points($txn['uid'], $txn['tid']);
+      $max_points = userpoints_get_max_points($txn['uid'], $txn['tid']);
+      // If the new points are higher then the max, update the maximum.
+      if ($updates['points'] > $max_points) {
+        $updates['max_points'] = $updates['points'];
+      }
+    }
+  } else  {
+    // For existing transactions, it is a bit more complex.
+
+    // Expired transactions that were expired before can be ignored.
+    if ($txn['expired'] == 1 && $old_txn['expired'] == 1) {
+      return;
+    }
+
+    if ($old_txn['tid'] != $txn['tid']) {
+      // If the category has changed, remove the points of the old transaction
+      // from the old category.
+      $remove_points = userpoints_get_current_points($txn['uid'], $old_txn['tid']) - $old_txn['points'];
+      db_merge('userpoints')
+        ->key(array(
+          'uid' => $txn['uid'],
+          'tid' => (int) $old_txn['tid'],
+        ))
+        ->fields(array(
+          'points' => $remove_points,
+        ))
+        ->execute();
+
+      if ($txn['status'] == USERPOINTS_TXN_STATUS_APPROVED) {
+        // Make sure to add the points so that they are added to the new category.
+        $updates['points'] = userpoints_get_current_points($txn['uid'], $txn['tid']) +  $txn['points'];
+      }
+    }
+    else if ($old_txn['status'] == USERPOINTS_TXN_STATUS_APPROVED && $txn['status'] != USERPOINTS_TXN_STATUS_APPROVED) {
+      // If the transaction goes from approved to not approved, subtract the
+      // points to the total.
+      $updates['points'] = userpoints_get_current_points($txn['uid'], $txn['tid']) - $old_txn['points'];
+      $max_points = userpoints_get_max_points($txn['uid'], $txn['tid']);
+      // If the new points are higher then the max, update the maximum.
+      if ($updates['points'] > $max_points) {
+        $updates['max_points'] = $updates['points'];
+      }
+    }
+    else if ($txn['points'] != $old_txn['points'] && $old_txn['status'] == USERPOINTS_TXN_STATUS_APPROVED && $txn['status'] == USERPOINTS_TXN_STATUS_APPROVED) {
+      // If the category did not change but the points and the transaction
+      // was and still is approved, update the points difference.
+      $updates['points'] = userpoints_get_current_points($txn['uid'], $txn['tid']) + ($txn['points'] - $old_txn['points']);
+    }
+    elseif ($old_txn['status'] != USERPOINTS_TXN_STATUS_APPROVED && $txn['status'] == USERPOINTS_TXN_STATUS_APPROVED) {
+      // Calculate the current points based upon the tid.
+      $updates['points'] = userpoints_get_current_points($txn['uid'], $txn['tid']) + $txn['points'];
+    }
   }
-  if (!isset($params['tid'])) {
-    $params['tid'] = 0;
+  if (empty($updates)) {
+    return;
   }
 
-  // Calculate the current points based upon the tid.
-  $current_points = (int) $params['points'] + userpoints_get_current_points($params['uid'], $params['tid']);
-  // Grab the user's maximum points to preserve it.
-  $max_points = (int) db_query('SELECT max_points FROM {userpoints} WHERE uid = :uid AND tid = :tid',
-                  array(':uid' => $params['uid'], ':tid' => (int) $params['tid']))->fetchField();
-  if ($params['points'] > 0) {
-    // Points are greater than zero, update their max_points.
-    $max_points = (int) $params['points'] + (int) $max_points;
+  $max_points = userpoints_get_max_points($txn['uid'], $txn['tid']);
+  // If the new points are higher then the maximum, update it.
+  if ($updates['points'] > $max_points) {
+    $updates['max_points'] = $updates['points'];
   }
+  $updates['last_update'] = REQUEST_TIME;
 
-  // Insert or update the userpoints caching table with the user's current points.
+  // Insert or update the userpoints caching table with the user's current
+  // points.
   db_merge('userpoints')
-          ->key(array(
-              'uid' => $params['uid'],
-              'tid' => (int) $params['tid'],
-          ))
-          ->fields(array(
-              'points' => (int) $current_points,
-              'max_points' => (int) $max_points,
-              'last_update' => REQUEST_TIME,
-          ))
-          ->execute();
-  unset($params);
+    ->key(array(
+      'uid' => $txn['uid'],
+      'tid' => (int) $txn['tid'],
+    ))
+    ->fields($updates)
+    ->execute();
 }
 
 /**
@@ -1250,7 +1305,7 @@ function userpoints_views_api() {
  *   The operation being performed.
  * @param &$params
  *   Parameters to be passed to the hook.
- * 
+ *
  * @return
  *   An array of return values of the hook implementations. If modules return
  *   arrays from their implementations, those are merged into one array.
Index: tests/userpoints_api.test
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/userpoints/tests/Attic/userpoints_api.test,v
retrieving revision 1.1.4.3
diff -u -p -r1.1.4.3 userpoints_api.test
--- tests/userpoints_api.test	10 Oct 2010 13:30:20 -0000	1.1.4.3
+++ tests/userpoints_api.test	27 Jan 2011 18:44:08 -0000
@@ -7,7 +7,82 @@
  * @file
  * Contains test classes for userpoints module.
  */
-class UserpointsTestCase extends DrupalWebTestCase {
+
+
+/**
+ * Userpoints base test class with various helper functions.
+ */
+class UserpointsBaseTestCase extends DrupalWebTestCase {
+
+  /**
+   * Add points through the admin form.
+   *
+   * @param $points
+   *   Amount of points to add.
+   * @param $user
+   *   User object for which to grant points for.
+   * @param $total
+   *   Amount of points the user should have after the points have been added.
+   *   If not NULL, the confirmation string is checked and so is
+   *   userpoints_get_current_points().
+   *
+   * @return
+   *   The most recent transaction id, assuming that this belongs to this
+   *   transaction.
+   */
+  function addPoints($points, $user, $total = NULL) {
+    $edit = array(
+      'txn_user' => $user->name,
+      'points' => $points,
+    );
+    $this->drupalPost('admin/config/people/userpoints/add', $edit, t('Save'));
+    if ($total !== NULL) {
+      $categories = userpoints_get_categories();
+      $tid = userpoints_get_default_tid();
+      $category = $categories[$tid];
+      $this->assertText(t('@name just earned @points points and now has @total points in the @category category.', array('@name' => $user->name, '@points' => $points, '@total' => $total, '@category' => $category)), t('Correct confirmation message displayed.'));
+      $this->assertEqual($total, userpoints_get_current_points($user->uid, $tid), t('User has the correct total amount of points.'));
+    }
+    return db_query('SELECT MAX(txn_id) FROM {userpoints_txn} WHERE uid = :uid', array(':uid' => $user->uid))->fetchField();
+  }
+
+  /**
+   * Verify the current and optionally max points in a specific category.
+   *
+   * @param $uid
+   *   User uid for the user that needs to be tested.
+   * @param $current
+   *   The amount of points the user is currently supposed to have.
+   * @param $max
+   *   The amount of max points of the user. Only tested if not NULL.
+   * @param $tid
+   *   The category that needs to be checked. Default is used is none is
+   *   provided.
+   */
+  function verifyPoints($uid, $current, $max = NULL, $tid = NULL) {
+
+    // Check if a term id is passed as a parameter.
+    if (!$tid) {
+      // It is not, so get the default term id.
+      $tid = userpoints_get_default_tid();
+    }
+
+    debug('Current is: ' . userpoints_get_current_points($uid, $tid) . ', should be: ' . $current);
+    $this->assertEqual($current, userpoints_get_current_points($uid, $tid), t('Current points are correct.'));
+    if ($max !== NULL) {
+      // Hijack static cache, delete this item from it.
+      $max_cache = &drupal_static('userpoints_get_max_points', array());
+      unset($max_cache[$uid][$tid]);
+      debug('Max is: ' . userpoints_get_max_points($uid, $tid) . ', should be: ' . $max);
+      $this->assertEqual($max, userpoints_get_max_points($uid, $tid), t('Max points are correct.'));
+    }
+  }
+}
+
+/**
+ * API Tests.
+ */
+class UserpointsAPITestCase extends UserpointsBaseTestCase {
 
   private $admin_user;
   private $non_admin_user;
@@ -99,7 +174,7 @@ class UserpointsTestCase extends DrupalW
    */
   function testBasicCall() {
     global $user;
-    
+
     $points = (int) rand(1, 500);
     $sumpoints = $points;
 
@@ -131,10 +206,10 @@ class UserpointsTestCase extends DrupalW
       if (rand() & 1) {
         $points = - $points;
       }
-      if ($points > 0) {
-        $maxpoints = $maxpoints + $points;
-      }
       $sumpoints = $sumpoints + $points;
+      if ($sumpoints > $maxpoints) {
+        $maxpoints = $sumpoints;
+      }
       userpoints_userpointsapi($points);
     }
 
@@ -507,5 +582,221 @@ class UserpointsTestCase extends DrupalW
     $this->assertTrue(is_array($cats), 'Successfully verified userpoints_get_categories() returned an array');
   }
 
+
+  /**
+   * Test that editing points correctly updates the current and max points.
+   */
+  function testEditingTransactions() {
+    // First, add some points to two different categories.
+    $uid = $this->non_admin_user->uid;
+    $params = array(
+      'points' => 100,
+      'tid' => 0,
+      'uid' => $uid,
+    );
+    userpoints_userpointsapi($params);
+
+    $params = array(
+      'points' => 50,
+      'tid' => 1,
+      'uid' => $uid,
+    );
+    userpoints_userpointsapi($params);
+
+    // Add a third transaction that can be edited.
+    $params = array(
+      'points' => 5,
+      'tid' => 0,
+      'uid' => $uid,
+    );
+    $return = userpoints_userpointsapi($params);
+    $txn_id = $return['transaction']['txn_id'];
+
+    // Verify points up to this point.
+    $this->verifyPoints($uid, 105, 105, 0);
+    $this->verifyPoints($uid, 50, 50, 1);
+
+    // Now, edit the transaction. Mix any combination of point, category and
+    // status changes. After the change, verify current and max points.
+
+    // Points change.
+    $params = array(
+      'txn_id' => $txn_id,
+      'points' => -5,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 95, 105, 0);
+
+    // Change status to pending.
+    $params = array(
+      'txn_id' => $txn_id,
+      'status' => USERPOINTS_TXN_STATUS_PENDING,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 100, 105, 0);
+
+    // Change status back to approved.
+    $params = array(
+      'txn_id' => $txn_id,
+      'status' => USERPOINTS_TXN_STATUS_APPROVED,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 95, 105, 0);
+
+    // Change category.
+    $params = array(
+      'txn_id' => $txn_id,
+      'tid' => 1,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 100, 105, 0);
+    $this->verifyPoints($uid, 45, 50, 1);
+
+    // Change points and status.
+    $params = array(
+      'txn_id' => $txn_id,
+      'points' => 3,
+      'status' => USERPOINTS_TXN_STATUS_PENDING,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 100, 105, 0);
+    $this->verifyPoints($uid, 50, 50, 1);
+
+    // Change status back to approved.
+    $params = array(
+      'txn_id' => $txn_id,
+      'status' => USERPOINTS_TXN_STATUS_APPROVED,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 53, 53, 1);
+
+    // Change points and category.
+    $params = array(
+      'txn_id' => $txn_id,
+      'points' => 4,
+      'tid' => 0,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 104, 105, 0);
+    $this->verifyPoints($uid, 50, 53, 1);
+
+    // Change points and status and category.
+    $params = array(
+      'txn_id' => $txn_id,
+      'points' => 10,
+      'tid' => 1,
+      'status' => USERPOINTS_TXN_STATUS_DECLINED,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 100, 105, 0);
+    $this->verifyPoints($uid, 50, 53, 1);
+
+    // Change points and status back to approved.
+    $params = array(
+      'txn_id' => $txn_id,
+      'points' => 9,
+      'status' => USERPOINTS_TXN_STATUS_APPROVED,
+    );
+    userpoints_userpointsapi($params);
+    $this->verifyPoints($uid, 59, 59, 1);
+  }
+
 }
 
+/**
+ * Administration UI tests
+ */
+class UserpointsAdminTestCase extends UserpointsBaseTestCase {
+
+  private $admin_user;
+  private $non_admin_user;
+
+  /**
+   * Implements getInfo().
+   */
+  function getInfo() {
+    return array(
+        'name' => t('Userpoints Admin'),
+        'description' => t('Test various userpoints administration forms and listings.'),
+        'group' => t('Userpoints'),
+    );
+  }
+
+  /**
+   * Install userpoints module and create users.
+   */
+  function setUp() {
+    parent::setUp('userpoints');
+
+    // Create an administrator account and log in with that.
+    $this->admin_user = $this->drupalCreateUser(array('administer userpoints'));
+    $this->drupalLogin($this->admin_user);
+  }
+
+
+  function testAddEditPoints() {
+    $user = $this->drupalCreateUser();
+
+    $categories = userpoints_get_categories();
+    $tid = userpoints_get_default_tid();
+    $category = $categories[$tid];
+
+    // Grant some points with admin user.
+    $txn_id = $this->addPoints(10, $user, 10);
+
+    // Go to the listing page, verify that the user is shown.
+    $row = $this->xpath('//table/tbody/tr');
+    //$this->assertEqual(strip_tags((string)$row[0]->td[0]), t('@name (details)', array('@name' => $user->name)), t('User name with details link displayed.'));
+    $this->assertEqual((string)$row[0]->td[1], $category, t('Category correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[2], 10, t('Points correctly displayed.'));
+
+    // Go to the transaction listing page, verify that the transaction is shown.
+    $this->clickLink(t('Transactions'));
+    $row = $this->xpath('//table/tbody/tr');
+    $transaction = userpoints_transaction_load($txn_id);
+    $this->assertEqual((string)$row[0]->td[0], 10, t('Points correctly displayed.'));
+    //$this->assertEqual(strip_tags((string)$row[0]->td[1]), $user->name, t('User correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[2], format_date($transaction->time_stamp, 'small'), t('Date correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[3], 'admin', t('Reason correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[4], t('Approved'), t('Status correctly displayed.'));
+
+    $this->clickLink(t('edit'));
+
+    // Verify default values.
+    $this->assertFieldByName('points', 10);
+    $value = $this->xpath("//input[@name=:name and @disabled=:disabled]/@value", array(':name' => 'txn_user', ':disabled' => 'disabled'));
+    $this->assertEqual($value[0]['value'], $user->name, t('User field has the correct value and is disabled.'));
+    $this->assertFieldByName('approver', $this->admin_user->name);
+
+    $edit = array(
+      'points' => 7,
+      'operation' => $this->randomName(),
+      'description' => $this->randomName(),
+      'reference' => $this->randomName(),
+    );
+    $this->drupalPost(NULL, $edit, t('Save'));
+
+    // Verify that the transaction has been updated.
+    $this->assertEqual(7, userpoints_get_current_points($user->uid));
+    $row = $this->xpath('//table/tbody/tr');
+    $transaction = userpoints_transaction_load($transaction->txn_id);
+    $this->assertEqual((string)$row[0]->td[0], 7, t('Points correctly displayed.'));
+    //$this->assertEqual(strip_tags((string)$row[0]->td[1]), $user->name, t('User correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[2], format_date($transaction->time_stamp, 'small'), t('Date correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[3], $edit['description'], t('Reason correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[4], t('Approved'), t('Status correctly displayed.'));
+
+    // Go to the listing page, verify that the total points have been updated.
+    $this->clickLink(t('Totals'));
+    $row = $this->xpath('//table/tbody/tr');
+    //$this->assertEqual(strip_tags((string)$row[0]->td[0]), t('@name (details)', array('@name' => $user->name)), t('User name with details link displayed.'));
+    $this->assertEqual((string)$row[0]->td[1], $category, t('Category correctly displayed.'));
+    $this->assertEqual((string)$row[0]->td[2], 7, t('Points correctly displayed.'));
+
+    // View transaction details.
+    $this->clickLink(t('Transactions'));
+    $this->clickLink('view');
+
+
+  }
+}