Check and apply max deduction for handgrading annotations

src/components/project_view/handgrading/handgrading.vue

@@ -278,6 +278,7 @@
 import { Component, Inject, Prop, Vue, Watch } from 'vue-property-decorator';
 
 import {
+  Annotation,
   AppliedAnnotation,
   AppliedAnnotationObserver,
   Comment,
@@ -495,24 +496,61 @@ export default class Handgrading extends Vue implements AppliedAnnotationObserve
     });
   }
 
+  unchecked_total_deduction_by_annotation_pk(annotation: Annotation) {
+    return this.d_handgrading_result!.applied_annotations
+      .filter(result_annotation => result_annotation.annotation.pk === annotation.pk)
+      .map(applied_annotation => applied_annotation.annotation.deduction)
+      .reduce((a, b) => a + b, 0);
+  }
+
+  limited_deduction_by_annotation_pk(unchecked_total_deduction: number, annotation: Annotation) {
+    if (annotation.max_deduction !== null && unchecked_total_deduction < annotation.max_deduction) {
+      return annotation.max_deduction;
+    }
+    return unchecked_total_deduction;
+  }
+
+  // Must call before applying change to this.d_handgrading_result.applied_annotations
+  deduction_change(annotation: Annotation, unchecked_score_change: number) {
+    const unchecked_deduction_before = this.unchecked_total_deduction_by_annotation_pk(annotation);
+    const actual_deduction_before = this.limited_deduction_by_annotation_pk(
+      unchecked_deduction_before,
+      annotation
+    );
+    const unchecked_deduction_after = unchecked_deduction_before + unchecked_score_change;
+    const actual_deduction_after = this.limited_deduction_by_annotation_pk(
+      unchecked_deduction_after,
+      annotation
+    );
+    return actual_deduction_after - actual_deduction_before;
+  }
+
   update_applied_annotation_created(applied_annotation: AppliedAnnotation): void {
     if (applied_annotation.handgrading_result === this.d_handgrading_result!.pk) {
+      const score_change = this.deduction_change(
+        applied_annotation.annotation,
+        applied_annotation.annotation.deduction
+      );
       this.d_handgrading_result!.applied_annotations.push(applied_annotation);
       // tslint:disable-next-line no-floating-promises
-      this.update_score(applied_annotation.annotation.deduction);
+      this.update_score(score_change);
     }
   }
 
   update_applied_annotation_deleted(applied_annotation: AppliedAnnotation): void {
     if (applied_annotation.handgrading_result === this.d_handgrading_result!.pk) {
+      const score_change = this.deduction_change(
+        applied_annotation.annotation,
+        -applied_annotation.annotation.deduction
+      );
       this.d_handgrading_result!.applied_annotations.splice(
         this.d_handgrading_result!.applied_annotations.findIndex(
           item => item.pk === applied_annotation.pk
         ),
         1
       );
       // tslint:disable-next-line no-floating-promises
-      this.update_score(-applied_annotation.annotation.deduction);
+      this.update_score(score_change);
     }
   }
 

src/components/project_view/submission_detail/ag_test_command_result_detail.vue

@@ -33,7 +33,9 @@
                         {'actual-return-code-correct': ag_test_command_result !== null
                           && ag_test_command_result.return_code_correct,
                           'actual-return-code-incorrect': ag_test_command_result !== null
-                          && !ag_test_command_result.return_code_correct}]"
+                          // return_code_correct can be null so we need to check for false
+                          // to avoid showing the red warning color when it's null
+                          && ag_test_command_result.return_code_correct === false}]"
           >{{ag_test_command_result.actual_return_code}}</div>
         </div>
       </div>

tests/test_components/test_project_view/test_handgrading/test_handgrading.ts

@@ -432,6 +432,7 @@ describe('Comment tests', () => {
 
     let annotation_with_long_description: ag_cli.Annotation;
     let annotation_empty_long_description: ag_cli.Annotation;
+    let annotation_unaligned_max_deduction: ag_cli.Annotation;
 
     // The number at the end of these names describes the ordering
     // of these elements in the displayed comment/applied annotation
@@ -460,6 +461,16 @@ describe('Comment tests', () => {
             }
         );
 
+        annotation_unaligned_max_deduction = data_ut.make_annotation(
+            result.handgrading_rubric.pk,
+            {
+                short_description: "I have unaligned max deduction",
+                deduction: -3,
+                // Not a multiple of -3
+                max_deduction: -5
+            }
+        );
+
         result.handgrading_rubric.annotations = [
             annotation_with_long_description, annotation_empty_long_description
         ];
@@ -475,6 +486,8 @@ describe('Comment tests', () => {
             result, annotation_with_long_description,
             {filename: 'file2.cpp', first_line: 10, last_line: 10}
         );
+        result.total_points_possible = 10;
+        result.total_points = 7;
 
         data_ut.set_global_user_roles(data_ut.make_user_roles({is_staff: true}));
         wrapper = managed_mount(Handgrading, {
@@ -518,7 +531,7 @@ describe('Comment tests', () => {
     test('Handgraders allowed to leave custom comments', async () => {
         wrapper.vm.d_handgrading_result!.handgrading_rubric.handgraders_can_leave_comments = true;
         data_ut.set_global_user_roles(data_ut.make_user_roles({is_handgrader: true}));
-        wrapper.vm.$forceUpdate();
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
         await wrapper.vm.$nextTick();
         expect(wrapper.find('#new-comment').exists()).toBe(true);
 
@@ -637,6 +650,109 @@ describe('Comment tests', () => {
             [applied_long_description_annotation_1]);
         expect(delete_stub.calledOnce).toBe(true);
     });
+
+    test('Add applied annotations beyond max deduction', async () => {
+        for (let i = 1; i <= 3; ++i) {
+            let applied_annotation = data_ut.make_applied_annotation(
+                result,
+                annotation_empty_long_description,
+                {filename: 'file1.txt', first_line: i, last_line: i}
+            );
+            ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation);
+        }
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        await wrapper.vm.$nextTick();
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('5/10');
+    });
+
+    test('Delete all applied annotations after exceeding max deduction', async () => {
+        for (let i = 1; i <= 4; ++i) {
+            let applied_annotation = data_ut.make_applied_annotation(
+                result,
+                annotation_empty_long_description,
+                {filename: 'file1.txt', first_line: i, last_line: i}
+            );
+            ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation);
+        }
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.d_handgrading_result!.applied_annotations.length).toEqual(6);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('5/10');
+
+        const delete_stubs =
+            wrapper.vm.d_handgrading_result!.applied_annotations.map(annotation => {
+                return sinon.stub(
+                    annotation, 'delete'
+                ).callsFake(() => {
+                    ag_cli.AppliedAnnotation.notify_applied_annotation_deleted(annotation);
+                    return Promise.resolve();
+                });
+            });
+        for (let i = 0; i < 6; ++i) {
+            wrapper.findAll('.comment').at(1).find('.delete').trigger('click');
+            expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        }
+        expect(wrapper.vm.d_handgrading_result!.applied_annotations.length).toEqual(0);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('10/10');
+
+        for (const stub of delete_stubs) {
+            expect(stub.calledOnce).toBe(true);
+        }
+    });
+
+    test('Add applied annotation across unaligned max deduction', async () => {
+        const add_annotation = (i: number) => {
+            let applied_annotation = data_ut.make_applied_annotation(
+                result,
+                annotation_unaligned_max_deduction,
+                {filename: 'file3.py', first_line: i, last_line: i}
+            );
+            ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation);
+        };
+
+        add_annotation(1);
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.d_handgrading_result!.applied_annotations.length).toEqual(3);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('4/10');
+
+        add_annotation(2);
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.d_handgrading_result!.applied_annotations.length).toEqual(4);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('2/10');
+    });
+
+    test('Delete applied annotation across unaligned max deduction', async () => {
+        const add_annotation = (i: number) => {
+            let applied_annotation = data_ut.make_applied_annotation(
+                result,
+                annotation_unaligned_max_deduction,
+                {filename: 'file3.py', first_line: i, last_line: i}
+            );
+            ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation);
+        };
+
+        add_annotation(1);
+        add_annotation(2);
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.d_handgrading_result!.applied_annotations.length).toEqual(4);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('2/10');
+
+        let to_delete = wrapper.vm.d_handgrading_result!.applied_annotations[3];
+        let delete_stub = sinon.stub(
+            to_delete, 'delete'
+        ).callsFake(() => {
+            ag_cli.AppliedAnnotation.notify_applied_annotation_deleted(to_delete);
+            return Promise.resolve();
+        });
+        wrapper.findAll('.comment').at(3).find('.delete').trigger('click');
+        expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true);
+
+        expect(delete_stub.calledOnce).toBe(true);
+        expect(wrapper.find('.grading-sidebar-header .score').text()).toEqual('4/10');
+    });
 });
 
 describe('Annotations reference list', () => {