Refactored grading annotation routes to respond with JSON

Changelog.md

@@ -39,6 +39,7 @@
 - Updated images: moved externally-hosted images into repository, ensured images are vertically centred and displayed as new paragraphs, and cropped images to remove blank space (#8053)
 
 ### 🔧 Internal changes
+- Refactored annotation routes to respond with JSON rather than Javascript (#8127)
 - Removed `:blob` CSP exception for `img_src` for `groups_controller.rb` and `exam_templates_controller.rb`
 - Replaced `jcrop` with `cropperjs` for the exam template cover page crop selector (#8114)
 - Migrated `image_viewer.jsx` file to use `heic-convert` instead of `heic2any`; Updated CSP exceptions for controllers depending on `image_viewer.jsx` (#8100)

app/controllers/annotations_controller.rb

@@ -21,7 +21,7 @@ def add_existing_annotation
       **params.to_unsafe_h.slice(*annotation_class.required_fields).symbolize_keys,
       **base_attributes
     )
-    render :create
+    render json: @annotation.to_json(current_role: current_role)
   end
 
   def create
@@ -62,6 +62,7 @@ def create
       **params.to_unsafe_h.slice(*annotation_class.required_fields).symbolize_keys,
       **base_attributes
     )
+    render json: @annotation.to_json(current_role: current_role)
   end
 
   def destroy
@@ -79,13 +80,15 @@ def destroy
       end
     end
     text = @annotation.annotation_text
+    annotation_text_id = @annotation.annotation_text_id
     text.destroy if text.annotation_category_id.nil?
     @annotation.destroy
     result.annotations.reload.each do |annot|
       if annot.annotation_number > @annotation.annotation_number
         annot.update(annotation_number: annot.annotation_number - 1)
       end
     end
+    render json: { id: @annotation.id, annotation_text_id: annotation_text_id }
   end
 
   def update
@@ -99,7 +102,7 @@ def update
     end
 
     change_all = !params[:annotation_text] || !params[:annotation_text][:change_all] ||
-        params[:annotation_text][:change_all] == '1'
+        params[:annotation_text][:change_all].to_s == '1'
     if change_all
       @annotation_text.update(content: params[:content])
     else
@@ -114,6 +117,7 @@ def update
         @annotation.update(annotation_text: new_text)
       end
     end
+    render json: { annotation: @annotation.reload.get_data(include_creator: true) }
   end
 
   protected

app/javascript/Components/Result/result.jsx

@@ -296,6 +296,14 @@ class Result extends React.Component {
   };
 
   /* Callbacks for annotations */
+  // Headers for annotation routes, which are submitted as JSON via fetch rather than
+  // through jQuery, so the CSRF token needs to be attached by hand.
+  jsonHeaders = () => ({
+    "Content-Type": "application/json",
+    Accept: "application/json",
+    "X-CSRF-Token": document.querySelector('[name="csrf-token"]').content,
+  });
+
   newAnnotation = () => {
     const submission_file_id =
       this.leftPane.current.submissionFilePanel.current.state.selectedFile[1];
@@ -315,14 +323,18 @@ class Result extends React.Component {
 
     let onSubmit = formData => {
       let data = {...formData, ...metadata};
-      return $.post({
-        url: Routes.course_annotations_path(this.props.course_id),
-        data,
-      }).then(() => {
-        this.setState({
-          annotationModal: INITIAL_ANNOTATION_MODAL_STATE,
-        });
-      }); // Resetting back to original
+      return fetch(Routes.course_annotations_path(this.props.course_id), {
+        method: "POST",
+        headers: this.jsonHeaders(),
+        body: JSON.stringify(data),
+      })
+        .then(response => response.json())
+        .then(json => {
+          this.addAnnotation(json.annotation, json.mark_update);
+          this.setState({
+            annotationModal: INITIAL_ANNOTATION_MODAL_STATE,
+          });
+        }); // Resetting back to original
     };
 
     this.setState({
@@ -355,27 +367,20 @@ class Result extends React.Component {
     }
   };
 
-  addAnnotation = (
-    annotation,
-    criterion_id = null,
-    mark_value = null,
-    new_subtotal = null,
-    new_total = null,
-    new_num_marked = null
-  ) => {
+  addAnnotation = (annotation, markUpdate = null) => {
     this.setState({annotations: this.state.annotations.concat([annotation])});
 
-    if (!!criterion_id) {
+    if (markUpdate) {
       let newMarks = [...this.state.marks];
-      let i = newMarks.findIndex(m => m.id === criterion_id);
+      let i = newMarks.findIndex(m => m.id === markUpdate.criterion_id);
       if (i >= 0) {
         newMarks[i] = {...newMarks[i]};
-        newMarks[i].mark = mark_value;
+        newMarks[i].mark = markUpdate.mark;
         this.setState({
           marks: newMarks,
-          subtotal: new_subtotal,
-          total: new_total,
-          num_marked: new_num_marked,
+          subtotal: markUpdate.subtotal,
+          total: markUpdate.total,
+          num_marked: markUpdate.num_marked,
         });
       }
     }
@@ -404,7 +409,15 @@ class Result extends React.Component {
 
     data = this.extend_with_selection_data(data);
     if (data) {
-      $.post(Routes.add_existing_annotation_course_annotations_path(this.props.course_id), data);
+      fetch(Routes.add_existing_annotation_course_annotations_path(this.props.course_id), {
+        method: "POST",
+        headers: this.jsonHeaders(),
+        body: JSON.stringify(data),
+      })
+        .then(response => response.json())
+        .then(json => {
+          this.addAnnotation(json.annotation, json.mark_update);
+        });
     }
   };
 
@@ -424,7 +437,15 @@ class Result extends React.Component {
 
     data = this.extend_with_selection_data(data);
     if (data) {
-      $.post(Routes.course_annotations_path(this.props.course_id), data, undefined, "script");
+      fetch(Routes.course_annotations_path(this.props.course_id), {
+        method: "POST",
+        headers: this.jsonHeaders(),
+        body: JSON.stringify(data),
+      })
+        .then(response => response.json())
+        .then(json => {
+          this.addAnnotation(json.annotation, json.mark_update);
+        });
     }
   };
 
@@ -468,18 +489,24 @@ class Result extends React.Component {
 
     let onSubmit = formData => {
       let data = {...formData, ...metadata};
-      $.ajax({
-        url: Routes.course_annotation_path(this.props.course_id, annot_id),
-        data,
+      return fetch(Routes.course_annotation_path(this.props.course_id, annot_id), {
         method: "PUT",
-        dataType: "json",
-      }).always(() => {
-        this.setState({
-          annotationModal: INITIAL_ANNOTATION_MODAL_STATE,
+        headers: this.jsonHeaders(),
+        body: JSON.stringify(data),
+      })
+        .then(response => (response.ok ? response.json() : null))
+        .then(json => {
+          if (json) {
+            this.updateAnnotation(json.annotation);
+          }
+        })
+        .finally(() => {
+          this.setState({
+            annotationModal: INITIAL_ANNOTATION_MODAL_STATE,
+          });
+          this.refreshAnnotations();
+          this.refreshAnnotationCategories();
         });
-        this.refreshAnnotations();
-        this.refreshAnnotationCategories();
-      });
     };
 
     let annotation = this.state.annotations.find(
@@ -543,7 +570,7 @@ class Result extends React.Component {
     }
   }
 
-  destroyAnnotation(annotation_id, range, annotation_text_id) {
+  destroyAnnotation(annotation_id, annotation_text_id) {
     if (
       !!window.annotation_manager &&
       window.annotation_manager.annotation_text_manager.annotationTextExists(annotation_text_id)
@@ -565,15 +592,21 @@ class Result extends React.Component {
   }
 
   removeAnnotation = annot_id => {
-    $.ajax({
-      url: Routes.course_annotation_path(this.props.course_id, annot_id),
+    fetch(Routes.course_annotation_path(this.props.course_id, annot_id), {
       method: "DELETE",
-      data: {
+      headers: this.jsonHeaders(),
+      body: JSON.stringify({
         result_id: this.state.result_id,
         assignment_id: this.state.assignment_id,
-      },
-      dataType: "script",
-    }).then(this.fetchData);
+      }),
+    })
+      .then(response => (response.ok ? response.json() : null))
+      .then(json => {
+        if (json) {
+          this.destroyAnnotation(json.id, json.annotation_text_id);
+        }
+      })
+      .then(this.fetchData);
   };
 
   /* Callbacks for RightPane */
@@ -1026,12 +1059,9 @@ class Result extends React.Component {
                   submission_files={this.state.submission_files}
                   student_view={this.props.role === "Student"}
                   newAnnotation={this.newAnnotation}
-                  addAnnotation={this.addAnnotation}
                   addExistingAnnotation={this.addExistingAnnotation}
                   editAnnotation={this.editAnnotation}
-                  updateAnnotation={this.updateAnnotation}
                   removeAnnotation={this.removeAnnotation}
-                  destroyAnnotation={this.destroyAnnotation}
                   rmd_convert_enabled={this.props.rmd_convert_enabled}
                 />
               </Panel>

app/models/annotation.rb

@@ -105,6 +105,28 @@ def get_data(include_creator: false)
     data
   end
 
+  # Serializes this annotation for the Result component. When the annotation carries a
+  # deduction that isn't overridden, also includes the updated mark/subtotal/total so
+  # the front end can reflect the deduction without a full refetch.
+  def to_json(current_role: nil)
+    data = { annotation: get_data(include_creator: true) }
+
+    criterion = annotation_text.annotation_category&.flexible_criterion
+    mark = result.marks.find_by(criterion: criterion)
+    unless annotation_text.deduction.nil? || annotation_text.deduction == 0 || mark.override
+      grader_id = current_role&.instructor? ? nil : current_role&.id
+      data[:mark_update] = {
+        criterion_id: criterion.id,
+        mark: mark.mark,
+        subtotal: result.get_subtotal,
+        total: result.get_total_mark,
+        num_marked: result.grouping.assignment.get_num_marked(grader_id)
+      }
+    end
+
+    data.to_json
+  end
+
   private
 
   # check if the submission file is associated with a remark result or a released result