From 5e51be733b883b745268c8d0cae7303a483b3481 Mon Sep 17 00:00:00 2001
From: Ralph Bean <rbean@redhat.com>
Date: Fri, 17 Jan 2014 15:01:00 +0000
Subject: [PATCH] Correct the logic for checking group membership when awarding
 old group badges.

Reviewed by Patrick Uiterwijk.
---
 .../files/cron/award-oldschool-badges         | 34 +++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/roles/badges-backend/files/cron/award-oldschool-badges b/roles/badges-backend/files/cron/award-oldschool-badges
index fa4b46f43a..62e3dd6174 100755
--- a/roles/badges-backend/files/cron/award-oldschool-badges
+++ b/roles/badges-backend/files/cron/award-oldschool-badges
@@ -35,6 +35,35 @@ fedmsg.init(**fm_config)
 import fedbadges.utils
 
 
+def user_in_group(user, group_name):
+    # First, bail out if they're not in the group at all
+    if not any([g.name == group_name for g in user.memberships]):
+        return False
+
+    # Find the group_id of the group we're looking for..
+    group_id = None
+    for g in user.memberships:
+        if g.name == group_name:
+            group_id = g.group_id
+            break
+
+    if not group_id:
+        return False
+
+    # For that group_id, find the relevant role
+    relevant_role = None
+    for role in user.roles:
+        if role.group_id == group_id:
+            relevant_role = role
+            break
+
+    if not relevant_role:
+        return False
+
+    # They must be actually 'approved' in that group for this to count
+    return relevant_role.role_status == 'approved':
+
+
 def get_fas_groupings(fas_credentials, lookup, **config):
     creds = fas_credentials
 
@@ -60,10 +89,11 @@ def get_fas_groupings(fas_credentials, lookup, **config):
     for user in mega_list:
         # This is the main check.
         for group_name, badge_id in lookup.iteritems():
-            if any([group.name == group_name for group in user.memberships]):
+            if user_in_group(user, group_name):
                 results[group_name] = results.get(group_name, []) + [user]
 
-        # This is special.. we're checking for being a packager-sponsor
+        # Beyond the main check, here is a special check that makes sure they
+        # are a sponsor in the packager group.
         if not packager_id:
             for group in user.memberships:
                 if group.name == 'packager':