Merge pull request #8 from edx/ammar/database-constrainst-plus-improv…

…ements

fix: database constrainsts plus db query improvements

CHANGELOG.rst

@@ -15,6 +15,12 @@ Unreleased
 ~~~~~~~~~~
 
 
+[2.1.0] - 2023-02-03
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Add uniqe constraints on table fields
+* Replace `get_or_create`` with custom implementation
+* Gracefully exit command upon `SurveyMonkeyDailyRateLimitConsumed` exception
+
 [2.0.0] - 2023-02-01
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Django management command to import data from SurveyMonkey

outcome_surveys/__init__.py

@@ -2,6 +2,6 @@
 Outcome Surveys.
 """
 
-__version__ = '2.0.0'
+__version__ = '2.1.0'
 
 default_app_config = 'outcome_surveys.apps.OutcomeSurveysConfig'  # pylint: disable=invalid-name

outcome_surveys/management/commands/import_survey_data.py

@@ -7,7 +7,7 @@
 from django.core.management.base import BaseCommand
 
 from outcome_surveys.models import CourseGoal, CourseReflection, SurveyExport
-from outcome_surveys.surveymonkey_client import SurveyMonkeyApiClient
+from outcome_surveys.surveymonkey_client import SurveyMonkeyApiClient, SurveyMonkeyDailyRateLimitConsumed
 
 LOGGER = logging.getLogger(__name__)
 
@@ -337,8 +337,13 @@ def handle(self, *args, **options):
                     url
                 )
 
-                # fetch 100 responses at a time
-                survey_responses = client.fetch_survey_responses(url)
+                try:
+                    # fetch 100 responses at a time
+                    survey_responses = client.fetch_survey_responses(url)
+                except SurveyMonkeyDailyRateLimitConsumed:
+                    LOGGER.info("Consumed daily api call limit. Can not make more calls.")
+                    return
+
                 survey_responses = survey_responses.json()
                 date_modified = None
 

outcome_surveys/management/commands/tests/test_import_survey_data.py

@@ -14,7 +14,7 @@
 from outcome_surveys.management.commands.import_survey_data import Command
 from outcome_surveys.management.commands.tests.mock_responses import MOCK_API_RESPONSES, MOCK_RESPONSE_HEADERS
 from outcome_surveys.models import CourseGoal, CourseReflection, MultiChoiceResponse, SurveyExport
-from outcome_surveys.surveymonkey_client import Session, SurveyMonkeyDailyRateLimitConsumed
+from outcome_surveys.surveymonkey_client import Session
 
 
 class MockResponse:
@@ -156,18 +156,14 @@ def verify_survey_export(self):
             survey_date_modified = survey['data'][0]['date_modified']
             assert SurveyExport.last_successfull_export_timestamp(survey_id=survey_id) == survey_date_modified
 
-    def test_command_with_ratelimit(self):
+    @patch('outcome_surveys.management.commands.import_survey_data.LOGGER')
+    def test_command_with_ratelimit(self, mocked_logger):
         """
         Verify that management command works as expected in non-commit mode.
         """
         with patch.object(Session, 'get', side_effect=mocked_get_function_with_rate_limit_exception):
-            try:
-                exception_raised = False
-                call_command(self.command)
-            except SurveyMonkeyDailyRateLimitConsumed:
-                exception_raised = True
-
-            assert exception_raised
+            call_command(self.command)
+            mocked_logger.info.assert_called_with('Consumed daily api call limit. Can not make more calls.')
 
     def test_command_with_commit(self):
         """

outcome_surveys/migrations/0004_auto_20230203_0147.py

@@ -0,0 +1,47 @@
+# Generated by Django 3.2.16 on 2023-02-03 07:47
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('outcome_surveys', '0003_auto_20230130_0352'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='coursegoal',
+            name='survey_response_id',
+            field=models.BigIntegerField(),
+        ),
+        migrations.AlterField(
+            model_name='coursereflection',
+            name='survey_response_id',
+            field=models.BigIntegerField(),
+        ),
+        migrations.AlterUniqueTogether(
+            name='coursegoal',
+            unique_together={('survey_id', 'survey_response_id')},
+        ),
+        migrations.AlterUniqueTogether(
+            name='coursereflection',
+            unique_together={('survey_id', 'survey_response_id')},
+        ),
+        migrations.AddIndex(
+            model_name='coursegoal',
+            index=models.Index(fields=['survey_id', 'survey_response_id'], name='outcome_sur_survey__8d827f_idx'),
+        ),
+        migrations.AddIndex(
+            model_name='coursegoal',
+            index=models.Index(fields=['survey_response_id'], name='outcome_sur_survey__f64c25_idx'),
+        ),
+        migrations.AddIndex(
+            model_name='coursereflection',
+            index=models.Index(fields=['survey_id', 'survey_response_id'], name='outcome_sur_survey__8a6a99_idx'),
+        ),
+        migrations.AddIndex(
+            model_name='coursereflection',
+            index=models.Index(fields=['survey_response_id'], name='outcome_sur_survey__16601f_idx'),
+        ),
+    ]

outcome_surveys/models.py

@@ -60,6 +60,21 @@ class MultiChoiceResponse(TimeStampedModel):
 
     answer = models.TextField()
 
+    @staticmethod
+    def save_answers(parent, related_field_name, user_choices):
+        """
+        Store answers.
+        """
+        answers = []
+        for user_choice in user_choices:
+            answer = MultiChoiceResponse.objects.filter(answer=user_choice).first()
+            if answer is None:
+                answer = MultiChoiceResponse.objects.create(answer=user_choice)
+            answers.append(answer)
+
+        instance_related_field = getattr(parent, related_field_name)
+        instance_related_field.set(answers)
+
     class Meta:
         """
         Meta class for MultiChoiceResponse.
@@ -82,7 +97,7 @@ class CourseReflection(TimeStampedModel):
     """
 
     survey_id = models.IntegerField()
-    survey_response_id = models.IntegerField()
+    survey_response_id = models.BigIntegerField()
     enrollment_type = models.CharField(
         max_length=16,
         null=True,
@@ -140,25 +155,19 @@ def save_response(cls, response):
             defaults=survey_response
         )
 
-        online_learning_goal_objs = []
-        for online_learning_goal in online_learning_goals:
-            obj, __ = MultiChoiceResponse.objects.get_or_create(answer=online_learning_goal)
-            online_learning_goal_objs.append(obj)
-        course_reflection.online_learning_goals.set(online_learning_goal_objs)
-
-        goal_decision_objs = []
-        for goal_decision in goal_decisions:
-            obj, __ = MultiChoiceResponse.objects.get_or_create(answer=goal_decision)
-            goal_decision_objs.append(obj)
-        course_reflection.goal_decisions.set(goal_decision_objs)
+        MultiChoiceResponse.save_answers(course_reflection, 'online_learning_goals', online_learning_goals)
+        MultiChoiceResponse.save_answers(course_reflection, 'goal_decisions', goal_decisions)
 
     class Meta:
         """
         Meta class for CourseReflection.
         """
 
         app_label = "outcome_surveys"
+        unique_together = ("survey_id", "survey_response_id",)
         indexes = [
+            models.Index(fields=['survey_id', 'survey_response_id']),
+            models.Index(fields=['survey_response_id']),
             models.Index(fields=['lms_enrollment_id']),
         ]
 
@@ -177,7 +186,7 @@ class CourseGoal(TimeStampedModel):
     """
 
     survey_id = models.IntegerField()
-    survey_response_id = models.IntegerField()
+    survey_response_id = models.BigIntegerField()
     enrollment_type = models.CharField(
         max_length=16,
         null=True,
@@ -258,19 +267,18 @@ def save_response(cls, response):
             defaults=survey_response
         )
 
-        online_learning_goal_objs = []
-        for online_learning_goal in online_learning_goals:
-            obj, __ = MultiChoiceResponse.objects.get_or_create(answer=online_learning_goal)
-            online_learning_goal_objs.append(obj)
-        course_goal.online_learning_goals.set(online_learning_goal_objs)
+        MultiChoiceResponse.save_answers(course_goal, 'online_learning_goals', online_learning_goals)
 
     class Meta:
         """
         Meta class for CourseGoal.
         """
 
         app_label = "outcome_surveys"
+        unique_together = ("survey_id", "survey_response_id",)
         indexes = [
+            models.Index(fields=['survey_id', 'survey_response_id']),
+            models.Index(fields=['survey_response_id']),
             models.Index(fields=['lms_enrollment_id']),
         ]