Userpoints_badges for 7.x

I think the problem is that you're using new\ instead of new/, git then looks for a folder caled "new\userpoints_badges".

I'm on a Windows 7 machine. Using patch.exe for GnuWin32, which has worked for me before:

Hmm...  Looks like a unified diff to me...
(Stripping trailing CRs from patch.)
can't find file to patch at input line 4
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -up old\userpoints_badges/userpoints_badges.info new\userpoints_badges/use
rpoints_badges.info
|--- old\userpoints_badges/userpoints_badges.info       2011-07-08 00:44:32.0000
00000 -0400
|+++ new\userpoints_badges/userpoints_badges.info       2011-10-25 10:19:50.3906
25000 -0400
--------------------------
File to patch: 

userpoints_badges.info

Patching file userpoints_badges.info using Plan A...
Reversed (or previously applied) patch detected!  Assume -R? [n]

Yes, because you're on Windows and on Windows, \ is a path delimiter but on Unix (and the test bot is on Linux), it is perfectly valid to have a path with the name "a\b".

That's good to know when attempting to apply patches in general. Thank you, Berdir.

You don't need git, just do replace all "\" in the patch with "/" with your favorite text editor :)

Below is a first review of your changes. Note that I have quite high expectations/standards, especially compared to the sad state of most stuff in the 6.x branch of this project. Even though the review seems to be long, your efforts are very much appreciated, keep it coming! :)

+++ new\userpoints_badges/userpoints_badges.info	2011-10-25 10:19:50.390625000 -0400
@@ -3,11 +3,4 @@ description = Assign badges to users as 
 dependencies[] = userpoints

I've started a new userpoints 7.x-2.x branch with major API changes.

To avoid confusion, can you specify that this module is (for now) only compatible with 7.x-1.x by specifying the major version like the following example: "dependencies[] = exampleapi (1.x)". See http://drupal.org/node/542202 for more information.

This is something I need to do for all other modules too.

+++ new\userpoints_badges/userpoints_badges.info	2011-10-25 10:19:50.390625000 -0400
@@ -3,11 +3,4 @@ description = Assign badges to users as 
-datestamp = "1310085871"
-

Let's add a "configure = admin/config/people/userpoints/settings" then a link to the configuration page is displayed directly on the modules pages.

+++ new\userpoints_badges/userpoints_badges.install	2011-10-25 10:22:26.187500000 -0400
@@ -29,10 +33,8 @@ function userpoints_badges_schema() {
 function userpoints_badges_install() {
-  drupal_install_schema('userpoints_badges');
 }
 
 function userpoints_badges_uninstall() {
-  drupal_uninstall_schema('userpoints_badges');

No need to keep empty functions, just drop the _install implementation completely.

The _uninstall() hook should use variable_del() to delete all variables which are used by it. This doesn't work very well for dynamic ones, so you might need to do a db_delete() query directly.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -1,47 +1,72 @@
+ * Implements  hook_menu().

Nitpicking, two spaces after Implements, same in other docblocks.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -1,47 +1,72 @@
-      $userpoints_goal = db_result(db_query("SELECT userpoints_goal FROM {userpoints_badges} WHERE bid=%d", $form['bid']['#value']));
+      $userpoints_goal = db_select('userpoints_badges', 'ub')
+        ->fields('ub', array('userpoints_goal'))
+        ->condition('bid', $form['bid']['#value'])
+        ->execute()

It is not necessary to convert simple queries like this one to db_select(). See http://drupal.stackexchange.com/questions/1200/given-that-db-select-is-m... for an explanation why and when db_select() needs to be used.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -1,47 +1,72 @@
-      '#description' => t('Enter the minimum number of points a user must have to get this badge'),
+      '#description' => t('Enter the minimum number of points a user must have

I am not sure about breaking translated sentences into multiple lines, is this a new coding standard?

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -1,47 +1,72 @@
-  else if ('user_badges_delete_form' === $form_id) {
-    $form['#submit'][] = 'userpoints_badges_delete_badge_submit';
+  else {
+    if ('user_badges_delete_form' === $form_id) {
+      $form['#submit'][] = 'userpoints_badges_delete_badge_submit';
+    }

You should instead use elseif, easier than another nested condition.

Also, might be worth to check if userpoints_badges 7.x provides any hooks for when badges are deleted, so that we can do this in a hook implementation. Uhm, looking at the module's code, there isn't a single module_invoke() in there, sad panda :(

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -1,47 +1,72 @@
-    $sql = db_query('SELECT b.bid, b.weight, b.name, b.image, u.userpoints_goal FROM {user_badges_badges} b INNER JOIN {userpoints_badges} u ON b.bid=u.bid ORDER BY b.weight, b.name');
-    while ($badge = db_fetch_object($sql)) {
-      $userpoints_badges_list['select']["badge-$badge->bid"] = $badge->name;
+    $result = db_select('user_badges_badges', 'b')
+      ->fields('b', array('bid', 'weight', 'name', 'image'))
+      ->fields('u', array('userpoints_goal'))
+      ->join('userpoints_badges', 'u', 'b.bid = u.bid')
+      ->orderBy('b.weight', 'asc')
+      ->orderBy('b.name', 'asc')
+      ->execute();
+    foreach ($result as $badge) {
+      $userpoints_badges_list['select']["badge-$badge->bid"] = check_plain($badge->name);

Same here, as there are not even placeholders in this query, it should work as-is you just need to replace the while with a foreach loop. I agree on using $result instead of $sql, though.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -67,40 +92,57 @@ function userpoints_badges_edit_badge_su
+  $resultset = db_query("SELECT uid FROM {userpoints} WHERE uid > 0
+    GROUP BY uid HAVING SUM(points) > :goal",
+    array(':goal' => $userpoints_goal));
+  foreach ($resultset as $account) {
+    user_badges_user_add_badge($account->uid, $bid,

If you want a total per user over all categories, you can query the new userpoints_total table. That avoids the need for the GROUP BY. Also, looks like this module doesn't support userpoints categories yet. Which is fine for now, although something I try to support everyhwere properly.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -108,17 +150,21 @@ function userpoints_badges_userpoints($o
+        '#value' => t('Go to <a href="!link">Userpoints Badges Settings</a> to edit userpoint badges', array('!link' => url('admin/people/user_badges')))

I guess this is based on #1306032: Cumulative D7 Changes. Since you're working on that too, I suggest you change it to admin/config/people/user_badges, because that's the place for user related configuration IMHO.

+++ new\userpoints_badges/userpoints_badges.module	2011-10-25 11:05:58.218750000 -0400
@@ -108,17 +150,21 @@ function userpoints_badges_userpoints($o
+      // Remove all existing badges for this user.
+      db_delete('user_badges_user')
+        ->condition('uid', $params['uid'])
+        ->condition('type', 'Userpoints%', 'like')
+        ->execute();

Not sure if the lower case like works since there are some defined special cases for this for PostgreSQL support. For consistency, let's use the uppper case version :)

Ok, two generic problems with the patch format remaining.

- Windows style file endings. The test bot doesn't like these nor does git.

- You are patching the -dev snapshot which contains the project/build information in the .info file. But the patch is applied against a git checkout which doesn't. After manually removing these parts (which is tricky), I was able to apply your patch successfully.

I suggest you look into learning some git basics sooner or later because you just don't have problems like this with git diff. I gladly provide any assistance you need, you just need to ask. It's really not that complicated :)

For reference, attaching the altered patch file that finally worked for me, this one should get past the bot (which is kinda pointless as there are no tests ;))

About People/Configuration. It doesn't need to be like Drupal 6. But it should follow the UX Guidelines/Information Architecture of Drupal 7. Have a look at http://drupal.org/node/549094#people.

I don't know Git GUI's in detail (Although I've heard that EGit which is a Eclipse plugin is pretty good), but I can still help to explain Git in general. I can also recommend #drupal-gitsupport if there are questions.

userpoints_badges_7.x.patch queued for re-testing.

There are no tests, this is not going to get any better than that message :)

Berdir,

I can't make your patch work with TortoiseSVN on Windows.

I had installed following userpoints_contrib module:

http://ftp.drupal.org/files/projects/userpoints_contrib-7.x-1.x-dev.tar.gz

I am getting following error when TortoiseMerge is run:

'C:\xampp\htdocs\drupal7\sites\all\modules\userpoints_contrib\userpoints_badges' is not a working copy.

No idea about that error, but as I said, my patch is against the git checkout, it doesn't apply against the -dev snapshot.

That error is because I have not checked out first. I don't get it, can't I apply patches to local copies only, without any version control repository involved?

In theory yes, you can.

This case differs because the .info file is changed when the package is built, so it is not the same file.

Try the patch from NancyDru, that is against the snapshot.

I have directly used the userpoints_badges.zip from NancyDru.

It works better than the untouched 6.x version.

Here is my report as of now:

EDIT:

- Cannot edit a badge: Trying to edit a badge, although the URL seems to be correct (program/admin/people/user_badges/edit/2), it takes me to People page.

ADD:

- Notice when add is clicked: "Notice: Undefined variable: userpoints_goal in userpoints_badges_form_alter() (line 29 of C:\xampp\htdocs\drupal\sites\all\modules\userpoints_contrib\userpoints_badges\userpoints_badges.module)."

- There are two form buttons named (Save Badge) at the bottom.

LIST:

- Can we put Userpoints Goal as a column?

Have a look at http://blog.worldempire.ch/story/writing-automated-tests-drupal-7 and check out the existing UserpointsTests in userpoints.test.

Basically, you will need to implement what you would do in a manual test in PHP with the methods provided by Simpletest. However, writing tests takes quite some time and it might might make more sense to write tests for User Badges itself first. Then it would be easy to adapt and extend those with stuff added by this module.

Not having tests is IMHO fine for the moment, just ignore the testbot for now. You can use a something-d7.patch name, then the test bot will ignore but I can still do a manual review.

Ah sorry, misunderstood you. A test case for me is something related to Simpletest :)

I think all you need is at admin/config/people/userpoints. You can see how many points each user has, the transactions, grant/remove points for a user and configure the settings.

Not sure, but my suggestion is to fix user_badges first and then work on UP/User Badges integration :)

From #871114: Drupal 7 port (userpoints_invite), let's not add versioned dependencies until it works for dev snapshots and git checkouts..

Just curious, should interested parties assume that a port to Drupal 7 is indefinitely postponed? Was hoping to utilize points with badges for an upcoming project being developed in D7.

The zip file in #26 worked well against the dev branch.

It did raise an error when the user_badges-edit_form came up as part of creating a NEW badge, and userpoints_goal was not yet found in the dB, and thus not properly initialized. I added an extra line to initialize the varialbe and the error went away.

function userpoints_badges_form_alter(&$form, &$form_state, $form_id) {
  if ('user_badges_edit_form' === $form_id) {
    $userpoints_goal = '';    
    if (isset($form['bid']) && isset($form['bid']['#value'])) { ....

Still appears to be a problem (new?). We were getting an error when submitting comments:

Fatal error: Call to a member function orderBy() on a non-object in .../userpoints_contrib/userpoints_badges/userpoints_badges.module on line 68

    $result = db_select('user_badges_badges', 'b')
    ->fields('b', array('bid', 'weight', 'name', 'image'))
    ->fields('u', array('userpoints_goal'))
    ->join ('userpoints_badges', 'u', 'b.bid = u.bid')
    ->orderBy('b.weight', 'asc')
    ->orderBy('b.name', 'asc')
    ->execute();

It appears that the join in that statement destroys the object.

I then found this comment in a discussion on db_select:
http://api.drupal.org/api/drupal/includes!database!database.inc/function...

Following the example, I rewrote the statement as shown here, and this appears to work fine:

    $result = db_select('user_badges_badges', 'b');
    $result->join('userpoints_badges', 'u', 'b.bid = u.bid');
    $result->fields('b', array('bid', 'weight', 'name', 'image'));
    $result->fields('u', array('userpoints_goal'));
    $result->orderBy('b.weight', 'asc');
    $result->orderBy('b.name', 'asc');
    $result->execute();

Here is the output from dpq($result), to check the SQL that is generated:

SELECT b.bid AS bid, b.weight AS weight, b.name AS name, b.image AS image, u.userpoints_goal AS userpoints_goal
FROM 
{user_badges_badges} b
INNER JOIN {userpoints_badges} u ON b.bid = u.bid
ORDER BY b.weight asc, b.name asc

..so changing this will make it work?
Im really keen on using this module :D

I can confirm that the fix in #39 fixes the error about the non-object.

I have to do some more testing but will post again to confirm functionality.

I have found an error while adding the badges.

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '))' at line 1: DELETE FROM {userpoints_badges} WHERE (bid IN ()) ; Array ( ) in userpoints_badges_edit_badge_submit() (line 106 of /sites/all/modules/userpoints_contrib/userpoints_badges/userpoints_badges.module).

and a second error

Notice: Undefined variable: userpoints_goal in userpoints_badges_form_alter() (line 29 of /sites/all/modules/userpoints_contrib/userpoints_badges/userpoints_badges.module).

and a third:

Notice: Undefined index: image in userpoints_badges_edit_badge_submit() (line 98 of /sites/all/modules/userpoints_contrib/userpoints_badges/userpoints_badges.module).

Have yet to investigate further as it does not prohibit the uploading of the badges, but I'm sure it's breaking something—I just don't know what, yet.

fix in #39 totally worked for me on my issue,https://drupal.org/node/2266699, which is a duplicate of #39.

While trying to figure out how to give users special badges when they accumulate a certain amount of points, I found something interesting to share. Basically I figured out how to give users new badges each time they were awarded points. Per screenshot, all you need are Rules (7.x-2.8), User Badges (7.x-4.x-dev) & User Points (7.x-1.0) to reproduce what I did. Here's what I found:

1) #26 causes WSOD for the purpose of this test, so I disabled it.
2) In "Data Comparison", I used "userpoints-transaction:points" and set it to 1. This successfully added a badge each time a user was awarded 1 point.

It looked like the important pieces of code that made this possible were found in lines 22 - 26 of userpoints_rules.rules.inc from User Points, which gets data from the userpoints_txn table in the database. So here's the million dollar question to achieve what I (and hopefully most of you guys) really want to achieve, which is to give users badges when they accumulate a certain amount of points:

Don't we just need to make the total amount of points a user has, from the userpoints_total table in the database, to show up as a token under "Data Comparison"? Or has this already been mentioned somewhere before?

P.S. I haven't done testing with User Badges 7.x-3.2 yet, but I have a feeling it'll actually work since even the newest & unstable 7.x-4.x-dev works.

Merged a modified version of #1321082: Userpoints_badges for 7.x