Skip to content

Commit 8d8da47

Browse files
committed
Report meaningful errors for incomplete node.
Report incomplete chef nodes on sentry, but do not mark as incomplete yet.
1 parent 890936f commit 8d8da47

File tree

2 files changed

+45
-18
lines changed

2 files changed

+45
-18
lines changed

contentcuration/contentcuration/models.py

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,38 +1723,44 @@ def recalculate_editors_storage(self):
17231723
for editor in self.files.values_list('uploaded_by_id', flat=True).distinct():
17241724
calculate_user_storage(editor)
17251725

1726-
def mark_complete(self):
1726+
def mark_complete(self): # noqa C901
1727+
errors = []
17271728
# Is complete if title is falsy but only if not a root node.
1728-
is_complete = (bool(self.title) or self.parent_id is None)
1729+
if not (bool(self.title) or self.parent_id is None):
1730+
errors.append("Empty title")
17291731
if self.kind_id != content_kinds.TOPIC:
1730-
is_complete = is_complete and bool(self.license)
1731-
if self.license and self.license.is_custom:
1732-
is_complete = is_complete and bool(self.license_description)
1733-
if self.license and self.license.copyright_holder_required:
1734-
is_complete = is_complete and bool(self.copyright_holder)
1735-
if self.kind_id != content_kinds.EXERCISE:
1736-
is_complete = is_complete and self.files.filter(preset__supplementary=False).exists()
1737-
else:
1732+
if not self.license:
1733+
errors.append("Missing license")
1734+
if self.license and self.license.is_custom and not self.license_description:
1735+
errors.append("Missing license description for custom license")
1736+
if self.license and self.license.copyright_holder_required and not self.copyright_holder:
1737+
errors.append("Missing required copyright holder")
1738+
if self.kind_id != content_kinds.EXERCISE and not self.files.filter(preset__supplementary=False).exists():
1739+
errors.append("Missing default file")
1740+
if self.kind_id == content_kinds.EXERCISE:
17381741
# Check to see if the exercise has at least one assessment item that has:
1739-
is_complete = is_complete and self.assessment_items.filter(
1742+
if not self.assessment_items.filter(
17401743
# A non-blank question
17411744
~Q(question='')
17421745
# Non-blank answers
17431746
& ~Q(answers='[]')
17441747
# With either an input question or one answer marked as correct
17451748
& (Q(type=exercises.INPUT_QUESTION) | Q(answers__iregex=r'"correct":\s*true'))
1746-
).exists()
1749+
).exists():
1750+
errors.append("No questions with question text and complete answers")
17471751
# Check that it has a mastery model set
17481752
# Either check for the previous location for the mastery model, or rely on our completion criteria validation
17491753
# that if it has been set, then it has been set correctly.
17501754
criterion = self.extra_fields.get("options", {}).get("completion_criteria")
1751-
is_complete = is_complete and (self.extra_fields.get("mastery_model") or criterion)
1755+
if not (self.extra_fields.get("mastery_model") or criterion):
1756+
errors.append("Missing mastery criterion")
17521757
if criterion:
17531758
try:
17541759
completion_criteria.validate(criterion, kind=content_kinds.EXERCISE)
17551760
except completion_criteria.ValidationError:
1756-
is_complete = False
1757-
self.complete = is_complete
1761+
errors.append("Mastery criterion is defined but is invalid")
1762+
self.complete = not errors
1763+
return errors
17581764

17591765
def on_create(self):
17601766
self.changed = True

contentcuration/contentcuration/views/internal.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
from contentcuration.utils.nodes import map_files_to_assessment_item
5454
from contentcuration.utils.nodes import map_files_to_node
5555
from contentcuration.utils.nodes import map_files_to_slideshow_slide_item
56+
from contentcuration.utils.sentry import report_exception
5657
from contentcuration.utils.tracing import trace
5758
from contentcuration.viewsets.sync.constants import CHANNEL
5859
from contentcuration.viewsets.sync.utils import generate_publish_event
@@ -555,6 +556,20 @@ def create_channel(channel_data, user):
555556
return channel # Return new channel
556557

557558

559+
class IncompleteNodeError(Exception):
560+
"""
561+
Used to track incomplete nodes from ricecooker. We don't raise this error,
562+
just feed it to Sentry for reporting.
563+
"""
564+
565+
def __init__(self, node, errors):
566+
self.message = (
567+
"Node {} had the following errors: {}".format(node, ",".join(errors))
568+
)
569+
570+
super(IncompleteNodeError, self).__init__(self.message)
571+
572+
558573
@trace
559574
def convert_data_to_nodes(user, content_data, parent_node):
560575
""" Parse dict and create nodes accordingly """
@@ -596,10 +611,16 @@ def convert_data_to_nodes(user, content_data, parent_node):
596611
user, new_node, slides, node_data["files"]
597612
)
598613

599-
new_node.mark_complete()
614+
# Wait until after files have been set on the node to check for node completeness
615+
# as some node kinds are counted as incomplete if they lack a default file.
616+
completion_errors = new_node.mark_complete()
600617

601-
if not new_node.complete:
602-
new_node.save()
618+
if completion_errors:
619+
try:
620+
# we need to raise it to get Python to fill out the stack trace.
621+
raise IncompleteNodeError(new_node, completion_errors)
622+
except IncompleteNodeError as e:
623+
report_exception(e)
603624

604625
# Track mapping between newly created node and node id
605626
root_mapping.update({node_data["node_id"]: new_node.pk})

0 commit comments

Comments
 (0)