From c18c35377652250e3ddc57a59d67eaca8c06cb49 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 17 Nov 2022 15:22:02 -0800 Subject: [PATCH 1/4] Allow exercises with raw_data set to be complete. --- contentcuration/contentcuration/models.py | 15 +++++++++------ .../contentcuration/tests/test_contentnodes.py | 9 +++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 121bea10dc..af0578bf37 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1756,12 +1756,15 @@ def mark_complete(self): # noqa C901 if self.kind_id == content_kinds.EXERCISE: # Check to see if the exercise has at least one assessment item that has: if not self.assessment_items.filter( - # A non-blank question - ~Q(question='') - # Non-blank answers - & ~Q(answers='[]') - # With either an input question or one answer marked as correct - & (Q(type=exercises.INPUT_QUESTION) | Q(answers__iregex=r'"correct":\s*true')) + # Item with non-blank raw data + ~Q(raw_data="") | ( + # A non-blank question + ~Q(question='') + # Non-blank answers + & ~Q(answers='[]') + # With either an input question or one answer marked as correct + & (Q(type=exercises.INPUT_QUESTION) | Q(answers__iregex=r'"correct":\s*true')) + ) ).exists(): errors.append("No questions with question text and complete answers") # Check that it has a mastery model set diff --git a/contentcuration/contentcuration/tests/test_contentnodes.py b/contentcuration/contentcuration/tests/test_contentnodes.py index 25569f8988..06864ecd3f 100644 --- a/contentcuration/contentcuration/tests/test_contentnodes.py +++ b/contentcuration/contentcuration/tests/test_contentnodes.py @@ -1061,6 +1061,15 @@ def test_create_exercise_valid_assessment_items(self): new_obj.mark_complete() self.assertTrue(new_obj.complete) + def test_create_exercise_valid_assessment_items_raw_data(self): + licenses = list(License.objects.filter(copyright_holder_required=False, is_custom=False).values_list("pk", flat=True)) + channel = testdata.channel() + new_obj = ContentNode(title="yes", kind_id=content_kinds.EXERCISE, parent=channel.main_tree, license_id=licenses[0], extra_fields=self.new_extra_fields) + new_obj.save() + AssessmentItem.objects.create(contentnode=new_obj, raw_data="{\"question\": {}}") + new_obj.mark_complete() + self.assertTrue(new_obj.complete) + def test_create_exercise_no_extra_fields(self): licenses = list(License.objects.filter(copyright_holder_required=False, is_custom=False).values_list("pk", flat=True)) channel = testdata.channel() From 23021fb87d2f8ba31ae040a2c046664204160b24 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 21 Nov 2022 15:23:57 -0800 Subject: [PATCH 2/4] Revert server side ordering to fix performance issues. --- .../views/ImportFromChannels/ChannelList.vue | 1 - .../contentcuration/viewsets/channel.py | 17 ++++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue index 58bb5390af..dfa961a3b6 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue @@ -124,7 +124,6 @@ [this.channelFilter]: true, page: this.$route.query.page || 1, exclude: this.currentChannelId, - ordering: this.channelFilter === 'public' ? 'name' : '-modified', }).then(page => { this.pageCount = page.total_pages; this.channels = page.results; diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 4ab57c4595..07d27b7281 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -400,8 +400,6 @@ class ChannelViewSet(ValuesViewset): serializer_class = ChannelSerializer pagination_class = ChannelListPagination filterset_class = ChannelFilter - ordering_fields = ["modified", "name"] - ordering = "-modified" field_map = channel_field_map values = base_channel_values + ("edit", "view", "unpublished_changes") @@ -414,15 +412,6 @@ def get_queryset(self): queryset = super(ChannelViewSet, self).get_queryset() user_id = not self.request.user.is_anonymous and self.request.user.id user_queryset = User.objects.filter(id=user_id) - # Add the last modified node modified value as the channel last modified - channel_main_tree_nodes = ContentNode.objects.filter( - tree_id=OuterRef("main_tree__tree_id") - ) - queryset = queryset.annotate( - modified=Subquery( - channel_main_tree_nodes.values("modified").order_by("-modified")[:1] - ) - ) return queryset.annotate( edit=Exists(user_queryset.filter(editable_channels=OuterRef("id"))), @@ -445,6 +434,12 @@ def annotate_queryset(self, queryset): queryset = queryset.annotate( count=SQCount(non_topic_content_ids, field="content_id"), ) + # Add the last modified node modified value as the channel last modified + queryset = queryset.annotate( + modified=Subquery( + channel_main_tree_nodes.values("modified").order_by("-modified")[:1] + ) + ) queryset = queryset.annotate(unpublished_changes=Exists(_unpublished_changes_query(OuterRef("id")))) From 912583294feca5038d17e4e8504c2809a82be743 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 21 Nov 2022 16:06:52 -0800 Subject: [PATCH 3/4] Simplify sentry request error collection. --- .../contentcuration/frontend/shared/client.js | 49 ++++--------------- 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/client.js b/contentcuration/contentcuration/frontend/shared/client.js index a37c9cc7fa..6588c77c3e 100644 --- a/contentcuration/contentcuration/frontend/shared/client.js +++ b/contentcuration/contentcuration/frontend/shared/client.js @@ -32,12 +32,11 @@ const client = axios.create({ client.interceptors.response.use( response => response, error => { - let message; - let url; - let config; + const url = error.config.url; + let message = error.message; + let status = 0; if (error.response) { - config = error.response.config; - url = config.url; + status = error.response.status; message = error.response.statusText; // Don't send a Sentry report for permissions errors // Many 404s are in fact also unauthorized requests so @@ -45,48 +44,20 @@ client.interceptors.response.use( // to catch legitimate request issues in the backend. // // Allow 412 too as that's specific to out of storage checks - if ( - error.response.status === 403 || - error.response.status === 404 || - error.response.status === 405 || - error.response.status === 412 - ) { + if (status === 403 || status === 404 || status === 405 || status === 412) { return Promise.reject(error); } - - if (error.response.status === 0) { - message = 'Network Error: ' + url; - } - - // Put the URL in the main message for timeouts - // so we can see which timeouts are most frequent. - if (error.response.status === 504) { - message = 'Request Timed Out: ' + url; - } - } else if (error.request && error.request.config) { - // Request was sent but no response received - config = error.request.config; - url = config.url; - message = 'Network Error: ' + url; - } else { - message = error.message; } - const extraData = { - url, - type: config ? config.responseType : null, - data: config ? config.data : null, - status: error.response ? error.response.status : null, - error: message, - response: error.response ? error.response.data : null, - }; + message = message ? `${message}: ${url}` : `Network Error: ${url}`; + if (process.env.NODE_ENV !== 'production') { // In dev build log warnings to console for developer use console.warn('AJAX Request Error: ' + message); // eslint-disable-line no-console - console.warn('Error data: ' + JSON.stringify(extraData)); // eslint-disable-line no-console + console.warn('Error data: ' + JSON.stringify(error)); // eslint-disable-line no-console } else { - Sentry.captureMessage(message, { - extra: extraData, + Sentry.captureException(new Error(message), { + contexts: { error }, }); } return Promise.reject(error); From 9f0da6985e3a9e01ded23f9eb4736081ffc0e1b8 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 22 Nov 2022 10:27:20 -0800 Subject: [PATCH 4/4] Upload error as attachment. --- .../contentcuration/frontend/shared/client.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/client.js b/contentcuration/contentcuration/frontend/shared/client.js index 6588c77c3e..056f120827 100644 --- a/contentcuration/contentcuration/frontend/shared/client.js +++ b/contentcuration/contentcuration/frontend/shared/client.js @@ -54,10 +54,23 @@ client.interceptors.response.use( if (process.env.NODE_ENV !== 'production') { // In dev build log warnings to console for developer use console.warn('AJAX Request Error: ' + message); // eslint-disable-line no-console - console.warn('Error data: ' + JSON.stringify(error)); // eslint-disable-line no-console + console.warn('Error data: ', error); // eslint-disable-line no-console } else { - Sentry.captureException(new Error(message), { - contexts: { error }, + Sentry.withScope(function(scope) { + scope.addAttachment({ + filename: 'error.json', + data: JSON.stringify(error), + contentType: 'application/json', + }); + Sentry.captureException(new Error(message), { + extra: { + Request: { + headers: error.config.headers, + method: error.config.method, + url, + }, + }, + }); }); } return Promise.reject(error);