Address platform final review notes.

common/djangoapps/student/models.py

@@ -46,6 +46,7 @@ from simple_history.models import HistoricalRecords
 from track import contexts
 from xmodule_django.models import CourseKeyField, NoneToEmptyManager
 
+from lms.djangoapps.badges.utils import badges_enabled
 from certificates.models import GeneratedCertificate
 from course_modes.models import CourseMode
 from enrollment.api import _default_course_mode
@@ -1213,7 +1214,7 @@ class CourseEnrollment(models.Model):
         # User is allowed to enroll if they've reached this point.
         enrollment = cls.get_or_create_enrollment(user, course_key)
         enrollment.update_enrollment(is_active=True, mode=mode)
-        if settings.FEATURES.get("ENABLE_OPENBADGES"):
+        if badges_enabled():
             from lms.djangoapps.badges.events.course_meta import award_enrollment_badge
             award_enrollment_badge(user)
 

common/static/common/js/spec_helpers/ajax_helpers.js

@@ -73,7 +73,8 @@ define(['sinon', 'underscore', 'URI'], function(sinon, _, URI) {
         expect(request.url).toEqual(url);
         expect(request.method).toEqual(method);
         if (typeof body === 'undefined') {
-            // The contents if this call may not be germane to the current test.
+            // The body of the request may not be germane to the current test-- like some call by a library,
+            // so allow it to be ignored.
             return;
         }
         expect(request.requestBody).toEqual(body);

common/test/acceptance/pages/lms/learner_profile.py

 """
 Bok-Choy PageObject class for learner profile page.
 """
+from bok_choy.query import BrowserQuery
+
 from . import BASE_URL
 from bok_choy.page_object import PageObject
 from .fields import FieldsMixin
@@ -16,6 +18,35 @@ FIELD_ICONS = {
 }
 
 
+class Badge(PageObject):
+    """
+    Represents a single badge displayed on the learner profile page.
+    """
+    url = None
+
+    def __init__(self, element, browser):
+        self.full_view = browser
+        # Element API is similar to browser API, should allow subqueries.
+        super(Badge, self).__init__(element)
+
+    def is_browser_on_page(self):
+        return self.q(css=".badge-details").visible
+
+    def modal_displayed(self):
+        """
+        Verifies that the share modal is diplayed.
+        """
+        # The modal is on the page at large, and not a subelement of the badge div.
+        return BrowserQuery(self.full_view, css=".badges-modal").visible
+
+    def display_modal(self):
+        """
+        Click the share button to display the sharing modal for the badge.
+        """
+        self.q(css=".share-button").click()
+        EmptyPromise(self.modal_displayed, "Share modal displayed").fulfill()
+
+
 class LearnerProfilePage(FieldsMixin, PageObject):
     """
     PageObject methods for Learning Profile Page.
@@ -58,6 +89,27 @@ class LearnerProfilePage(FieldsMixin, PageObject):
         """
         return 'all_users' if self.q(css=PROFILE_VISIBILITY_SELECTOR.format('all_users')).selected else 'private'
 
+    def accomplishments_available(self):
+        """
+        Verify that the accomplishments tab is available.
+        """
+        return self.q(css="button[data-url='accomplishments']").visible
+
+    def display_accomplishments(self):
+        """
+        Click the accomplishments tab and wait for the accomplishments to load.
+        """
+        EmptyPromise(self.accomplishments_available, "Accomplishments tab is displayed").fulfill()
+        self.q(css="button[data-url='accomplishments']").click()
+        self.wait_for_element_visibility(".badge-list", "Badge list displayed")
+
+    @property
+    def badges(self):
+        """
+        Get all currently listed badges.
+        """
+        return [Badge(element, self.browser) for element in self.q(css=".badge-display:not(.badge-placeholder)")]
+
     @privacy.setter
     def privacy(self, privacy):
         """

common/test/acceptance/tests/lms/test_learner_profile.py

@@ -800,3 +800,22 @@ class LearnerProfileA11yTest(LearnerProfileTestMixin, WebAppTest):
         })
 
         profile_page.a11y_audit.check_for_accessibility_errors()
+
+    def test_badges_accessibility(self):
+        """
+        Test the accessibility of the badge listings and sharing modal.
+        """
+        username = 'testcert'
+        AutoAuthPage(self.browser, username=username).visit()
+        profile_page = self.visit_profile_page(username)
+
+        profile_page.a11y_audit.config.set_rules({
+            "ignore": [
+                'skip-link',  # TODO: AC-179
+                'link-href',  # TODO: AC-231
+            ],
+        })
+        profile_page.display_accomplishments()
+        profile_page.a11y_audit.check_for_accessibility_errors()
+        profile_page.badges[0].display_modal()
+        profile_page.a11y_audit.check_for_accessibility_errors()

lms/djangoapps/badges/admin.py

@@ -7,4 +7,5 @@ from config_models.admin import ConfigurationModelAdmin
 
 admin.site.register(CourseCompleteImageConfiguration)
 admin.site.register(BadgeClass)
+# Use the standard Configuration Model Admin handler for this model.
 admin.site.register(CourseEventBadgesConfiguration, ConfigurationModelAdmin)

lms/djangoapps/badges/api/tests.py

 """
 Tests for the badges API views.
 """
+from ddt import ddt, data, unpack
 from django.conf import settings
 from django.test.utils import override_settings
 
@@ -20,8 +21,6 @@ class UserAssertionTestCase(UrlResetMixin, ModuleStoreTestCase, ApiTestCase):
     """
     Mixin for badge API tests.
     """
-    WILDCARD = False
-    CHECK_COURSE = False
 
     def setUp(self, *args, **kwargs):
         super(UserAssertionTestCase, self).setUp(*args, **kwargs)
@@ -55,24 +54,24 @@ class UserAssertionTestCase(UrlResetMixin, ModuleStoreTestCase, ApiTestCase):
         self.assertEqual(assertion.assertion_url, json_assertion['assertion_url'])
         self.check_class_structure(assertion.badge_class, json_assertion['badge_class'])
 
-    def get_course_id(self, badge_class):
+    def get_course_id(self, wildcard, badge_class):
         """
         Used for tests which may need to test for a course_id or a wildcard.
         """
-        if self.WILDCARD:
+        if wildcard:
             return '*'
         else:
             return unicode(badge_class.course_id)
 
-    def create_badge_class(self, **kwargs):
+    def create_badge_class(self, check_course, **kwargs):
         """
         Create a badge class, using a course id if it's relevant to the URL pattern.
         """
-        if self.CHECK_COURSE:
+        if check_course:
             return RandomBadgeClassFactory.create(course_id=self.course.location.course_key, **kwargs)
         return RandomBadgeClassFactory.create(**kwargs)
 
-    def get_qs_args(self, badge_class):
+    def get_qs_args(self, check_course, wildcard, badge_class):
         """
         Get a dictionary to be serialized into querystring params based on class settings.
         """
@@ -80,8 +79,8 @@ class UserAssertionTestCase(UrlResetMixin, ModuleStoreTestCase, ApiTestCase):
             'issuing_component': badge_class.issuing_component,
             'slug': badge_class.slug,
         }
-        if self.CHECK_COURSE:
-            qs_args['course_id'] = self.get_course_id(badge_class)
+        if check_course:
+            qs_args['course_id'] = self.get_course_id(wildcard, badge_class)
         return qs_args
 
 
@@ -100,13 +99,13 @@ class TestUserBadgeAssertions(UserAssertionTestCase):
         BadgeAssertionFactory(user=self.user, badge_class=BadgeClassFactory(course_id=self.course.location.course_key))
         # Should not be included.
         for dummy in range(3):
-            self.create_badge_class()
+            self.create_badge_class(False)
         response = self.get_json(self.url())
         # pylint: disable=no-member
         self.assertEqual(len(response['results']), 4)
 
     def test_assertion_structure(self):
-        badge_class = self.create_badge_class()
+        badge_class = self.create_badge_class(False)
         assertion = BadgeAssertionFactory.create(user=self.user, badge_class=badge_class)
         response = self.get_json(self.url())
         # pylint: disable=no-member
@@ -117,7 +116,6 @@ class TestUserCourseBadgeAssertions(UserAssertionTestCase):
     """
     Test the Badge Assertions view with the course_id filter.
     """
-    CHECK_COURSE = True
 
     def test_get_assertions(self):
         """
@@ -127,10 +125,10 @@ class TestUserCourseBadgeAssertions(UserAssertionTestCase):
         badge_class = BadgeClassFactory.create(course_id=course_key)
         for dummy in range(3):
             BadgeAssertionFactory.create(user=self.user, badge_class=badge_class)
-        # Should not be included.
+        # Should not be included, as they don't share the target badge class.
         for dummy in range(3):
             BadgeAssertionFactory.create(user=self.user)
-        # Also should not be included
+        # Also should not be included, as they don't share the same user.
         for dummy in range(6):
             BadgeAssertionFactory.create(badge_class=badge_class)
         response = self.get_json(self.url(), data={'course_id': course_key})
@@ -143,7 +141,7 @@ class TestUserCourseBadgeAssertions(UserAssertionTestCase):
 
     def test_assertion_structure(self):
         """
-        Verify the badge assertion structure is not mangled in this mode.
+        Verify the badge assertion structure is as expected when a course is involved.
         """
         course_key = self.course.location.course_key
         badge_class = BadgeClassFactory.create(course_id=course_key)
@@ -153,16 +151,19 @@ class TestUserCourseBadgeAssertions(UserAssertionTestCase):
         self.check_assertion_structure(assertion, response['results'][0])
 
 
+@ddt
 class TestUserBadgeAssertionsByClass(UserAssertionTestCase):
     """
     Test the Badge Assertions view with the badge class filter.
     """
 
-    def test_get_assertions(self):
+    @unpack
+    @data((False, False), (True, False), (True, True))
+    def test_get_assertions(self, check_course, wildcard):
         """
         Verify we can get assertions via the badge class and username.
         """
-        badge_class = self.create_badge_class()
+        badge_class = self.create_badge_class(check_course)
         for dummy in range(3):
             BadgeAssertionFactory.create(user=self.user, badge_class=badge_class)
         if badge_class.course_id:
@@ -172,62 +173,52 @@ class TestUserBadgeAssertionsByClass(UserAssertionTestCase):
                 course_id=CourseFactory.create().location.course_key
             )
             BadgeAssertionFactory.create(user=self.user, badge_class=alt_class)
-        # Should not be in list.
+        # Same badge class, but different user. Should not show up in the list.
         for dummy in range(5):
             BadgeAssertionFactory.create(badge_class=badge_class)
-        # Also should not be in list.
+        # Different badge class AND different user. Certainly shouldn't show up in the list!
         for dummy in range(6):
             BadgeAssertionFactory.create()
 
         response = self.get_json(
             self.url(),
-            data=self.get_qs_args(badge_class),
+            data=self.get_qs_args(check_course, wildcard, badge_class),
         )
-        if self.WILDCARD:
+        if wildcard:
             expected_length = 4
         else:
             expected_length = 3
         # pylint: disable=no-member
         self.assertEqual(len(response['results']), expected_length)
-        unused_class = self.create_badge_class(slug='unused_slug', issuing_component='unused_component')
+        unused_class = self.create_badge_class(check_course, slug='unused_slug', issuing_component='unused_component')
 
         response = self.get_json(
             self.url(),
-            data=self.get_qs_args(unused_class),
+            data=self.get_qs_args(check_course, wildcard, unused_class),
         )
         # pylint: disable=no-member
         self.assertEqual(len(response['results']), 0)
 
-    def check_badge_class_assertion(self, badge_class):
+    def check_badge_class_assertion(self, check_course, wildcard, badge_class):
         """
         Given a badge class, create an assertion for the current user and fetch it, checking the structure.
         """
         assertion = BadgeAssertionFactory.create(badge_class=badge_class, user=self.user)
         response = self.get_json(
             self.url(),
-            data=self.get_qs_args(badge_class),
+            data=self.get_qs_args(check_course, wildcard, badge_class),
         )
         # pylint: disable=no-member
         self.check_assertion_structure(assertion, response['results'][0])
 
-    def test_assertion_structure(self):
-        self.check_badge_class_assertion(self.create_badge_class())
-
-    def test_empty_issuing_component(self):
-        self.check_badge_class_assertion(self.create_badge_class(issuing_component=''))
-
-
-# pylint: disable=test-inherits-tests
-class TestUserBadgeAssertionsByClassCourse(TestUserBadgeAssertionsByClass):
-    """
-    Test searching all assertions for a user with a course bound badge class.
-    """
-    CHECK_COURSE = True
-
+    @unpack
+    @data((False, False), (True, False), (True, True))
+    def test_assertion_structure(self, check_course, wildcard):
+        self.check_badge_class_assertion(check_course, wildcard, self.create_badge_class(check_course))
 
-# pylint: disable=test-inherits-tests
-class TestUserBadgeAssertionsByClassWildCard(TestUserBadgeAssertionsByClassCourse):
-    """
-    Test searching slugs/issuing_components across all course IDs.
-    """
-    WILDCARD = True
+    @unpack
+    @data((False, False), (True, False), (True, True))
+    def test_empty_issuing_component(self, check_course, wildcard):
+        self.check_badge_class_assertion(
+            check_course, wildcard, self.create_badge_class(check_course, issuing_component='')
+        )

lms/djangoapps/badges/api/views.py

@@ -11,17 +11,18 @@ from openedx.core.lib.api.authentication import (
     OAuth2AuthenticationAllowInactiveUser,
     SessionAuthenticationAllowInactiveUser
 )
+from xmodule_django.models import CourseKeyField
+
 from badges.models import BadgeAssertion
 from .serializers import BadgeAssertionSerializer
-from xmodule_django.models import CourseKeyField
 
 
-class CourseKeyError(APIException):
+class InvalidCourseKeyError(APIException):
     """
     Raised the course key given isn't valid.
     """
     status_code = 400
-    default_detail = "The course key provided could not be parsed."
+    default_detail = "The course key provided was invalid."
 
 
 class UserBadgeAssertions(generics.ListAPIView):
@@ -118,11 +119,13 @@ class UserBadgeAssertions(generics.ListAPIView):
             try:
                 course_id = CourseKey.from_string(provided_course_id)
             except InvalidKeyError:
-                raise CourseKeyError
+                raise InvalidCourseKeyError
         elif 'slug' not in self.request.query_params:
             # Need to get all badges for the user.
             course_id = None
         else:
+            # Django won't let us use 'None' for querying a ForeignKey field. We have to use this special
+            # 'Empty' value to indicate we're looking only for badges without a course key set.
             course_id = CourseKeyField.Empty
 
         if course_id is not None:

lms/djangoapps/badges/backends/badgr.py

@@ -175,5 +175,8 @@ class BadgrBackend(BadgeBackend):
         BadgrBackend.badges.append(slug)
 
     def award(self, badge_class, user, evidence_url=None):
+        """
+        Make sure the badge class has been created on the backend, and then award the badge class to the user.
+        """
         self._ensure_badge_created(badge_class)
         return self._create_assertion(badge_class, user, evidence_url)

lms/djangoapps/badges/backends/tests/dummy_backend.py

+"""
+Dummy backend, for use in testing.
+"""
+from lms.djangoapps.badges.backends.base import BadgeBackend
+from lms.djangoapps.badges.tests.factories import BadgeAssertionFactory
+
+
+class DummyBackend(BadgeBackend):
+    """
+    Dummy backend that creates assertions without contacting any real-world backend.
+    """
+    def award(self, badge_class, user, evidence_url=None):
+        return BadgeAssertionFactory(badge_class=badge_class, user=user)

lms/djangoapps/badges/backends/tests/test_badgr_backend.py

@@ -24,6 +24,9 @@ BADGR_SETTINGS = {
     'BADGR_ISSUER_SLUG': 'test-issuer',
 }
 
+# Should be the hashed result of test_slug as the slug, and test_component as the component
+EXAMPLE_SLUG = '15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a'
+
 
 # pylint: disable=protected-access
 @ddt.ddt
@@ -104,7 +107,7 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase):
             kwargs['data'],
             {
                 'name': 'Test Badge',
-                'slug': '15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a',
+                'slug': EXAMPLE_SLUG,
                 'criteria': 'https://example.com/syllabus',
                 'description': "Yay! It's a test badge.",
             }
@@ -114,14 +117,14 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase):
         """
         Make sure ensure_badge_created doesn't call create_badge if we know the badge is already there.
         """
-        BadgrBackend.badges.append('15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a')
+        BadgrBackend.badges.append(EXAMPLE_SLUG)
         self.handler._create_badge = Mock()
         self.handler._ensure_badge_created(self.badge_class)
         self.assertFalse(self.handler._create_badge.called)
 
     @ddt.unpack
     @ddt.data(
-        ('badge_class', '15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a'),
+        ('badge_class', EXAMPLE_SLUG),
         ('legacy_badge_class', 'test_slug'),
         ('no_course_badge_class', 'test_componenttest_slug')
     )
@@ -140,11 +143,11 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase):
         args, kwargs = get.call_args
         self.assertEqual(
             args[0],
-            'https://example.com/v1/issuer/issuers/test-issuer/badges/'
-            '15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a'
+            'https://example.com/v1/issuer/issuers/test-issuer/badges/' +
+            EXAMPLE_SLUG
         )
         self.check_headers(kwargs['headers'])
-        self.assertIn('15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a', BadgrBackend.badges)
+        self.assertIn(EXAMPLE_SLUG, BadgrBackend.badges)
         self.assertFalse(self.handler._create_badge.called)
 
     @patch('requests.get')
@@ -152,12 +155,12 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase):
         response = Mock()
         response.status_code = 404
         get.return_value = response
-        self.assertNotIn('15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a', BadgrBackend.badges)
+        self.assertNotIn(EXAMPLE_SLUG, BadgrBackend.badges)
         self.handler._create_badge = Mock()
         self.handler._ensure_badge_created(self.badge_class)
         self.assertTrue(self.handler._create_badge.called)
         self.assertEqual(self.handler._create_badge.call_args, call(self.badge_class))
-        self.assertIn('15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a', BadgrBackend.badges)
+        self.assertIn(EXAMPLE_SLUG, BadgrBackend.badges)
 
     @patch('requests.post')
     def test_badge_creation_event(self, post):
@@ -175,8 +178,9 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase):
         args, kwargs = post.call_args
         self.assertEqual(
             args[0],
-            'https://example.com/v1/issuer/issuers/test-issuer/badges/'
-            '15bb687e0c59ef2f0a49f6838f511bf4ca6c566dd45da6293cabbd9369390e1a/assertions'
+            'https://example.com/v1/issuer/issuers/test-issuer/badges/' +
+            EXAMPLE_SLUG +
+            '/assertions'
         )
         self.check_headers(kwargs['headers'])
         assertion = BadgeAssertion.objects.get(user=self.user, badge_class__course_id=self.course.location.course_key)

lms/djangoapps/badges/events/course_meta.py

@@ -15,6 +15,9 @@ def award_badge(config, count, user):
     config is a dictionary with integer keys and course keys as values.
     count is the key to retrieve from this dictionary.
     user is the user to award the badge to.
+
+    Example config:
+        {3: 'slug_for_badge_for_three_enrollments', 5: 'slug_for_badge_with_five_enrollments'}
     """
     slug = config.get(count)
     if not slug:

lms/djangoapps/badges/events/tests/test_course_meta.py

 """
 Tests the course meta badging events
 """
-
+from ddt import ddt, unpack, data
 from django.test.utils import override_settings
 from mock import patch
 
 from django.conf import settings
 
-from badges.backends.base import BadgeBackend
-from badges.tests.factories import RandomBadgeClassFactory, CourseEventBadgesConfigurationFactory, BadgeAssertionFactory
+from badges.tests.factories import RandomBadgeClassFactory, CourseEventBadgesConfigurationFactory
 from certificates.models import GeneratedCertificate, CertificateStatuses
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
@@ -16,16 +15,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 
-class DummyBackend(BadgeBackend):
-    """
-    Dummy backend that creates assertions without contacting any real-world backend.
-    """
-    def award(self, badge_class, user, evidence_url=None):
-        return BadgeAssertionFactory(badge_class=badge_class, user=user)
-
-
+@ddt
 @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True})
-@override_settings(BADGING_BACKEND='lms.djangoapps.badges.events.tests.test_course_meta.DummyBackend')
+@override_settings(BADGING_BACKEND='lms.djangoapps.badges.backends.tests.dummy_backend.DummyBackend')
 class CourseEnrollmentBadgeTest(ModuleStoreTestCase):
     """
     Tests the event which awards badges based on number of courses a user is enrolled in.
@@ -58,42 +50,25 @@ class CourseEnrollmentBadgeTest(ModuleStoreTestCase):
         CourseEnrollment.enroll(user, course_key=course.location.course_key)
         self.assertFalse(user.badgeassertion_set.all())
 
-    def test_checkpoint_matches(self):
+    @unpack
+    @data((1, 3), (2, 5), (3, 8))
+    def test_checkpoint_matches(self, checkpoint, required_badges):
         """
         Make sure the proper badges are awarded at the right checkpoints.
         """
         user = UserFactory()
-        courses = [CourseFactory() for _i in range(3)]
-        for course in courses:
-            CourseEnrollment.enroll(user, course_key=course.location.course_key)
-        # pylint: disable=no-member
-        assertions = user.badgeassertion_set.all()
-        self.assertEqual(user.badgeassertion_set.all().count(), 1)
-        self.assertEqual(assertions[0].badge_class, self.badge_classes[0])
-
-        courses = [CourseFactory() for _i in range(2)]
-        for course in courses:
-            # pylint: disable=no-member
-            CourseEnrollment.enroll(user, course_key=course.location.course_key)
-        # pylint: disable=no-member
-        assertions = user.badgeassertion_set.all().order_by('id')
-        # pylint: disable=no-member
-        self.assertEqual(user.badgeassertion_set.all().count(), 2)
-        self.assertEqual(assertions[1].badge_class, self.badge_classes[1])
-
-        courses = [CourseFactory() for _i in range(3)]
+        courses = [CourseFactory() for _i in range(required_badges)]
         for course in courses:
-            # pylint: disable=no-member
             CourseEnrollment.enroll(user, course_key=course.location.course_key)
         # pylint: disable=no-member
         assertions = user.badgeassertion_set.all().order_by('id')
-        # pylint: disable=no-member
-        self.assertEqual(user.badgeassertion_set.all().count(), 3)
-        self.assertEqual(assertions[2].badge_class, self.badge_classes[2])
+        self.assertEqual(user.badgeassertion_set.all().count(), checkpoint)
+        self.assertEqual(assertions[checkpoint - 1].badge_class, self.badge_classes[checkpoint - 1])
 
 
+@ddt
 @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True})
-@override_settings(BADGING_BACKEND='lms.djangoapps.badges.events.tests.test_course_meta.DummyBackend')
+@override_settings(BADGING_BACKEND='lms.djangoapps.badges.backends.tests.dummy_backend.DummyBackend')
 class CourseCompletionBadgeTest(ModuleStoreTestCase):
     """
     Tests the event which awards badges based on the number of courses completed.
@@ -130,47 +105,28 @@ class CourseCompletionBadgeTest(ModuleStoreTestCase):
         # pylint: disable=no-member
         self.assertFalse(user.badgeassertion_set.all())
 
-    def test_checkpoint_matches(self):
+    @unpack
+    @data((1, 2), (2, 6), (3, 9))
+    def test_checkpoint_matches(self, checkpoint, required_badges):
         """
         Make sure the proper badges are awarded at the right checkpoints.
         """
         user = UserFactory()
-        courses = [CourseFactory() for _i in range(2)]
+        courses = [CourseFactory() for _i in range(required_badges)]
         for course in courses:
             GeneratedCertificate(
                 # pylint: disable=no-member
                 user=user, course_id=course.location.course_key, status=CertificateStatuses.downloadable
             ).save()
         # pylint: disable=no-member
-        assertions = user.badgeassertion_set.all()
-        # pylint: disable=no-member
-        self.assertEqual(user.badgeassertion_set.all().count(), 1)
-        self.assertEqual(assertions[0].badge_class, self.badge_classes[0])
-
-        courses = [CourseFactory() for _i in range(6)]
-        for course in courses:
-            GeneratedCertificate(
-                user=user, course_id=course.location.course_key, status=CertificateStatuses.downloadable
-            ).save()
-        # pylint: disable=no-member
-        assertions = user.badgeassertion_set.all().order_by('id')
-        self.assertEqual(user.badgeassertion_set.all().count(), 2)
-        self.assertEqual(assertions[1].badge_class, self.badge_classes[1])
-
-        courses = [CourseFactory() for _i in range(9)]
-        for course in courses:
-            GeneratedCertificate(
-                user=user, course_id=course.location.course_key, status=CertificateStatuses.downloadable
-            ).save()
-        # pylint: disable=no-member
         assertions = user.badgeassertion_set.all().order_by('id')
         # pylint: disable=no-member
-        self.assertEqual(user.badgeassertion_set.all().count(), 3)
-        self.assertEqual(assertions[2].badge_class, self.badge_classes[2])
+        self.assertEqual(user.badgeassertion_set.all().count(), checkpoint)
+        self.assertEqual(assertions[checkpoint - 1].badge_class, self.badge_classes[checkpoint - 1])
 
 
 @patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True})
-@override_settings(BADGING_BACKEND='lms.djangoapps.badges.events.tests.test_course_meta.DummyBackend')
+@override_settings(BADGING_BACKEND='lms.djangoapps.badges.backends.tests.dummy_backend.DummyBackend')
 class CourseGroupBadgeTest(ModuleStoreTestCase):
     """
     Tests the event which awards badges when a user completes a set of courses.

lms/djangoapps/badges/migrations/0002_data__migrate_assertions.py

@@ -39,10 +39,9 @@ def forwards(apps, schema_editor):
                 mode=image_config.mode,
                 course_id=badge.course_id,
             )
-            file_content = ContentFile(icon.read())
             badge_class._meta.get_field('image').generate_filename = \
                 lambda inst, fn: os.path.join('badge_classes', fn)
-            badge_class.image.save(icon.name, file_content)
+            badge_class.image.name = icon.name
             badge_class.save()
             classes[(badge.course_id, badge.mode)] = badge_class
         if isinstance(badge.data, basestring):
@@ -66,17 +65,15 @@ def forwards(apps, schema_editor):
         assertion.save()
 
     for configuration in BadgeImageConfiguration.objects.all():
-        file_content = ContentFile(configuration.icon.read())
         new_conf = CourseCompleteImageConfiguration(
             default=configuration.default,
             mode=configuration.mode,
         )
-        new_conf.icon.save(configuration.icon.name, file_content)
+        new_conf.icon.name = configuration.icon.name
         new_conf.save()
 
 #
 def backwards(apps, schema_editor):
-    from django.core.files.base import ContentFile
     OldBadgeAssertion = apps.get_model("certificates", "BadgeAssertion")
     BadgeAssertion = apps.get_model("badges", "BadgeAssertion")
     BadgeImageConfiguration = apps.get_model("certificates", "BadgeImageConfiguration")
@@ -97,12 +94,11 @@ def backwards(apps, schema_editor):
         ).save()
 
     for configuration in CourseCompleteImageConfiguration.objects.all():
-        file_content = ContentFile(configuration.icon.read())
         new_conf = BadgeImageConfiguration(
             default=configuration.default,
             mode=configuration.mode,
         )
-        new_conf.icon.save(configuration.icon.name, file_content)
+        new_conf.icon.name = configuration.icon.name
         new_conf.save()
 
 

lms/djangoapps/badges/models.py

@@ -8,20 +8,21 @@ from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
+from jsonfield import JSONField
 from lazy import lazy
 from model_utils.models import TimeStampedModel
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 
+from badges.utils import deserialize_count_specs
 from config_models.models import ConfigurationModel
 from xmodule.modulestore.django import modulestore
 from xmodule_django.models import CourseKeyField
-from jsonfield import JSONField
 
 
 def validate_badge_image(image):
     """
-    Validates that a particular image is small enough, of the right type, and square to be a badge.
+    Validates that a particular image is small enough to be a badge and square.
     """
     if image.width != image.height:
         raise ValidationError(_(u"The badge image must be square."))
@@ -70,6 +71,11 @@ class BadgeClass(models.Model):
         """
         Looks up a badge class by its slug, issuing component, and course_id and returns it should it exist.
         If it does not exist, and create is True, creates it according to the arguments. Otherwise, returns None.
+
+        The expectation is that an XBlock or platform developer should not need to concern themselves with whether
+        or not a badge class has already been created, but should just feed all requirements to this function
+        and it will 'do the right thing'. It should be the exception, rather than the common case, that a badge class
+        would need to be looked up without also being created were it missing.
         """
         slug = slug.lower()
         issuing_component = issuing_component.lower()
@@ -253,32 +259,19 @@ class CourseEventBadgesConfiguration(ConfigurationModel):
     def __unicode__(self):
         return u"<CourseEventBadgesConfiguration ({})>".format(u"Enabled" if self.enabled else u"Disabled")
 
-    @staticmethod
-    def get_specs(text):
-        """
-        Takes a string in the format of:
-            int,course_key
-            int,course_key
-
-        And returns a dictionary with the keys as the numbers and the values as the course keys.
-        """
-        specs = text.splitlines()
-        specs = [line.split(',') for line in specs if line.strip()]
-        return {int(num): slug.strip().lower() for num, slug in specs}
-
     @property
     def completed_settings(self):
         """
         Parses the settings from the courses_completed field.
         """
-        return self.get_specs(self.courses_completed)
+        return deserialize_count_specs(self.courses_completed)
 
     @property
     def enrolled_settings(self):
         """
         Parses the settings from the courses_completed field.
         """
-        return self.get_specs(self.courses_enrolled)
+        return deserialize_count_specs(self.courses_enrolled)
 
     @property
     def course_group_settings(self):

lms/djangoapps/badges/utils.py

@@ -20,7 +20,27 @@ def requires_badges_enabled(function):
         """
         Wrapped function which bails out early if bagdes aren't enabled.
         """
-        if not settings.FEATURES.get('ENABLE_OPENBADGES', False):
+        if not badges_enabled():
             return
         return function(*args, **kwargs)
     return wrapped
+
+
+def badges_enabled():
+    """
+    returns a boolean indicating whether or not openbadges are enabled.
+    """
+    return settings.FEATURES.get('ENABLE_OPENBADGES', False)
+
+
+def deserialize_count_specs(text):
+    """
+    Takes a string in the format of:
+        int,course_key
+        int,course_key
+
+    And returns a dictionary with the keys as the numbers and the values as the course keys.
+    """
+    specs = text.splitlines()
+    specs = [line.split(',') for line in specs if line.strip()]
+    return {int(num): slug.strip().lower() for num, slug in specs}

lms/djangoapps/certificates/views/webview.py

@@ -15,6 +15,7 @@ from django.utils.translation import ugettext as _
 from django.utils.encoding import smart_str
 
 from badges.events.course_complete import get_completion_badge
+from badges.utils import badges_enabled
 from courseware.access import has_access
 from edxmako.shortcuts import render_to_response
 from edxmako.template import Template
@@ -445,7 +446,7 @@ def _update_badge_context(context, course, user):
     Updates context with badge info.
     """
     badge = None
-    if settings.FEATURES.get('ENABLE_OPENBADGES') and course.issue_badges:
+    if badges_enabled() and course.issue_badges:
         badges = get_completion_badge(course.location.course_key, user).get_for_user(user)
         if badges:
             badge = badges[0]

lms/djangoapps/lms_xblock/runtime.py

@@ -7,6 +7,7 @@ from django.core.urlresolvers import reverse
 from django.conf import settings
 
 from badges.service import BadgingService
+from badges.utils import badges_enabled
 from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api
 from request_cache.middleware import RequestCache
 import xblock.reference.plugins
@@ -215,7 +216,7 @@ class LmsModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
         store = modulestore()
         services['settings'] = SettingsService()
         services['user_tags'] = UserTagsService(self)
-        if settings.FEATURES["ENABLE_OPENBADGES"]:
+        if badges_enabled():
             services['badging'] = BadgingService(course_id=kwargs.get('course_id'), modulestore=store)
         self.request_token = kwargs.pop('request_token', None)
         super(LmsModuleSystem, self).__init__(**kwargs)

lms/djangoapps/student_profile/views.py

@@ -9,6 +9,7 @@ from django.views.decorators.http import require_http_methods
 from django_countries import countries
 from django.contrib.staticfiles.storage import staticfiles_storage
 
+from badges.utils import badges_enabled
 from edxmako.shortcuts import render_to_response, marketing_link
 from microsite_configuration import microsite
 from openedx.core.djangoapps.user_api.accounts.api import get_account_settings
@@ -96,7 +97,7 @@ def learner_profile_context(request, profile_username, user_is_staff):
         'disable_courseware_js': True,
     }
 
-    if settings.FEATURES.get("ENABLE_OPENBADGES"):
+    if badges_enabled():
         context['data']['badges_api_url'] = reverse("badges_api:user_assertions", kwargs={'username': profile_username})
 
     return context

lms/envs/bok_choy.py

@@ -161,6 +161,9 @@ FEATURES['ENABLE_COURSEWARE_SEARCH'] = True
 # Enable dashboard search for tests
 FEATURES['ENABLE_DASHBOARD_SEARCH'] = True
 
+# Enable support for OpenBadges accomplishments
+FEATURES['ENABLE_OPENBADGES'] = True
+
 # Use MockSearchEngine as the search engine for test scenario
 SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine"
 # Path at which to store the mock index
@@ -184,6 +187,8 @@ PROFILE_IMAGE_BACKEND = {
 FEATURES['ENABLE_CSMH_EXTENDED'] = True
 INSTALLED_APPS += ('coursewarehistoryextended',)
 
+BADGING_BACKEND = 'lms.djangoapps.badges.backends.tests.dummy_backend.DummyBackend'
+
 #####################################################################
 # Lastly, see if the developer has any local overrides.
 try:

lms/static/js/student_profile/views/share_modal_view.js

@@ -8,6 +8,7 @@
                 attributes: {
                     'class': 'badges-overlay'
                 },
+                template: _.template(badgeModalTemplate),
                 events: {
                     'click .badges-modal': function (event) {event.stopPropagation();},
                     'click .badges-modal .close': 'close',
@@ -38,7 +39,7 @@
                     this.$el.find('.badges-modal').focus();
                 },
                 render: function () {
-                    this.$el.html(_.template(badgeModalTemplate, this.model.toJSON()));
+                    this.$el.html(this.template(this.model.toJSON()));
                     return this;
                 }
             });

lms/static/sass/views/_learner-profile.scss

@@ -317,6 +317,14 @@
     .badge-set-display {
         @extend .container;
         padding: 0 0;
+
+        .badge-list {
+            // We're using a div instead of ul for accessibility, so we have to match the style
+            // used by ul.
+            margin: 1em 0;
+            padding: 0 0 0 40px;
+        }
+
         .badge-display {
             width: 50%;
             display: inline-block;

lms/templates/student_profile/badge_list.underscore

 <div class="sr-is-focusable sr-<%= type %>-view" tabindex="-1"></div>
 <div class="<%= type %>-paging-header"></div>
-<ul class="<%= type %>-list cards-list"></ul>
+<div class="<%= type %>-list cards-list"></div>
 <div class="<%= type %>-paging-footer"></div>

openedx/core/djangoapps/user_api/accounts/serializers.py

@@ -6,6 +6,7 @@ from django.contrib.auth.models import User
 from django.conf import settings
 from django.core.urlresolvers import reverse
 
+from lms.djangoapps.badges.utils import badges_enabled
 from . import (
     NAME_MIN_LENGTH, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY,
     ALL_USERS_VISIBILITY,
@@ -63,7 +64,7 @@ class UserReadOnlySerializer(serializers.Serializer):
         :return: Dict serialized account
         """
         profile = user.profile
-        accomplishments_shared = settings.FEATURES.get('ENABLE_OPENBADGES') or False
+        accomplishments_shared = badges_enabled()
 
         data = {
             "username": user.username,