From 47ca9077873e1618486021e0c52662d0f6e43f36 Mon Sep 17 00:00:00 2001 From: Tarlan Ismayilsoy Date: Fri, 21 Jul 2023 12:32:25 +0400 Subject: [PATCH] Assessment: Fix the loss of submission text due to overlapping manual text blocks (#6916) --- .../assess/text-assessment-base.component.ts | 14 ++++++- ...xt-submission-assessment.component.spec.ts | 41 ++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/exercises/text/assess/text-assessment-base.component.ts b/src/main/webapp/app/exercises/text/assess/text-assessment-base.component.ts index fdec45c7e38c..c8b094dbab08 100644 --- a/src/main/webapp/app/exercises/text/assess/text-assessment-base.component.ts +++ b/src/main/webapp/app/exercises/text/assess/text-assessment-base.component.ts @@ -92,7 +92,19 @@ export abstract class TextAssessmentBaseComponent implements OnInit { } else if ([ref, previousRef].every((r) => r.block?.type === TextBlockType.AUTOMATIC)) { console.error('Overlapping AUTOMATIC Text Blocks!', previousRef, ref); } else if ([ref, previousRef].every((r) => r.block?.type === TextBlockType.MANUAL)) { - console.error('Overlapping MANUAL Text Blocks!', previousRef, ref); + // Make sure to select a TextBlockRef that has a feedback. + let selectedRef = ref; + if (!selectedRef.feedback) { + selectedRef = previousRef; + } + + // Non-overlapping part of previousRef and ref should be added as a new text block (otherwise, some text is lost) + // But before, make sure that the selectedRef does not already cover the exact same range (otherwise, duplicate text blocks will appear) + if (selectedRef.block!.startIndex != previousRef.block!.startIndex! && selectedRef.block!.endIndex != nextIndex) { + TextAssessmentBaseComponent.addTextBlockByIndices(previousRef.block!.startIndex!, nextIndex, submission!, textBlockRefs); + } + + ref = selectedRef; } else { // Find which block is Manual and only keep that one. Automatic block is stored in `unusedTextBlockRefs` in case we need to restore. switch (TextBlockType.MANUAL) { diff --git a/src/test/javascript/spec/component/text-submission-assessment/text-submission-assessment.component.spec.ts b/src/test/javascript/spec/component/text-submission-assessment/text-submission-assessment.component.spec.ts index ffc9a7511cf8..479b4e72288b 100644 --- a/src/test/javascript/spec/component/text-submission-assessment/text-submission-assessment.component.spec.ts +++ b/src/test/javascript/spec/component/text-submission-assessment/text-submission-assessment.component.spec.ts @@ -22,7 +22,7 @@ import { ConfirmIconComponent } from 'app/shared/confirm-icon/confirm-icon.compo import { Course } from 'app/entities/course.model'; import { ManualTextblockSelectionComponent } from 'app/exercises/text/assess/manual-textblock-selection/manual-textblock-selection.component'; import { TextAssessmentService } from 'app/exercises/text/assess/text-assessment.service'; -import { TextBlock } from 'app/entities/text-block.model'; +import { TextBlock, TextBlockType } from 'app/entities/text-block.model'; import { Feedback, FeedbackType } from 'app/entities/feedback.model'; import { ComplaintResponse } from 'app/entities/complaint-response.model'; import { AlertService } from 'app/core/util/alert.service'; @@ -42,6 +42,7 @@ import { HttpErrorResponse, HttpResponse } from '@angular/common/http'; import { TranslateService } from '@ngx-translate/core'; import { MockTranslateService } from '../../helpers/mocks/service/mock-translate.service'; import { AssessmentAfterComplaint } from 'app/complaints/complaints-for-tutor/complaints-for-tutor.component'; +import { TextAssessmentBaseComponent } from 'app/exercises/text/assess/text-assessment-base.component'; describe('TextSubmissionAssessmentComponent', () => { let component: TextSubmissionAssessmentComponent; @@ -112,6 +113,7 @@ describe('TextSubmissionAssessmentComponent', () => { text: 'Second text.', startIndex: 12, endIndex: 24, + type: TextBlockType.MANUAL, submission, } as TextBlock, ]; @@ -423,4 +425,41 @@ describe('TextSubmissionAssessmentComponent', () => { expect(component.textBlockRefs).toHaveLength(2); expect(component.unusedTextBlockRefs).toHaveLength(0); }); + + it('should handle overlapping manual text blocks correctly', () => { + const sortAndSetTextBlockRefsSpy = jest.spyOn(TextAssessmentBaseComponent.prototype as any, 'sortAndSetTextBlockRefs'); + + // BEGIN: Adding a new block (with feedback) that overlaps with an existing block + submission.blocks?.push({ + id: 'third text id', + text: 'text.', + startIndex: 19, + endIndex: 24, + type: TextBlockType.MANUAL, + submission, + } as TextBlock); + + getLatestSubmissionResult(submission)?.feedbacks?.push({ + id: 3, + detailText: 'Third Feedback', + credits: 0, + reference: 'third text id', + } as Feedback); + // END: Adding a new block (with feedback) that overlaps with an existing block + + component['setPropertiesFromServerResponse'](participation); + fixture.detectChanges(); + + expect(sortAndSetTextBlockRefsSpy).toHaveBeenCalled(); + + expect(component.textBlockRefs).toEqual( + // Checking if sortAndSetTextBlockRefs selected the right TextBlockRef (the one having a feedback) + // Performing partial match for { block: { text: ...}, feedback: { id: ... } } + expect.arrayContaining([expect.objectContaining({ block: expect.objectContaining({ text: 'text.' }), feedback: expect.objectContaining({ id: 3 }) })]), + ); + + // Checking if a new block was added to compensate for the loss of submitted text due to the overlap between blocks + // Performing partial match for { block: { text: ...} } + expect(component.textBlockRefs).toEqual(expect.arrayContaining([expect.objectContaining({ block: expect.objectContaining({ text: 'Second ' }) })])); + }); });