Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: database constrainsts plus db query improvements #8

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion outcome_surveys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 8 additions & 3 deletions outcome_surveys/management/commands/import_survey_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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):
"""
Expand Down
47 changes: 47 additions & 0 deletions outcome_surveys/migrations/0004_auto_20230203_0147.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
44 changes: 26 additions & 18 deletions outcome_surveys/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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']),
]

Expand All @@ -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,
Expand Down Expand Up @@ -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']),
]

Expand Down