diff --git a/lms/djangoapps/courseware/services.py b/lms/djangoapps/courseware/services.py index e0490a023d88b7e47b8bc7e431fe5d3fcc4ee21e..300960a385a816507543f67b4ab4d08c6e4d334c 100644 --- a/lms/djangoapps/courseware/services.py +++ b/lms/djangoapps/courseware/services.py @@ -5,7 +5,10 @@ from __future__ import absolute_import import json +from django.contrib.auth.models import User + from lms.djangoapps.courseware.models import StudentModule +from student.models import get_user_by_username_or_email class UserStateService(object): @@ -13,20 +16,24 @@ class UserStateService(object): User state service to make state accessible in runtime. """ - def get_state_as_dict(self, username, block_id): + def get_state_as_dict(self, username_or_email, block_id): """ Return dict containing user state for a given set of parameters. Arguments: - username: username of the user for whom the data is being retrieved + username_or_email: username or email of the user for whom the data is being retrieved block_id: string/object representation of the block whose user state is required Returns: Returns a dict containing user state, if present, else empty. """ + try: + user = get_user_by_username_or_email(username_or_email=username_or_email) + except User.DoesNotExist: + return {} try: student_module = StudentModule.objects.get( - student__username=username, + student=user, module_state_key=block_id ) return json.loads(student_module.state) diff --git a/lms/djangoapps/courseware/tests/test_services.py b/lms/djangoapps/courseware/tests/test_services.py index b120f6fca8b9b2e8b26fb0f37c3f5ccccd0909e6..09227452971871e9eab887efbb7743518198c300 100644 --- a/lms/djangoapps/courseware/tests/test_services.py +++ b/lms/djangoapps/courseware/tests/test_services.py @@ -3,6 +3,7 @@ Tests for courseware services. """ from __future__ import absolute_import +import itertools import json import ddt @@ -55,11 +56,28 @@ class TestUserStateService(ModuleStoreTestCase): state=json.dumps(state) ) + def _get_email_or_username(self, should_use_email): + """ + Helper function that returns either username or email of the user based on given criteria. + + Arguments: + should_use_email(bool): Flag to identify if the email is to be returned + Returns: + username/email depending upon the value of provided flag + """ + return self.user.email if should_use_email else self.user.username + @ddt.data( - ({'key_1a': 'value_1a', 'key_2a': 'value_2a'}), - ({'key_1b': 'value_1b', 'key_2b': 'value_2b', 'key_3b': 'value_3b'}) - ) - def test_student_state(self, expected_state): + *itertools.product( + [ + ({'key_1a': 'value_1a', 'key_2a': 'value_2a'}), + ({'key_1b': 'value_1b', 'key_2b': 'value_2b', 'key_3b': 'value_3b'}) + ], [ + True, False + ] + )) + @ddt.unpack + def test_student_state(self, expected_state, should_use_email): """ Verify the service gets the correct state from the student module. @@ -71,16 +89,22 @@ class TestUserStateService(ModuleStoreTestCase): """ self._create_student_module(expected_state) state = UserStateService().get_state_as_dict( - self.user.username, self.problem.location + self._get_email_or_username(should_use_email), self.problem.location ) self.assertDictEqual(state, expected_state) @ddt.data( - ({'username': 'no_user', 'block_id': 'block-v1:myOrg+1234+2030_T2+type@openassessment+block@hash'}), - ({'username': 'no_user'}), - ({'block_id': 'block-v1:myOrg+1234+2030_T2+type@openassessment+block@hash'}) - ) - def test_nonexistent_student_module_state(self, state_params): + *itertools.product( + [ + ({'username_or_email': 'no_user', 'block_id': 'block-v1:myOrg+123+2030_T2+type@openassessment+block@hash'}), # pylint: disable=line-too-long + ({'username_or_email': 'no_user'}), + ({'block_id': 'block-v1:myOrg+1234+2030_T2+type@openassessment+block@hash'}) + ], [ + True, False + ] + )) + @ddt.unpack + def test_nonexistent_student_module_state(self, state_params, should_use_email): """ Verify the user state service returns empty dict for non-existent student module entry. @@ -91,7 +115,7 @@ class TestUserStateService(ModuleStoreTestCase): Then an empty dict is returned """ params = { - 'username': self.user.username, + 'username_or_email': self._get_email_or_username(should_use_email), 'block_id': self.problem.location } params.update(state_params)