From 2571b6c1d960e02afa382c0f2cbcd1a61c63a34c Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Sun, 14 Jul 2024 07:44:29 +0800 Subject: [PATCH] fix(codaveri-live-feedback): remove filename coercion logic --- ...ramming_codaveri_async_feedback_service.rb | 27 ------------------ .../components/answers/Programming/index.jsx | 28 +++++++++---------- .../programming_codaveri_feedback_job_spec.rb | 4 +-- .../stubs/codaveri/feedback_api_stubs.rb | 5 ++-- 4 files changed, 17 insertions(+), 47 deletions(-) diff --git a/app/services/course/assessment/answer/programming_codaveri_async_feedback_service.rb b/app/services/course/assessment/answer/programming_codaveri_async_feedback_service.rb index 9806a03239b..276dab89485 100644 --- a/app/services/course/assessment/answer/programming_codaveri_async_feedback_service.rb +++ b/app/services/course/assessment/answer/programming_codaveri_async_feedback_service.rb @@ -58,15 +58,6 @@ def save_codaveri_feedback(response_body) private - # TODO: revert adapter when Codaveri no longer requires file path coercion - # Check if any object in the array has the :path attribute set to "main.py" - # If none do, coerce the first element to do so - def ensure_main_path!(objects, main_path) - return unless objects.any? && objects.none? { |obj| obj[:path] == main_path } - - objects.first[:path] = main_path - end - # Grades into the given +Course::Assessment::Answer::AutoGrading+ object. This assigns the grade # and makes sure answer is in the correct state. # @@ -90,7 +81,6 @@ def construct_feedback_object # rubocop:disable Metrics/AbcSize @answer_object[:files].append(file_template) end - ensure_main_path!(@answer_object[:files], 'main.py') @answer_object end @@ -113,31 +103,14 @@ def request_codaveri_feedback end def process_codaveri_feedback - main_path_parsed = false @answer_files.each do |file| feedback_lines = @feedback_files_hash[file.filename] - main_path_parsed = true if file.filename == 'main.py' next if feedback_lines.nil? feedback_lines.each do |line| save_annotation(file, line) end end - return if main_path_parsed - - process_main_file_codaveri_feedback - end - - # We do inverse of name coercion logic: - # if main.py does not exist in answer_files but it does in feedback_files_hash, - # run save_annotation for the first file in the array with feedback_lines taken from feedback_files_hash['main.py'] - def process_main_file_codaveri_feedback - return unless @answer_files.any? && @feedback_files_hash.key?('main.py') - - feedback_lines = @feedback_files_hash['main.py'] - feedback_lines.each do |line| - save_annotation(@answer_files[0], line) - end end def save_annotation(file, feedback_line) # rubocop:disable Metrics/AbcSize diff --git a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx index 1f3873dfe21..62bdfd435f3 100644 --- a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx +++ b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx @@ -151,7 +151,7 @@ const ProgrammingFiles = ({ } }; - const renderFeedbackCard = (feedbackItem) => { + const renderFeedbackCard = (filename, feedbackItem) => { let cardStyle = styles.card; if (feedbackItem.state === 'resolved') { cardStyle = { ...styles.card, ...styles.cardResolved }; @@ -199,7 +199,7 @@ const ProgrammingFiles = ({ type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_RESOLVED, payload: { questionId, - path: 'main.py', + path: filename, lineId: feedbackItem.id, }, }); @@ -223,7 +223,7 @@ const ProgrammingFiles = ({ type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_DISMISSED, payload: { questionId, - path: 'main.py', + path: filename, lineId: feedbackItem.id, }, }); @@ -246,7 +246,7 @@ const ProgrammingFiles = ({ type: actionTypes.LIVE_FEEDBACK_ITEM_DELETE, payload: { questionId, - path: 'main.py', + path: filename, lineId: feedbackItem.id, }, }); @@ -282,7 +282,7 @@ const ProgrammingFiles = ({ ); }; - const renderFeedbackDrawer = (keyString, annotations) => ( + const renderFeedbackDrawer = (keyString, annotations, filename) => ( -
{annotations.map(renderFeedbackCard)}
+
+ {annotations.map((feedbackItem) => + renderFeedbackCard(filename, feedbackItem), + )} +
); @@ -311,14 +315,7 @@ const ProgrammingFiles = ({ highlightedContent: field.highlightedContent, }; - let annotations = feedbackFiles[field.filename] ?? []; - // TODO: remove special casing around Codaveri name coercion issue - if ( - index === 0 && - !controlledProgrammingFields.some((elem) => elem.filename === 'main.py') - ) { - annotations = feedbackFiles['main.py'] ?? []; - } + const annotations = feedbackFiles[field.filename] ?? []; const keyString = `editor-container-${index}`; const shouldRenderDrawer = annotations.length > 0; @@ -337,7 +334,8 @@ const ProgrammingFiles = ({ saveAnswerAndUpdateClientVersion={saveAnswerAndUpdateClientVersion} /> - {shouldRenderDrawer && renderFeedbackDrawer(keyString, annotations)} + {shouldRenderDrawer && + renderFeedbackDrawer(keyString, annotations, field.filename)} ); }); diff --git a/spec/jobs/course/assessment/answer/programming_codaveri_feedback_job_spec.rb b/spec/jobs/course/assessment/answer/programming_codaveri_feedback_job_spec.rb index 0526bc449e5..cbfc3e43eb5 100644 --- a/spec/jobs/course/assessment/answer/programming_codaveri_feedback_job_spec.rb +++ b/spec/jobs/course/assessment/answer/programming_codaveri_feedback_job_spec.rb @@ -89,7 +89,7 @@ codaveri_feedback = post.codaveri_feedback expect(codaveri_feedback.codaveri_feedback_id).to eq( - '6311a0548c57aae93d260927:main.py:63141b108c57aae93d260a00' + '6311a0548c57aae93d260927:template.py:63141b108c57aae93d260a00' ) expect(codaveri_feedback.status).to eq('pending_review') expect(codaveri_feedback.original_feedback).to eq('This is a test feedback') @@ -145,7 +145,7 @@ codaveri_feedback = post.codaveri_feedback expect(codaveri_feedback.codaveri_feedback_id).to eq( - '6311a0548c57aae93d260927:main.py:63141b108c57aae93d260a00' + '6311a0548c57aae93d260927:template.py:63141b108c57aae93d260a00' ) expect(codaveri_feedback.status).to eq('pending_review') expect(codaveri_feedback.original_feedback).to eq('This is a test feedback') diff --git a/spec/support/stubs/codaveri/feedback_api_stubs.rb b/spec/support/stubs/codaveri/feedback_api_stubs.rb index b2ea794fec3..cf195cbd5d3 100644 --- a/spec/support/stubs/codaveri/feedback_api_stubs.rb +++ b/spec/support/stubs/codaveri/feedback_api_stubs.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true module Codaveri::FeedbackApiStubs - # TODO: update this fixture when name coercion logic is removed FEEDBACK_SUCCESS_FINAL_RESULT = { status: 200, body: { @@ -9,10 +8,10 @@ module Codaveri::FeedbackApiStubs data: { feedbackFiles: [ { - path: 'main.py', + path: 'template.py', feedbackLines: [ { - id: '6311a0548c57aae93d260927:main.py:63141b108c57aae93d260a00', + id: '6311a0548c57aae93d260927:template.py:63141b108c57aae93d260a00', linenum: 5, feedback: 'This is a test feedback', category: 'syntax',