From 10d8be330146bce245f7984f962887d50fe9f651 Mon Sep 17 00:00:00 2001 From: Michael Peng Date: Sat, 20 Apr 2024 12:24:40 -0400 Subject: [PATCH 1/3] Check max deduction for handgrading annotations --- .../project_view/handgrading/handgrading.vue | 42 ++++++- .../test_handgrading/test_handgrading.ts | 116 ++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) diff --git a/src/components/project_view/handgrading/handgrading.vue b/src/components/project_view/handgrading/handgrading.vue index 0e16e3f8..eb8e1d8b 100644 --- a/src/components/project_view/handgrading/handgrading.vue +++ b/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,16 +496,53 @@ 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 @@ -512,7 +550,7 @@ export default class Handgrading extends Vue implements AppliedAnnotationObserve 1 ); // tslint:disable-next-line no-floating-promises - this.update_score(-applied_annotation.annotation.deduction); + this.update_score(score_change); } } diff --git a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts b/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts index f734772e..646ff558 100644 --- a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts +++ b/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, { @@ -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); + } + wrapper.vm.$forceUpdate(); + 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); + } + wrapper.vm.$forceUpdate(); + 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); + wrapper.vm.$forceUpdate(); + 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); + wrapper.vm.$forceUpdate(); + 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); + wrapper.vm.$forceUpdate(); + 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', () => { From 522dea730ca440ca76937e89884f43ba6e5aa963 Mon Sep 17 00:00:00 2001 From: normanqw Date: Tue, 23 Apr 2024 12:29:56 -0400 Subject: [PATCH 2/3] Replace forceUpdate with checking for w.vm.saving --- .../test_handgrading/test_handgrading.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts b/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts index 646ff558..961c8efa 100644 --- a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts +++ b/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts @@ -660,7 +660,7 @@ describe('Comment tests', () => { ); ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation); } - wrapper.vm.$forceUpdate(); + 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'); }); @@ -674,7 +674,7 @@ describe('Comment tests', () => { ); ag_cli.AppliedAnnotation.notify_applied_annotation_created(applied_annotation); } - wrapper.vm.$forceUpdate(); + 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'); @@ -711,13 +711,13 @@ describe('Comment tests', () => { }; add_annotation(1); - wrapper.vm.$forceUpdate(); + 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); - wrapper.vm.$forceUpdate(); + 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'); @@ -735,7 +735,7 @@ describe('Comment tests', () => { add_annotation(1); add_annotation(2); - wrapper.vm.$forceUpdate(); + 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'); From a31c9669d1097cce9c7902193bc63de3a2f56d87 Mon Sep 17 00:00:00 2001 From: normanqw Date: Tue, 23 Apr 2024 12:35:10 -0400 Subject: [PATCH 3/3] Use checking w.vm.saving in test_handgrading Replaced forceUpdate with expect(await wait_until(wrapper, w => !w.vm.saving)).toBe(true); previously written test case Handgraders allowed to leave custom comments --- .../test_project_view/test_handgrading/test_handgrading.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts b/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts index 961c8efa..f275bb56 100644 --- a/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts +++ b/tests/test_components/test_project_view/test_handgrading/test_handgrading.ts @@ -531,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);