fix(codaveri-live-feedback): remove filename coercion logic

Coursemology · Jul 17, 2024 · 2571b6c · 2571b6c
1 parent 89532a6
commit 2571b6c
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 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
@@ -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
@@ -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) => (
     <Drawer
       anchor="right"
       ModalProps={{
@@ -294,7 +294,11 @@ const ProgrammingFiles = ({
       PaperProps={{ style: styles.drawerPaper }}
       variant="persistent"
     >
-      <div>{annotations.map(renderFeedbackCard)}</div>
+      <div>
+        {annotations.map((feedbackItem) =>
+          renderFeedbackCard(filename, feedbackItem),
+        )}
+      </div>
     </Drawer>
   );
 
@@ -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}
           />
         </Box>
-        {shouldRenderDrawer && renderFeedbackDrawer(keyString, annotations)}
+        {shouldRenderDrawer &&
+          renderFeedbackDrawer(keyString, annotations, field.filename)}
       </div>
     );
   });

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
@@ -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
@@ -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',