Merge pull request #394 from edx/fix/foldit-leaderboard

Fix/foldit leaderboard

common/lib/xmodule/xmodule/foldit_module.py

@@ -91,15 +91,18 @@ class FolditModule(FolditFields, XModule):
             PuzzleComplete.completed_puzzles(self.system.anonymous_student_id),
             key=lambda d: (d['set'], d['subset']))
 
-    def puzzle_leaders(self, n=10):
+    def puzzle_leaders(self, n=10, courses=None):
         """
         Returns a list of n pairs (user, score) corresponding to the top
         scores; the pairs are in descending order of score.
         """
         from foldit.models import Score
 
-        leaders = [(e['username'], e['score']) for e in Score.get_tops_n(10)]
-        leaders.sort(key=lambda x:-x[1])
+        if courses is None:
+            courses = [self.location.course_id]
+
+        leaders = [(leader['username'], leader['score']) for leader in Score.get_tops_n(10, course_list=courses)]
+        leaders.sort(key=lambda x: -x[1])
 
         return leaders
 

lms/djangoapps/foldit/models.py

@@ -6,6 +6,7 @@ from django.db import models
 
 log = logging.getLogger(__name__)
 
+
 class Score(models.Model):
     """
     This model stores the scores of different users on FoldIt problems.
@@ -35,9 +36,8 @@ class Score(models.Model):
         """
         return (-score) * 10 + 8000 * sum_of
 
-
     @staticmethod
-    def get_tops_n(n, puzzles=['994559']):
+    def get_tops_n(n, puzzles=['994559'], course_list=None):
         """
         Arguments:
             puzzles: a list of puzzle ids that we will use. If not specified,
@@ -46,22 +46,34 @@ class Score(models.Model):
 
 
         Returns:
-            The top n sum of scores for puzzles in <puzzles>. Output is a list
-            of disctionaries, sorted by display_score:
+            The top n sum of scores for puzzles in <puzzles>,
+            filtered by course. If no courses is specified we default
+            the pool of students to all courses. Output is a list
+            of dictionaries, sorted by display_score:
                 [ {username: 'a_user',
                    score: 12000} ...]
         """
-        if not(type(puzzles) == list):
+
+        if not isinstance(puzzles, list):
             puzzles = [puzzles]
-        scores = Score.objects \
-            .filter(puzzle_id__in=puzzles) \
-            .annotate(total_score=models.Sum('best_score')) \
-            .order_by('total_score')[:n]
+        if course_list is None:
+            scores = Score.objects \
+                .filter(puzzle_id__in=puzzles) \
+                .annotate(total_score=models.Sum('best_score')) \
+                .order_by('total_score')[:n]
+        else:
+            scores = Score.objects \
+                .filter(puzzle_id__in=puzzles) \
+                .filter(user__courseenrollment__course_id__in=course_list) \
+                .annotate(total_score=models.Sum('best_score')) \
+                .order_by('total_score')[:n]
         num = len(puzzles)
 
-        return [{'username': s.user.username,
-                 'score': Score.display_score(s.total_score, num)}
-                 for s in scores]
+        return [
+            {'username': score.user.username,
+             'score': Score.display_score(score.total_score, num)}
+            for score in scores
+        ]
 
 
 class PuzzleComplete(models.Model):
@@ -94,7 +106,6 @@ class PuzzleComplete(models.Model):
             self.puzzle_set, self.puzzle_subset,
             self.created)
 
-
     @staticmethod
     def completed_puzzles(anonymous_user_id):
         """

lms/djangoapps/foldit/tests.py

@@ -2,14 +2,14 @@ import json
 import logging
 from functools import partial
 
-from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
 
 from foldit.views import foldit_ops, verify_code
 from foldit.models import PuzzleComplete, Score
-from student.models import UserProfile, unique_id_for_user
+from student.models import unique_id_for_user
+from student.tests.factories import CourseEnrollmentFactory, UserFactory, UserProfileFactory
 
 from datetime import datetime, timedelta
 from pytz import UTC
@@ -23,17 +23,25 @@ class FolditTestCase(TestCase):
         self.factory = RequestFactory()
         self.url = reverse('foldit_ops')
 
-        pwd = 'abc'
-        self.user = User.objects.create_user('testuser', 'test@test.com', pwd)
-        self.user2 = User.objects.create_user('testuser2', 'test2@test.com', pwd)
-        self.unique_user_id = unique_id_for_user(self.user)
-        self.unique_user_id2 = unique_id_for_user(self.user2)
+        self.course_id = 'course/id/1'
+        self.course_id2 = 'course/id/2'
+
+        self.user = UserFactory.create()
+        self.user2 = UserFactory.create()
+
+        self.course_enrollment = CourseEnrollmentFactory.create(
+            user=self.user, course_id=self.course_id
+        )
+        self.course_enrollment2 = CourseEnrollmentFactory.create(
+            user=self.user2, course_id=self.course_id2
+        )
+
         now = datetime.now(UTC)
         self.tomorrow = now + timedelta(days=1)
         self.yesterday = now - timedelta(days=1)
 
-        UserProfile.objects.create(user=self.user)
-        UserProfile.objects.create(user=self.user2)
+        self.user.profile
+        self.user2.profile
 
     def make_request(self, post_data, user=None):
         request = self.factory.post(self.url, post_data)
@@ -150,6 +158,38 @@ class FolditTestCase(TestCase):
             delta=0.5
         )
 
+    def test_SetPlayerPuzzleScores_multiplecourses(self):
+        puzzle_id = "1"
+
+        player1_score = 0.05
+        player2_score = 0.06
+
+        course_list_1 = [self.course_id]
+        course_list_2 = [self.course_id2]
+
+        response1 = self.make_puzzle_score_request(
+            puzzle_id, player1_score, self.user
+        )
+        course_1_top_10 = Score.get_tops_n(10, puzzle_id, course_list_1)
+        course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2)
+        total_top_10 = Score.get_tops_n(10, puzzle_id)
+
+        #  player1 should now be in the top 10 of course 1 and not in course 2
+        self.assertEqual(len(course_1_top_10), 1)
+        self.assertEqual(len(course_2_top_10), 0)
+        self.assertEqual(len(total_top_10), 1)
+
+        response2 = self.make_puzzle_score_request(
+            puzzle_id, player2_score, self.user2
+        )
+        course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2)
+        total_top_10 = Score.get_tops_n(10, puzzle_id)
+
+        #  player2 should now be in the top 10 of course 2 and not in course 1
+        self.assertEqual(len(course_1_top_10), 1)
+        self.assertEqual(len(course_2_top_10), 1)
+        self.assertEqual(len(total_top_10), 2)
+
     def test_SetPlayerPuzzleScores_manyplayers(self):
         """
         Check that when we send scores from multiple users, the correct order
@@ -306,7 +346,7 @@ class FolditTestCase(TestCase):
                          self.set_puzzle_complete_response([13, 14, 15, 53524]))
 
         is_complete = partial(
-            PuzzleComplete.is_level_complete, self.unique_user_id)
+            PuzzleComplete.is_level_complete, unique_id_for_user(self.user))
 
         self.assertTrue(is_complete(1, 1))
         self.assertTrue(is_complete(1, 3))