Commit 74844931 by Stephen Sanchez

Split up create and get for Student Training Workflows

parent cd92efad
...@@ -309,7 +309,11 @@ def get_training_example(submission_uuid, rubric, examples): ...@@ -309,7 +309,11 @@ def get_training_example(submission_uuid, rubric, examples):
raise StudentTrainingRequestError(msg) raise StudentTrainingRequestError(msg)
# Get or create the workflow # Get or create the workflow
workflow = StudentTrainingWorkflow.get_or_create_workflow(submission_uuid=submission_uuid) workflow = StudentTrainingWorkflow.get_workflow(submission_uuid=submission_uuid)
if not workflow:
raise StudentTrainingRequestError(
u"No student training workflow found for submission {}".format(submission_uuid)
)
# Get or create the training examples # Get or create the training examples
examples = deserialize_training_examples(examples, rubric) examples = deserialize_training_examples(examples, rubric)
...@@ -336,6 +340,34 @@ def get_training_example(submission_uuid, rubric, examples): ...@@ -336,6 +340,34 @@ def get_training_example(submission_uuid, rubric, examples):
raise StudentTrainingInternalError(msg) raise StudentTrainingInternalError(msg)
def create_student_training_workflow(submission_uuid):
"""
Creates a new student training workflow.
This function should be called to indicate that a submission has entered the
student training workflow part of the assessment process.
Args:
submission_uuid (str): The submission UUID for the student that is
initiating training.
Returns:
None
Raises:
StudentTrainingInternalError: Raised when an error occurs persisting the
Student Training Workflow
"""
try:
StudentTrainingWorkflow.create_workflow(submission_uuid)
except Exception:
msg = (
u"An internal error has occurred while creating the student "
u"training workflow for submission UUID {}".format(submission_uuid)
)
logger.exception(msg)
raise StudentTrainingInternalError(msg)
def assess_training_example(submission_uuid, options_selected, update_workflow=True): def assess_training_example(submission_uuid, options_selected, update_workflow=True):
""" """
Assess a training example and update the workflow. Assess a training example and update the workflow.
......
...@@ -27,12 +27,13 @@ class StudentTrainingWorkflow(models.Model): ...@@ -27,12 +27,13 @@ class StudentTrainingWorkflow(models.Model):
app_label = "assessment" app_label = "assessment"
@classmethod @classmethod
def get_or_create_workflow(cls, submission_uuid): def create_workflow(cls, submission_uuid):
""" """
Create a student training workflow. Create a student training workflow.
Args: Args:
submission_uuid (str): The UUID of the submission from the student being trained. submission_uuid (str): The UUID of the submission from the student
being trained.
Returns: Returns:
StudentTrainingWorkflow StudentTrainingWorkflow
...@@ -41,30 +42,42 @@ class StudentTrainingWorkflow(models.Model): ...@@ -41,30 +42,42 @@ class StudentTrainingWorkflow(models.Model):
SubmissionError: There was an error retrieving the submission. SubmissionError: There was an error retrieving the submission.
""" """
# Try to retrieve an existing workflow
# If we find one, return it immediately
try:
return cls.objects.get(submission_uuid=submission_uuid) # pylint:disable=E1101
except cls.DoesNotExist:
pass
# Retrieve the student item info # Retrieve the student item info
submission = sub_api.get_submission_and_student(submission_uuid) submission = sub_api.get_submission_and_student(submission_uuid)
student_item = submission['student_item'] student_item = submission['student_item']
# Create the workflow # Create the workflow
try: try:
return cls.objects.create( workflow, __ = cls.objects.get_or_create(
submission_uuid=submission_uuid, submission_uuid=submission_uuid,
student_id=student_item['student_id'], student_id=student_item['student_id'],
item_id=student_item['item_id'], item_id=student_item['item_id'],
course_id=student_item['course_id'] course_id=student_item['course_id']
) )
return workflow
# If we get an integrity error, it means we've violated a uniqueness constraint # If we get an integrity error, it means we've violated a uniqueness constraint
# (someone has created this object after we checked if it existed) # (someone has created this object after we checked if it existed)
# We can therefore assume that the object exists and we can retrieve it. # We can therefore assume that the object exists and do nothing.
except IntegrityError: except IntegrityError:
return cls.objects.get(submission_uuid=submission_uuid) pass
@classmethod
def get_workflow(cls, submission_uuid):
"""
Get a student training workflow.
Args:
submission_uuid (str): The UUID of the submission from the student
being trained.
Returns:
StudentTrainingWorkflow. None if no workflow is found.
"""
try:
return cls.objects.get(submission_uuid=submission_uuid) # pylint:disable=E1101
except cls.DoesNotExist:
return None
@property @property
def num_completed(self): def num_completed(self):
...@@ -132,15 +145,11 @@ class StudentTrainingWorkflow(models.Model): ...@@ -132,15 +145,11 @@ class StudentTrainingWorkflow(models.Model):
# If we get an integrity error, it means we've violated a uniqueness constraint # If we get an integrity error, it means we've violated a uniqueness constraint
# (someone has created this object after we checked if it existed) # (someone has created this object after we checked if it existed)
# Since the object already exists, we don't need to do anything # Since the object already exists, we don't need to do anything
# However, the example might not be the one we intended to use, so # Use the example passed into the function, because attempting to
# we need to retrieve the actual training example. # retrieve the stored example would result in an race condition.
except IntegrityError: except IntegrityError:
workflow = StudentTrainingWorkflowItem.objects.get( pass
workflow=self, order_num=order_num return next_example
)
return workflow.training_example
else:
return next_example
@property @property
def current_item(self): def current_item(self):
......
...@@ -26,6 +26,7 @@ class StudentTrainingAssessmentTest(CacheResetTest): ...@@ -26,6 +26,7 @@ class StudentTrainingAssessmentTest(CacheResetTest):
Create a submission. Create a submission.
""" """
submission = sub_api.create_submission(STUDENT_ITEM, ANSWER) submission = sub_api.create_submission(STUDENT_ITEM, ANSWER)
training_api.create_student_training_workflow(submission['uuid'])
self.submission_uuid = submission['uuid'] self.submission_uuid = submission['uuid']
def test_training_workflow(self): def test_training_workflow(self):
...@@ -102,7 +103,7 @@ class StudentTrainingAssessmentTest(CacheResetTest): ...@@ -102,7 +103,7 @@ class StudentTrainingAssessmentTest(CacheResetTest):
# This will need to create the student training workflow and the first item # This will need to create the student training workflow and the first item
# NOTE: we *could* cache the rubric model to reduce the number of queries here, # NOTE: we *could* cache the rubric model to reduce the number of queries here,
# but we're selecting it by content hash, which is indexed and should be plenty fast. # but we're selecting it by content hash, which is indexed and should be plenty fast.
with self.assertNumQueries(6): with self.assertNumQueries(4):
training_api.get_training_example(self.submission_uuid, RUBRIC, EXAMPLES) training_api.get_training_example(self.submission_uuid, RUBRIC, EXAMPLES)
# Without assessing the first training example, try to retrieve a training example. # Without assessing the first training example, try to retrieve a training example.
...@@ -121,6 +122,7 @@ class StudentTrainingAssessmentTest(CacheResetTest): ...@@ -121,6 +122,7 @@ class StudentTrainingAssessmentTest(CacheResetTest):
def test_submitter_is_finished_num_queries(self): def test_submitter_is_finished_num_queries(self):
# Complete the first training example # Complete the first training example
training_api.create_student_training_workflow(self.submission_uuid)
training_api.get_training_example(self.submission_uuid, RUBRIC, EXAMPLES) training_api.get_training_example(self.submission_uuid, RUBRIC, EXAMPLES)
training_api.assess_training_example(self.submission_uuid, EXAMPLES[0]['options_selected']) training_api.assess_training_example(self.submission_uuid, EXAMPLES[0]['options_selected'])
...@@ -321,6 +323,7 @@ class StudentTrainingAssessmentTest(CacheResetTest): ...@@ -321,6 +323,7 @@ class StudentTrainingAssessmentTest(CacheResetTest):
""" """
pre_submission = sub_api.create_submission(STUDENT_ITEM, ANSWER) pre_submission = sub_api.create_submission(STUDENT_ITEM, ANSWER)
training_api.create_student_training_workflow(pre_submission['uuid'])
for example in examples: for example in examples:
training_api.get_training_example(pre_submission['uuid'], rubric, examples) training_api.get_training_example(pre_submission['uuid'], rubric, examples)
training_api.assess_training_example(pre_submission['uuid'], example['options_selected']) training_api.assess_training_example(pre_submission['uuid'], example['options_selected'])
...@@ -17,7 +17,7 @@ class StudentTrainingWorkflowTest(CacheResetTest): ...@@ -17,7 +17,7 @@ class StudentTrainingWorkflowTest(CacheResetTest):
""" """
@mock.patch.object(StudentTrainingWorkflow.objects, 'get') @mock.patch.object(StudentTrainingWorkflow.objects, 'get')
@mock.patch.object(StudentTrainingWorkflow.objects, 'create') @mock.patch.object(StudentTrainingWorkflow.objects, 'get_or_create')
def test_create_workflow_integrity_error(self, mock_create, mock_get): def test_create_workflow_integrity_error(self, mock_create, mock_get):
# Simulate a race condition in which someone creates a workflow # Simulate a race condition in which someone creates a workflow
# after we check if it exists. This will violate the database uniqueness # after we check if it exists. This will violate the database uniqueness
...@@ -28,27 +28,25 @@ class StudentTrainingWorkflowTest(CacheResetTest): ...@@ -28,27 +28,25 @@ class StudentTrainingWorkflowTest(CacheResetTest):
# The second time, we should get the workflow created by someone else # The second time, we should get the workflow created by someone else
mock_workflow = mock.MagicMock(StudentTrainingWorkflow) mock_workflow = mock.MagicMock(StudentTrainingWorkflow)
mock_get.side_effect = [ mock_get.side_effect = [
StudentTrainingWorkflow.DoesNotExist,
mock_workflow mock_workflow
] ]
# Expect that we retry and retrieve the workflow that someone else created # Expect that we retry and retrieve the workflow that someone else created
submission = sub_api.create_submission(STUDENT_ITEM, ANSWER) submission = sub_api.create_submission(STUDENT_ITEM, ANSWER)
workflow = StudentTrainingWorkflow.get_or_create_workflow(submission['uuid']) StudentTrainingWorkflow.create_workflow(submission['uuid'])
workflow = StudentTrainingWorkflow.get_workflow(submission['uuid'])
self.assertEqual(workflow, mock_workflow) self.assertEqual(workflow, mock_workflow)
@mock.patch.object(StudentTrainingWorkflowItem.objects, 'get')
@mock.patch.object(StudentTrainingWorkflowItem.objects, 'create') @mock.patch.object(StudentTrainingWorkflowItem.objects, 'create')
def test_create_workflow_item_integrity_error(self, mock_create, mock_get): def test_create_workflow_item_integrity_error(self, mock_create):
# Create a submission and workflow # Create a submission and workflow
submission = sub_api.create_submission(STUDENT_ITEM, ANSWER) submission = sub_api.create_submission(STUDENT_ITEM, ANSWER)
workflow = StudentTrainingWorkflow.get_or_create_workflow(submission['uuid']) workflow = StudentTrainingWorkflow.create_workflow(submission['uuid'])
# Simulate a race condition in which someone creates a workflow item # Simulate a race condition in which someone creates a workflow item
# after we check if it exists. # after we check if it exists.
mock_workflow_item = mock.MagicMock(StudentTrainingWorkflowItem) mock_workflow_item = mock.MagicMock(StudentTrainingWorkflowItem)
mock_create.side_effect = IntegrityError mock_create.side_effect = IntegrityError
mock_get.return_value = mock_workflow_item
# Expect that we retry and retrieve the workflow item created by someone else # Expect that we retry and retrieve the workflow item created by someone else
self.assertEqual(workflow.next_training_example(EXAMPLES), mock_workflow_item.training_example) self.assertEqual(workflow.next_training_example(EXAMPLES), EXAMPLES[0])
...@@ -8,7 +8,10 @@ import logging ...@@ -8,7 +8,10 @@ import logging
from django.db import DatabaseError from django.db import DatabaseError
from openassessment.assessment.api import peer as peer_api from openassessment.assessment.api import peer as peer_api
from openassessment.assessment.errors import PeerAssessmentError from openassessment.assessment.api import student_training as training_api
from openassessment.assessment.errors import (
PeerAssessmentError, StudentTrainingInternalError
)
from submissions import api as sub_api from submissions import api as sub_api
from .models import AssessmentWorkflow, AssessmentWorkflowStep from .models import AssessmentWorkflow, AssessmentWorkflowStep
from .serializers import AssessmentWorkflowSerializer from .serializers import AssessmentWorkflowSerializer
...@@ -140,6 +143,12 @@ def create_workflow(submission_uuid, steps): ...@@ -140,6 +143,12 @@ def create_workflow(submission_uuid, steps):
status = AssessmentWorkflow.STATUS.self status = AssessmentWorkflow.STATUS.self
elif steps[0] == "training": elif steps[0] == "training":
status = AssessmentWorkflow.STATUS.training status = AssessmentWorkflow.STATUS.training
try:
training_api.create_student_training_workflow(submission_uuid)
except StudentTrainingInternalError as err:
err_msg = u"Could not create assessment workflow: {}".format(err)
logger.exception(err_msg)
raise AssessmentWorkflowInternalError(err_msg)
try: try:
workflow = AssessmentWorkflow.objects.create( workflow = AssessmentWorkflow.objects.create(
......
...@@ -56,7 +56,7 @@ class TestAssessmentWorkflowApi(CacheResetTest): ...@@ -56,7 +56,7 @@ class TestAssessmentWorkflowApi(CacheResetTest):
def test_update_peer_workflow(self): def test_update_peer_workflow(self):
submission = sub_api.create_submission(ITEM_1, "Shoot Hot Rod") submission = sub_api.create_submission(ITEM_1, "Shoot Hot Rod")
workflow = workflow_api.create_workflow(submission["uuid"], ["training", "peer"]) workflow = workflow_api.create_workflow(submission["uuid"], ["training", "peer"])
StudentTrainingWorkflow.get_or_create_workflow(submission_uuid=submission["uuid"]) StudentTrainingWorkflow.create_workflow(submission_uuid=submission["uuid"])
requirements = { requirements = {
"training": { "training": {
"num_required": 2 "num_required": 2
......
...@@ -232,7 +232,7 @@ class StudentTrainingRenderTest(StudentTrainingAssessTest): ...@@ -232,7 +232,7 @@ class StudentTrainingRenderTest(StudentTrainingAssessTest):
self._assert_path_and_context(xblock, expected_template, expected_context) self._assert_path_and_context(xblock, expected_template, expected_context)
@scenario('data/student_training.xml', user_id="Plato") @scenario('data/student_training.xml', user_id="Plato")
@patch.object(StudentTrainingWorkflow, "get_or_create_workflow") @patch.object(StudentTrainingWorkflow, "get_workflow")
def test_internal_error(self, xblock, mock_workflow): def test_internal_error(self, xblock, mock_workflow):
mock_workflow.side_effect = DatabaseError("Oh no.") mock_workflow.side_effect = DatabaseError("Oh no.")
xblock.create_submission(xblock.get_student_item_dict(), self.SUBMISSION) xblock.create_submission(xblock.get_student_item_dict(), self.SUBMISSION)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment