Fix order of annotations on feedback view and file view to be consistent

app/controllers/application_controller.rb

@@ -331,6 +331,22 @@ def make_dlist(cuds)
     emails.join(",")
   end
 
+  # gets a filename string to be able to sort files properly
+  # by changing the filename if it's the autograded output
+  # or it's an archived file
+  def get_correct_filename(annotation, files, submission)
+    if annotation.position == -1
+      # position -1 maps to the Autograder Output
+      "Autograder Output"
+    elsif files && annotation.position != 0
+      # if the submission is an archive, use filename in archive;
+      # otherwise, use submission filename
+      Archive.get_nth_filename(files, annotation.position)
+    else
+      submission.filename
+    end
+  end
+
 private
 
   # called on Exceptions.  Shows a stack trace to course assistants, and above.

app/controllers/assessments_controller.rb

@@ -582,6 +582,10 @@ def viewFeedback
       @files = Archive.get_files @submission.handin_file_path
     end
     @problemReleased = @submission.scores.pluck(:released).all?
+    # get_correct_filename is protected, so we wrap around controller-specific call
+    @get_correct_filename = ->(annotation) {
+      get_correct_filename(annotation, @files, @submission)
+    }
   end
 
   def parseScore(feedback)

app/controllers/submissions_controller.rb

@@ -461,10 +461,10 @@ def view
       value = annotation.value || 0
       line = annotation.line
       problem = annotation.problem ? annotation.problem.name : "General"
-
+      filename = get_correct_filename(annotation, @files, @submission)
       @problemSummaries[problem] ||= []
       @problemSummaries[problem] << [description, value, line, annotation.submitted_by,
-                                     annotation.id, annotation.position]
+                                     annotation.id, annotation.position, filename]
 
       @problemGrades[problem] ||= 0
       @problemGrades[problem] += value

app/views/assessments/viewFeedback.html.erb

@@ -23,18 +23,23 @@ Yes, feedback is per problem but we show all annotations for this submission any
   <h2 style="margin-top: 1em">Remarks</h2>
 
   <ul>
-  <!-- TODO: Make order of annotations consistent with order of file list  -->
-  <% @submission.annotations.each do |annotation| %>
+  <!-- use custom sort b/c using ordering doesn't work with autograder output, archived files -->
+  <% @subs = @submission.annotations.sort do |a,b|
+     aFilename = @get_correct_filename.call(a)
+     bFilename = @get_correct_filename.call(b)
+     case
+       when aFilename < bFilename
+         -1
+       when aFilename > bFilename
+          1
+       else
+          a.line <=> b.line
+       end
+     end
+     @subs.each do |annotation| %>
     <li>
       <%= link_to annotation.as_text, [:view, @course, @assessment, @submission, header_position: annotation.position] %>
-      <% filename = if annotation.position == -1
-                     # position -1 maps to the Autograder Output
-                     "Autograder Output"
-                   else
-                     # if the submission is an archive, use filename in archive; otherwise, use submission filename
-                     @files && annotation.position ? Archive.get_nth_filename(@files, annotation.position) : @submission.filename
-                   end
-      %>
+      <% filename = @get_correct_filename.call(annotation) %>
       (<%= filename %>: <%= annotation.line %>)
     </li>
   <% end %>

app/views/submissions/_annotation_pane.html.erb

@@ -15,7 +15,8 @@
             </div>
             <div class="collapsible-body">
               <% if @cud.instructor? or @cud.course_assistant? or @problemReleased%>
-                <% descriptTuples.each do |description, value, line, user, id, position| %>
+                <!-- a[6] = filename, a[2] = line-->
+                <% descriptTuples.sort_by{|a| [a[6], a[2]]}.each do |description, value, line, user, id, position, filename| %>
                   <div class="descript" id="li-annotation-<%= id %>"> <!-- Line + 1 because the code line numbers starts with 1 not 0. -->
                     <%= link_to(
                         url_for([:view, @course, @assessment, @submission, header_position: position.nil? ? 0 : position, line: line.nil? ? 1 : line+1]),
@@ -24,7 +25,7 @@
                         "data-line": line.nil? ? 1 : line+1,
                         remote: true,
                       ) do %>
-                        <span class="point_badge <%= value > 0 ? "positive" : value < 0 ? "negative" : "neutral" %>"><%= plus_fix(value) %></span> <%= description %>
+                        <span class="point_badge <%= value > 0 ? "positive" : value < 0 ? "negative" : "neutral" %>"><%= plus_fix(value) %></span> <%= description  %>
                     <% end %>
                   </div>
                 <% end %>