From 346499ed79981487b07b8c71890e38f6579a189c Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Fri, 15 Nov 2024 00:04:31 +0100 Subject: [PATCH 01/26] add ui for toggling and translation for labels --- .../plagiarism-header/plagiarism-header.component.html | 5 +++++ .../plagiarism-header/plagiarism-header.component.ts | 8 ++++++++ src/main/webapp/i18n/de/plagiarism.json | 2 ++ src/main/webapp/i18n/en/plagiarism.json | 2 ++ 4 files changed, 17 insertions(+) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html index aff14deb4db6..599a2661d83c 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html @@ -23,6 +23,11 @@
jhiTranslate="artemisApp.plagiarism.deny" > + +
diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts index 780591ff5a6a..2f4d83f9fb95 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts @@ -8,6 +8,7 @@ import { PlagiarismCasesService } from 'app/course/plagiarism-cases/shared/plagi import { ConfirmAutofocusModalComponent } from 'app/shared/components/confirm-autofocus-modal.component'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { Exercise, getCourseId } from 'app/entities/exercise.model'; +import { IconDefinition, faLock, faUnlock } from '@fortawesome/free-solid-svg-icons'; @Component({ selector: 'jhi-plagiarism-header', @@ -19,8 +20,11 @@ export class PlagiarismHeaderComponent { @Input() exercise: Exercise; @Input() splitControlSubject: Subject; + protected readonly faLock: IconDefinition = faLock; + protected readonly faUnlock: IconDefinition = faUnlock; readonly plagiarismStatus = PlagiarismStatus; isLoading = false; + isLockFilesEnabled = false; constructor( private plagiarismCasesService: PlagiarismCasesService, @@ -76,4 +80,8 @@ export class PlagiarismHeaderComponent { resetSplitPanes() { this.splitControlSubject.next('even'); } + + toggleLockFiles() { + this.isLockFilesEnabled = !this.isLockFilesEnabled; + } } diff --git a/src/main/webapp/i18n/de/plagiarism.json b/src/main/webapp/i18n/de/plagiarism.json index 2e8f1d30798f..3461f3818dfd 100644 --- a/src/main/webapp/i18n/de/plagiarism.json +++ b/src/main/webapp/i18n/de/plagiarism.json @@ -4,6 +4,8 @@ "plagiarismDetection": "Plagiatskontrolle", "confirm": "Bestätigen", "deny": "Ablehnen", + "lockSync": "Sync sperren", + "unlockSync": "Sync entsperren", "denyAfterConfirmModalTitle": "Wechsel von Bestätigen zu Ablehnen", "denyAfterConfirmModalText": "Bist du dir sicher, dass du die Entscheidung von \"Bestätigung des Plagiats\" in \"Ablehnung\" ändern möchtest? Dadurch wird der entsprechende Plagiatsfall einschließlich der Kommunikation mit dem/der Studierenden und des Urteils gelöscht und kann nicht rückgängig gemacht werden.", "warning": "Die Bestätigung/Ablehnung eines Plagiats geht beim Neuladen der Seite verloren.", diff --git a/src/main/webapp/i18n/en/plagiarism.json b/src/main/webapp/i18n/en/plagiarism.json index 8f071d94b798..149a6decf416 100644 --- a/src/main/webapp/i18n/en/plagiarism.json +++ b/src/main/webapp/i18n/en/plagiarism.json @@ -4,6 +4,8 @@ "plagiarismDetection": "Plagiarism Detection", "confirm": "Confirm", "deny": "Deny", + "lockSync": "Lock Sync", + "unlockSync": "Unlock Sync", "denyAfterConfirmModalTitle": "Change from confirm to deny", "denyAfterConfirmModalText": "Are you sure that you want to change the decision from confirming the plagiarism to denying it? This will delete the corresponding plagiarism case incl. the communication with the student and the verdict and cannot be undone.", "warning": "Confirmation/Denial of plagiarism will be lost when refreshing the page.", From 9d7a69fc1d6e66e70e1c3dc7a8629289d410df12 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Fri, 15 Nov 2024 16:01:59 +0100 Subject: [PATCH 02/26] add working implementation --- .../plagiarism-details.component.html | 7 ++-- .../plagiarism-details.component.ts | 5 +++ .../plagiarism-header.component.ts | 8 ++++- .../plagiarism-split-view.component.html | 11 ++++++- .../plagiarism-split-view.component.ts | 7 ++-- .../split-pane-header.component.ts | 33 +++++++++++++++++-- .../text-submission-viewer.component.html | 8 ++++- .../text-submission-viewer.component.ts | 3 ++ 8 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html index 3fa33629ca83..c6cca5a6963a 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html @@ -1,6 +1,9 @@ @if (comparison) {
- - + + + + +
} diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts index a17fe6ff2421..60abc363abe2 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts @@ -18,4 +18,9 @@ export class PlagiarismDetailsComponent { * Subject to be passed into PlagiarismSplitViewComponent to control the split view. */ splitControlSubject: Subject = new Subject(); + + /** + * Boolean to determine whether file locking is enabled. + */ + isLockFilesEnabled: boolean = false; } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts index 2f4d83f9fb95..51c62739d692 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts @@ -1,4 +1,4 @@ -import { Component, Input } from '@angular/core'; +import { Component, EventEmitter, Input, Output } from '@angular/core'; import { Subject } from 'rxjs'; import { PlagiarismStatus } from 'app/exercises/shared/plagiarism/types/PlagiarismStatus'; import { PlagiarismComparison } from 'app/exercises/shared/plagiarism/types/PlagiarismComparison'; @@ -20,6 +20,8 @@ export class PlagiarismHeaderComponent { @Input() exercise: Exercise; @Input() splitControlSubject: Subject; + @Output() isLockFilesEnabledChange = new EventEmitter(); + protected readonly faLock: IconDefinition = faLock; protected readonly faUnlock: IconDefinition = faUnlock; readonly plagiarismStatus = PlagiarismStatus; @@ -81,7 +83,11 @@ export class PlagiarismHeaderComponent { this.splitControlSubject.next('even'); } + /** + * Toggles the state of file locking and emits the new state to the parent component. + */ toggleLockFiles() { this.isLockFilesEnabled = !this.isLockFilesEnabled; + this.isLockFilesEnabledChange.emit(this.isLockFilesEnabled); } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html index 4c6052b563d4..e379e1a4965f 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html @@ -5,7 +5,14 @@ } @if (isProgrammingOrTextExercise) { - + } }
@@ -20,9 +27,11 @@ } @if (isProgrammingOrTextExercise) { } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index 340d72154ce6..b484570c2738 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -10,6 +10,7 @@ import { PlagiarismCasesService } from 'app/course/plagiarism-cases/shared/plagi import { HttpResponse } from '@angular/common/http'; import { SimpleMatch } from 'app/exercises/shared/plagiarism/types/PlagiarismMatch'; import dayjs from 'dayjs/esm'; +import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component'; @Directive({ selector: '[jhiPane]' }) export class SplitPaneDirective { @@ -27,10 +28,12 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O @Input() splitControlSubject: Subject; @Input() sortByStudentLogin: string; @Input() forStudent: boolean; + @Input() isLockFilesEnabled = false; @ViewChildren(SplitPaneDirective) panes!: QueryList; plagiarismComparison: PlagiarismComparison; + fileSelectedSubject = new Subject<{ idx: number; file: FileWithHasMatch }>(); public split: Split.Instance; @@ -45,7 +48,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O constructor(private plagiarismCasesService: PlagiarismCasesService) {} /** - * Initialize third party libs inside this lifecycle hook. + * Initialize third-party libraries inside this lifecycle hook. */ ngAfterViewInit(): void { const paneElements = this.panes.map((pane: SplitPaneDirective) => pane.elementRef.nativeElement); @@ -122,7 +125,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O * @param matches list of objects containing the index and length of matched elements * @param submission the submission to map the elements of */ - mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { + private mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { // sort submission elements so that from and to indexes from matches reference correct elements const elements = submission.elements?.sort((a, b) => a.id - b.id); const filesToMatchedElements = new Map(); diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 773a297e53e5..25a961e82745 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -1,5 +1,6 @@ -import { Component, EventEmitter, Input, OnChanges, Output, SimpleChanges } from '@angular/core'; +import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; import { faChevronDown } from '@fortawesome/free-solid-svg-icons'; +import { Subject, Subscription } from 'rxjs'; /** * A file name that additionally stores if a plagiarism match has been found for it. @@ -14,17 +15,32 @@ export type FileWithHasMatch = { templateUrl: './split-pane-header.component.html', styleUrls: ['./split-pane-header.component.scss'], }) -export class SplitPaneHeaderComponent implements OnChanges { +export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { @Input() files: FileWithHasMatch[]; @Input() studentLogin: string; + @Input() fileSelectedSubject!: Subject<{ idx: number; file: FileWithHasMatch }>; + @Input() isLockFilesEnabled!: boolean; + @Output() selectFile = new EventEmitter(); public showFiles = false; public activeFileIndex = 0; + fileSelectSubscription: Subscription; + // Icons faChevronDown = faChevronDown; + ngOnInit(): void { + this.fileSelectSubscription = this.fileSelectedSubject.subscribe((val) => { + console.log('passed file', val); + console.log('passed and current index', val.idx, this.activeFileIndex); + if (val.file && this.isLockFilesEnabled) { + this.handleFileSelectWithoutPropagation(val.file, val.idx); + } + }); + } + ngOnChanges(changes: SimpleChanges) { if (changes.files) { const fileWithHasMatch: FileWithHasMatch[] = changes.files.currentValue; @@ -37,6 +53,12 @@ export class SplitPaneHeaderComponent implements OnChanges { } } + ngOnDestroy(): void { + if (this.fileSelectSubscription) { + this.fileSelectSubscription.unsubscribe(); + } + } + hasActiveFile(): boolean { return this.hasFiles() && this.activeFileIndex < this.files.length; } @@ -46,6 +68,13 @@ export class SplitPaneHeaderComponent implements OnChanges { } handleFileSelect(file: FileWithHasMatch, idx: number): void { + this.fileSelectedSubject.next({ idx: idx, file: file }); + this.activeFileIndex = idx; + this.showFiles = false; + this.selectFile.emit(file.file); + } + + handleFileSelectWithoutPropagation(file: FileWithHasMatch, idx: number) { this.activeFileIndex = idx; this.showFiles = false; this.selectFile.emit(file.file); diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html index e1becc774b87..d58609f29b60 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html @@ -1,4 +1,10 @@ - + @if (cannotLoadFiles) {
diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts index 9cd4b201f571..5267f3ddf51c 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts @@ -12,6 +12,7 @@ import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-spl import { escape } from 'lodash-es'; import { faExclamationTriangle } from '@fortawesome/free-solid-svg-icons'; import { TEXT_FILE_EXTENSIONS } from 'app/shared/constants/file-extensions.constants'; +import { Subject } from 'rxjs'; type FilesWithType = { [p: string]: FileType }; @@ -26,6 +27,8 @@ export class TextSubmissionViewerComponent implements OnChanges { @Input() matches: Map; @Input() plagiarismSubmission: PlagiarismSubmission; @Input() hideContent: boolean; + @Input() fileSelectedSubject!: Subject<{ idx: number; file: FileWithHasMatch }>; + @Input() isLockFilesEnabled = false; /** * Name of the currently selected file. From 6dedb6894a8b0a779af47ead2d7d4584d7c764af Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Fri, 15 Nov 2024 16:17:24 +0100 Subject: [PATCH 03/26] add code quality changes and comments --- .../plagiarism-split-view.component.ts | 4 ++-- .../split-pane-header.component.ts | 15 ++++++++++++--- .../text-submission-viewer.component.ts | 3 ++- .../types/text/TextPlagiarismFileElement.ts | 6 ++++++ 4 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index b484570c2738..093245a28823 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -10,7 +10,7 @@ import { PlagiarismCasesService } from 'app/course/plagiarism-cases/shared/plagi import { HttpResponse } from '@angular/common/http'; import { SimpleMatch } from 'app/exercises/shared/plagiarism/types/PlagiarismMatch'; import dayjs from 'dayjs/esm'; -import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component'; +import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; @Directive({ selector: '[jhiPane]' }) export class SplitPaneDirective { @@ -33,7 +33,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O @ViewChildren(SplitPaneDirective) panes!: QueryList; plagiarismComparison: PlagiarismComparison; - fileSelectedSubject = new Subject<{ idx: number; file: FileWithHasMatch }>(); + fileSelectedSubject = new Subject(); public split: Split.Instance; diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 25a961e82745..d9442ae9c924 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -1,6 +1,7 @@ import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; import { faChevronDown } from '@fortawesome/free-solid-svg-icons'; import { Subject, Subscription } from 'rxjs'; +import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; /** * A file name that additionally stores if a plagiarism match has been found for it. @@ -18,7 +19,7 @@ export type FileWithHasMatch = { export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { @Input() files: FileWithHasMatch[]; @Input() studentLogin: string; - @Input() fileSelectedSubject!: Subject<{ idx: number; file: FileWithHasMatch }>; + @Input() fileSelectedSubject!: Subject; @Input() isLockFilesEnabled!: boolean; @Output() selectFile = new EventEmitter(); @@ -33,8 +34,6 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { ngOnInit(): void { this.fileSelectSubscription = this.fileSelectedSubject.subscribe((val) => { - console.log('passed file', val); - console.log('passed and current index', val.idx, this.activeFileIndex); if (val.file && this.isLockFilesEnabled) { this.handleFileSelectWithoutPropagation(val.file, val.idx); } @@ -67,6 +66,11 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { return this.files[this.activeFileIndex].file; } + /** + * handles selection of file from dropdown, propagates change to fileslectionsubject component for lock sync + * @param file to be selected + * @param idx index of the file from the dropdown + */ handleFileSelect(file: FileWithHasMatch, idx: number): void { this.fileSelectedSubject.next({ idx: idx, file: file }); this.activeFileIndex = idx; @@ -74,6 +78,11 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { this.selectFile.emit(file.file); } + /** + * handles selection of file from dropdown, do NOT propagates change to fileslectionsubject component for lock sync + * @param file to be selected + * @param idx index of the file from the dropdown + */ handleFileSelectWithoutPropagation(file: FileWithHasMatch, idx: number) { this.activeFileIndex = idx; this.showFiles = false; diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts index 5267f3ddf51c..3f1b5e29dd52 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts @@ -13,6 +13,7 @@ import { escape } from 'lodash-es'; import { faExclamationTriangle } from '@fortawesome/free-solid-svg-icons'; import { TEXT_FILE_EXTENSIONS } from 'app/shared/constants/file-extensions.constants'; import { Subject } from 'rxjs'; +import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; type FilesWithType = { [p: string]: FileType }; @@ -27,7 +28,7 @@ export class TextSubmissionViewerComponent implements OnChanges { @Input() matches: Map; @Input() plagiarismSubmission: PlagiarismSubmission; @Input() hideContent: boolean; - @Input() fileSelectedSubject!: Subject<{ idx: number; file: FileWithHasMatch }>; + @Input() fileSelectedSubject!: Subject; @Input() isLockFilesEnabled = false; /** diff --git a/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts b/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts new file mode 100644 index 000000000000..ad00f2618d22 --- /dev/null +++ b/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts @@ -0,0 +1,6 @@ +import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component'; + +export class TextPlagiarismFileElement { + idx: number; + file: FileWithHasMatch; +} From 0d37874e208e7ecd88c0e47412204bfb9fda0243 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Fri, 15 Nov 2024 20:09:26 +0100 Subject: [PATCH 04/26] fix failing test --- .../plagiarism-split-view/plagiarism-split-view.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index 093245a28823..4228c82a94ba 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -125,7 +125,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O * @param matches list of objects containing the index and length of matched elements * @param submission the submission to map the elements of */ - private mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { + mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { // sort submission elements so that from and to indexes from matches reference correct elements const elements = submission.elements?.sort((a, b) => a.id - b.id); const filesToMatchedElements = new Map(); From e8d46fb4247259ac3af38e008fbb74837ada7376 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 13:33:48 +0100 Subject: [PATCH 05/26] attempt fix tests --- .../plagiarism-split-view/plagiarism-split-view.component.ts | 2 +- .../plagiarism/plagiarism-split-view.component.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index 4228c82a94ba..5a888c64a9e0 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -125,7 +125,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O * @param matches list of objects containing the index and length of matched elements * @param submission the submission to map the elements of */ - mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { + public mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { // sort submission elements so that from and to indexes from matches reference correct elements const elements = submission.elements?.sort((a, b) => a.id - b.id); const filesToMatchedElements = new Map(); diff --git a/src/test/javascript/spec/component/plagiarism/plagiarism-split-view.component.spec.ts b/src/test/javascript/spec/component/plagiarism/plagiarism-split-view.component.spec.ts index 4b06a312d72c..e5c160f6ecc4 100644 --- a/src/test/javascript/spec/component/plagiarism/plagiarism-split-view.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/plagiarism-split-view.component.spec.ts @@ -170,7 +170,7 @@ describe('Plagiarism Split View Component', () => { }); it('should parse text matches', () => { - jest.spyOn(comp, 'mapMatchesToElements').mockReturnValue(new Map()); + jest.spyOn(comp, 'mapMatchesToElements').mockReturnValue(new Map()); const matches: PlagiarismMatch[] = [ { startA: 0, startB: 0, length: 5 }, From 7f268d21dcc54f1f5f2ab9606ddd68ed00e1c69e Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 13:48:51 +0100 Subject: [PATCH 06/26] attempt fix more tests --- .../component/plagiarism/plagiarism-header.component.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts index c38202b631ec..548f5e9caba5 100644 --- a/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts @@ -13,6 +13,7 @@ import { HttpResponse } from '@angular/common/http'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { MockNgbModalService } from '../../helpers/mocks/service/mock-ngb-modal.service'; import { ButtonComponent } from 'app/shared/components/button.component'; +import { NO_ERRORS_SCHEMA } from '@angular/core'; describe('Plagiarism Header Component', () => { let comp: PlagiarismHeaderComponent; @@ -27,6 +28,7 @@ describe('Plagiarism Header Component', () => { { provide: TranslateService, useClass: MockTranslateService }, { provide: NgbModal, useClass: MockNgbModalService }, ], + schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); fixture = TestBed.createComponent(PlagiarismHeaderComponent); From efb80bd7ccba6bd69bc225213e4addba4967b98c Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 14:08:51 +0100 Subject: [PATCH 07/26] fix last failing test --- .../component/plagiarism/split-pane-header.component.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 31953315113a..2f495cc759ba 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -10,6 +10,8 @@ import { PlagiarismDetailsComponent } from 'app/exercises/shared/plagiarism/plag import { PlagiarismRunDetailsComponent } from 'app/exercises/shared/plagiarism/plagiarism-run-details/plagiarism-run-details.component'; import { PlagiarismSidebarComponent } from 'app/exercises/shared/plagiarism/plagiarism-sidebar/plagiarism-sidebar.component'; import { MockComponent, MockDirective, MockPipe } from 'ng-mocks'; +import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; +import { Subject } from 'rxjs'; describe('SplitPaneHeaderComponent', () => { let comp: SplitPaneHeaderComponent; @@ -42,6 +44,7 @@ describe('SplitPaneHeaderComponent', () => { comp.files = []; comp.studentLogin = 'ts10abc'; comp.selectFile = new EventEmitter(); + comp.fileSelectedSubject = new Subject(); }); }); From 55c5a4ed33bd97a3db3e682ccd803f09bcfeb0ab Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 15:08:59 +0100 Subject: [PATCH 08/26] add mock translate directive to fix test --- .../plagiarism/plagiarism-header.component.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts index 548f5e9caba5..a010211dc85c 100644 --- a/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/plagiarism-header.component.spec.ts @@ -13,7 +13,8 @@ import { HttpResponse } from '@angular/common/http'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { MockNgbModalService } from '../../helpers/mocks/service/mock-ngb-modal.service'; import { ButtonComponent } from 'app/shared/components/button.component'; -import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { MockDirective } from 'ng-mocks'; +import { TranslateDirective } from 'app/shared/language/translate.directive'; describe('Plagiarism Header Component', () => { let comp: PlagiarismHeaderComponent; @@ -23,12 +24,11 @@ describe('Plagiarism Header Component', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ArtemisTestModule, TranslateTestingModule], - declarations: [PlagiarismHeaderComponent], + declarations: [PlagiarismHeaderComponent, MockDirective(TranslateDirective)], providers: [ { provide: TranslateService, useClass: MockTranslateService }, { provide: NgbModal, useClass: MockNgbModalService }, ], - schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); fixture = TestBed.createComponent(PlagiarismHeaderComponent); From f573a95c3427f57feb458a9b3e541bb507e8c6a9 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 15:18:56 +0100 Subject: [PATCH 09/26] add coderabbit suggestions --- .../plagiarism-split-view.component.html | 4 ++-- .../plagiarism-split-view.component.ts | 21 +++++++++++++++---- .../types/text/TextPlagiarismFileElement.ts | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html index e379e1a4965f..f721bf043015 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html @@ -6,7 +6,7 @@ } @if (isProgrammingOrTextExercise) { ; @Input() exercise: Exercise; @Input() splitControlSubject: Subject; @@ -33,7 +33,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O @ViewChildren(SplitPaneDirective) panes!: QueryList; plagiarismComparison: PlagiarismComparison; - fileSelectedSubject = new Subject(); + private fileSelectedSubject = new Subject(); public split: Split.Instance; @@ -87,6 +87,11 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O } } + ngOnDestroy() { + this.fileSelectedSubject.complete(); + this.splitControlSubject.unsubscribe(); + } + /** * Swaps fields of A with fields of B in-place. * More specifically, swaps submissionA with submissionB and startA with startB in matches. @@ -106,6 +111,14 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O } } + /** + * get the subject/listener for file selection + * @returns observable for fileselection + */ + getFileSelectedSubject() { + return this.fileSelectedSubject; + } + parseTextMatches(plagComparison: PlagiarismComparison) { if (plagComparison.matches) { const matchesA = plagComparison.matches.map((match) => new SimpleMatch(match.startA, match.length)).sort((m1, m2) => m1.start - m2.start); @@ -125,7 +138,7 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O * @param matches list of objects containing the index and length of matched elements * @param submission the submission to map the elements of */ - public mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { + mapMatchesToElements(matches: SimpleMatch[], submission: PlagiarismSubmission) { // sort submission elements so that from and to indexes from matches reference correct elements const elements = submission.elements?.sort((a, b) => a.id - b.id); const filesToMatchedElements = new Map(); diff --git a/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts b/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts index ad00f2618d22..5780762bd07c 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement.ts @@ -1,6 +1,6 @@ import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component'; -export class TextPlagiarismFileElement { +export interface TextPlagiarismFileElement { idx: number; file: FileWithHasMatch; } From 82068997381c574b89cf2b84b7c399f8ff975e3c Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 16:51:05 +0100 Subject: [PATCH 10/26] add tests for feature --- .../split-pane-header.component.spec.ts | 139 +++++++++++++----- 1 file changed, 101 insertions(+), 38 deletions(-) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 2f495cc759ba..d927cd1bcaa5 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -12,10 +12,13 @@ import { PlagiarismSidebarComponent } from 'app/exercises/shared/plagiarism/plag import { MockComponent, MockDirective, MockPipe } from 'ng-mocks'; import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; import { Subject } from 'rxjs'; +import { TextSubmissionViewerComponent } from 'app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component'; describe('SplitPaneHeaderComponent', () => { - let comp: SplitPaneHeaderComponent; - let fixture: ComponentFixture; + let comp1: SplitPaneHeaderComponent; + let comp2: SplitPaneHeaderComponent; + let fixture1: ComponentFixture; + let fixture2: ComponentFixture; const files = [ { file: 'src/Main.java', hasMatch: true }, @@ -33,93 +36,153 @@ describe('SplitPaneHeaderComponent', () => { MockComponent(PlagiarismDetailsComponent), MockComponent(PlagiarismRunDetailsComponent), MockComponent(PlagiarismSidebarComponent), + MockComponent(TextSubmissionViewerComponent), ], providers: [{ provide: TranslateService, useClass: MockTranslateService }], }) .compileComponents() .then(() => { - fixture = TestBed.createComponent(SplitPaneHeaderComponent); - comp = fixture.componentInstance; - - comp.files = []; - comp.studentLogin = 'ts10abc'; - comp.selectFile = new EventEmitter(); - comp.fileSelectedSubject = new Subject(); + const fileSelectedSubject = new Subject(); + fixture1 = TestBed.createComponent(SplitPaneHeaderComponent); + comp1 = fixture1.componentInstance; + fixture2 = TestBed.createComponent(SplitPaneHeaderComponent); + comp2 = fixture2.componentInstance; + + comp1.files = []; + comp1.studentLogin = 'ts10abc'; + comp1.selectFile = new EventEmitter(); + comp1.fileSelectedSubject = fileSelectedSubject; + + comp2.files = []; + comp2.studentLogin = 'ts20abc'; + comp2.selectFile = new EventEmitter(); + comp2.fileSelectedSubject = fileSelectedSubject; }); }); - it('resets the active file index on change', () => { - comp.activeFileIndex = 1; + comp1.activeFileIndex = 1; - comp.ngOnChanges({ + comp1.ngOnChanges({ files: { currentValue: files } as SimpleChange, }); - expect(comp.activeFileIndex).toBe(0); + expect(comp1.activeFileIndex).toBe(0); }); it('selects the first file on change', () => { - comp.files = files; - jest.spyOn(comp.selectFile, 'emit'); + comp1.files = files; + jest.spyOn(comp1.selectFile, 'emit'); - comp.ngOnChanges({ + comp1.ngOnChanges({ files: { currentValue: files } as SimpleChange, }); - expect(comp.selectFile.emit).toHaveBeenCalledOnce(); - expect(comp.selectFile.emit).toHaveBeenCalledWith(files[0].file); + expect(comp1.selectFile.emit).toHaveBeenCalledOnce(); + expect(comp1.selectFile.emit).toHaveBeenCalledWith(files[0].file); }); it('does not find an active file', () => { - const activeFile = comp.hasActiveFile(); + const activeFile = comp1.hasActiveFile(); expect(activeFile).toBeFalse(); }); it('returns the active file', () => { - comp.files = files; - const activeFile = comp.getActiveFile(); + comp1.files = files; + const activeFile = comp1.getActiveFile(); expect(activeFile).toBe(files[0].file); }); it('handles selection of a file', () => { const idx = 1; - comp.showFiles = true; - jest.spyOn(comp.selectFile, 'emit'); + comp1.showFiles = true; + jest.spyOn(comp1.selectFile, 'emit'); - comp.handleFileSelect(files[idx], idx); + comp1.handleFileSelect(files[idx], idx); - expect(comp.activeFileIndex).toBe(idx); - expect(comp.showFiles).toBeFalse(); - expect(comp.selectFile.emit).toHaveBeenCalledOnce(); - expect(comp.selectFile.emit).toHaveBeenCalledWith(files[idx].file); + expect(comp1.activeFileIndex).toBe(idx); + expect(comp1.showFiles).toBeFalse(); + expect(comp1.selectFile.emit).toHaveBeenCalledOnce(); + expect(comp1.selectFile.emit).toHaveBeenCalledWith(files[idx].file); }); it('has no files', () => { - expect(comp.hasFiles()).toBeFalse(); + expect(comp1.hasFiles()).toBeFalse(); }); it('has files', () => { - comp.files = files; + comp1.files = files; - expect(comp.hasFiles()).toBeTrue(); + expect(comp1.hasFiles()).toBeTrue(); }); it('toggles "show files"', () => { - comp.showFiles = false; - comp.files = files; + comp1.showFiles = false; + comp1.files = files; - comp.toggleShowFiles(); + comp1.toggleShowFiles(); - expect(comp.showFiles).toBeTrue(); + expect(comp1.showFiles).toBeTrue(); }); it('does not toggle "show files"', () => { - comp.showFiles = false; + comp1.showFiles = false; + + comp1.toggleShowFiles(); + + expect(comp1.showFiles).toBeFalse(); + }); + + it('should emit selected file through fileSelectedSubject', () => { + const selectedFile = { idx: 0, file: files[0] }; + let emittedFile: TextPlagiarismFileElement | undefined; + comp1.fileSelectedSubject.subscribe((file) => { + emittedFile = file; + }); + + comp1.fileSelectedSubject.next(selectedFile); + + // Assert + expect(emittedFile).toBe(selectedFile); + }); + + it('should sync file selection when lockFilesEnabled true', () => { + const idx = 0; + const selectedFile = { idx: idx, file: files[idx] }; + const lockFilesEnabled = true; + + comp1.isLockFilesEnabled = lockFilesEnabled; + comp2.isLockFilesEnabled = lockFilesEnabled; + + const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp2, 'handleFileSelectWithoutPropagation'); + + fixture1.detectChanges(); + fixture2.detectChanges(); + + comp1.handleFileSelect(selectedFile.file, selectedFile.idx); + + fixture1.detectChanges(); + fixture2.detectChanges(); + + expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledWith(selectedFile.file, selectedFile.idx); + }); + + it('should not sync file selection when lockFilesEnabled true', () => { + const idx = 0; + const selectedFile = { idx: idx, file: files[idx] }; + const lockFilesEnabled = false; + + comp1.isLockFilesEnabled = lockFilesEnabled; + comp2.isLockFilesEnabled = lockFilesEnabled; - comp.toggleShowFiles(); + const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp1, 'handleFileSelectWithoutPropagation'); + fixture1.detectChanges(); + fixture2.detectChanges(); + comp1.handleFileSelect(selectedFile.file, selectedFile.idx); + fixture1.detectChanges(); + fixture2.detectChanges(); - expect(comp.showFiles).toBeFalse(); + expect(handleFileSelectWithoutPropagationSpy).not.toHaveBeenCalled(); }); }); From 68b65ec63b38442541ff57346952335b3c80b83e Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Mon, 18 Nov 2024 17:27:47 +0100 Subject: [PATCH 11/26] fix test --- .../plagiarism-split-view/plagiarism-split-view.component.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index 8f3bf6214011..c05e6b4e5bb9 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -89,7 +89,6 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O ngOnDestroy() { this.fileSelectedSubject.complete(); - this.splitControlSubject.unsubscribe(); } /** From f0e0bfd559095d3fe38fa8f2a5bbbc1dc34e9369 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Fri, 22 Nov 2024 12:34:26 +0100 Subject: [PATCH 12/26] adapt functionality to handle cases where files are not identically named, and extend functionality to simulate dropdown hovering --- .../plagiarism-details.component.html | 7 +- .../plagiarism-header.component.html | 5 - .../plagiarism-header.component.ts | 16 +--- .../plagiarism-split-view.component.html | 9 ++ .../plagiarism-split-view.component.scss | 4 + .../plagiarism-split-view.component.ts | 28 +++++- .../split-pane-header.component.html | 8 +- .../split-pane-header.component.scss | 4 + .../split-pane-header.component.ts | 93 ++++++++++++++++++- .../text-submission-viewer.component.html | 2 + .../text-submission-viewer.component.ts | 4 +- src/main/webapp/i18n/de/plagiarism.json | 2 - src/main/webapp/i18n/en/plagiarism.json | 2 - 13 files changed, 149 insertions(+), 35 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html index c6cca5a6963a..c66bd6ef1907 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html @@ -1,9 +1,6 @@ @if (comparison) {
- - - - - + +
} diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html index 599a2661d83c..aff14deb4db6 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html @@ -23,11 +23,6 @@
jhiTranslate="artemisApp.plagiarism.deny" > - -
diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts index 51c62739d692..780591ff5a6a 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, Output } from '@angular/core'; +import { Component, Input } from '@angular/core'; import { Subject } from 'rxjs'; import { PlagiarismStatus } from 'app/exercises/shared/plagiarism/types/PlagiarismStatus'; import { PlagiarismComparison } from 'app/exercises/shared/plagiarism/types/PlagiarismComparison'; @@ -8,7 +8,6 @@ import { PlagiarismCasesService } from 'app/course/plagiarism-cases/shared/plagi import { ConfirmAutofocusModalComponent } from 'app/shared/components/confirm-autofocus-modal.component'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { Exercise, getCourseId } from 'app/entities/exercise.model'; -import { IconDefinition, faLock, faUnlock } from '@fortawesome/free-solid-svg-icons'; @Component({ selector: 'jhi-plagiarism-header', @@ -20,13 +19,8 @@ export class PlagiarismHeaderComponent { @Input() exercise: Exercise; @Input() splitControlSubject: Subject; - @Output() isLockFilesEnabledChange = new EventEmitter(); - - protected readonly faLock: IconDefinition = faLock; - protected readonly faUnlock: IconDefinition = faUnlock; readonly plagiarismStatus = PlagiarismStatus; isLoading = false; - isLockFilesEnabled = false; constructor( private plagiarismCasesService: PlagiarismCasesService, @@ -82,12 +76,4 @@ export class PlagiarismHeaderComponent { resetSplitPanes() { this.splitControlSubject.next('even'); } - - /** - * Toggles the state of file locking and emits the new state to the parent component. - */ - toggleLockFiles() { - this.isLockFilesEnabled = !this.isLockFilesEnabled; - this.isLockFilesEnabledChange.emit(this.isLockFilesEnabled); - } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html index f721bf043015..d7ea6b48f411 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html @@ -12,10 +12,17 @@ [plagiarismSubmission]="getTextSubmissionA()" [isLockFilesEnabled]="isLockFilesEnabled" [hideContent]="false" + [dropdownHoverSubject]="getDropdownHoverSubject()" + [showFilesSubject]="getShowFilesSubject()" /> } }
+
+ +
@if (plagiarismComparison) { @if (isModelingExercise) { @@ -33,6 +40,8 @@ [plagiarismSubmission]="getTextSubmissionB()" [isLockFilesEnabled]="isLockFilesEnabled" [hideContent]="forStudent && dayjs(exercise.dueDate).isAfter(dayjs())" + [dropdownHoverSubject]="getDropdownHoverSubject()" + [showFilesSubject]="getShowFilesSubject()" /> } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.scss b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.scss index 4956ee867320..12ac2a9ad4a2 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.scss +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.scss @@ -19,3 +19,7 @@ position: relative; } } + +.split-pane-header-color { + background-color: var(--bs-body-bg); +} diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index c05e6b4e5bb9..8405241b0a8b 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -11,6 +11,7 @@ import { HttpResponse } from '@angular/common/http'; import { SimpleMatch } from 'app/exercises/shared/plagiarism/types/PlagiarismMatch'; import dayjs from 'dayjs/esm'; import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; +import { IconDefinition, faLock, faUnlock } from '@fortawesome/free-solid-svg-icons'; @Directive({ selector: '[jhiPane]' }) export class SplitPaneDirective { @@ -28,12 +29,13 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O @Input() splitControlSubject: Subject; @Input() sortByStudentLogin: string; @Input() forStudent: boolean; - @Input() isLockFilesEnabled = false; @ViewChildren(SplitPaneDirective) panes!: QueryList; plagiarismComparison: PlagiarismComparison; private fileSelectedSubject = new Subject(); + private showFilesSubject = new Subject(); + private dropdownHoverSubject = new Subject(); public split: Split.Instance; @@ -42,8 +44,11 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O public matchesA: Map; public matchesB: Map; + isLockFilesEnabled = false; readonly dayjs = dayjs; + protected readonly faLock: IconDefinition = faLock; + protected readonly faUnlock: IconDefinition = faUnlock; constructor(private plagiarismCasesService: PlagiarismCasesService) {} @@ -89,6 +94,8 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O ngOnDestroy() { this.fileSelectedSubject.complete(); + this.showFilesSubject.complete(); + this.dropdownHoverSubject.complete(); } /** @@ -118,6 +125,14 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O return this.fileSelectedSubject; } + /** + * get the subject/listener for checking dropdown toggle status of split-pane-header in text-viewer component + * @returns subject for showFile change + */ + getShowFilesSubject() { + return this.showFilesSubject; + } + parseTextMatches(plagComparison: PlagiarismComparison) { if (plagComparison.matches) { const matchesA = plagComparison.matches.map((match) => new SimpleMatch(match.startA, match.length)).sort((m1, m2) => m1.start - m2.start); @@ -192,4 +207,15 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O } } } + + getDropdownHoverSubject() { + return this.dropdownHoverSubject; + } + + /** + * Toggles the state of file locking and emits the new state to the parent component. + */ + toggleLockFiles() { + this.isLockFilesEnabled = !this.isLockFilesEnabled; + } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html index 865d3dfe654c..e5a8c7b201d4 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html @@ -11,7 +11,13 @@ @if (showFiles) {
    @for (file of files; track file; let idx = $index) { -
  • +
  • {{ file.file }}
    • } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss index 7b335e961b86..c6f405d8eb97 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss @@ -50,3 +50,7 @@ overflow: scroll; } } + +.split-pane-header-file.hover { + background-color: lightskyblue; +} diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index d9442ae9c924..a36f7f33a3c1 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -21,25 +21,83 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { @Input() studentLogin: string; @Input() fileSelectedSubject!: Subject; @Input() isLockFilesEnabled!: boolean; + @Input() showFilesSubject!: Subject; + @Input() dropdownHoverSubject!: Subject; @Output() selectFile = new EventEmitter(); public showFiles = false; public activeFileIndex = 0; - fileSelectSubscription: Subscription; + private fileSelectSubscription: Subscription; + private showFilesSubscription: Subscription; // Icons faChevronDown = faChevronDown; + hoveredFileIdx: number; ngOnInit(): void { + this.subscribeToFileSelection(); + this.subscribeToShowFiles(); + this.subscribeToDropdownHover(); + } + + /** + * subscribes to listening onto file changes in component instance + * @private helper method + */ + private subscribeToFileSelection(): void { this.fileSelectSubscription = this.fileSelectedSubject.subscribe((val) => { - if (val.file && this.isLockFilesEnabled) { - this.handleFileSelectWithoutPropagation(val.file, val.idx); + if (this.isLockFilesEnabled) { + this.handleLockedFileSelection(val.file, val.idx); + } + }); + } + + private handleLockedFileSelection(file: FileWithHasMatch, idx: number): void { + let index; + if (this.files[idx]?.file === file.file) { + this.handleFileSelectWithoutPropagation(file, idx); + } else if ((index = this.getIndexOf(file)) > 0) { + this.handleFileSelectWithoutPropagation(file, index); + } else { + this.showFiles = false; + } + } + + /** + * subscribes to listening onto dropdown toggle in component instance + * @private helper method + */ + private subscribeToShowFiles(): void { + this.showFilesSubscription = this.showFilesSubject.subscribe((showFiles) => { + if (this.isLockFilesEnabled || (!this.isLockFilesEnabled && !showFiles)) { + this.toggleShowFilesWithoutPropagation(showFiles); } }); } + /** + * subscribes to listening onto mouse enter changes in dropdown in component instance + * @private helper method + */ + private subscribeToDropdownHover(): void { + this.dropdownHoverSubject.subscribe((val) => { + if (this.isLockFilesEnabled) { + this.handleDropdownHover(val.file, val.idx); + } + }); + } + + private handleDropdownHover(file: FileWithHasMatch, idx: number): void { + let index; + if (this.files[idx]?.file === file.file) { + this.hoveredFileIdx = idx; + } else if ((index = this.getIndexOf(file)) > 0) { + this.hoveredFileIdx = index; + } else this.hoveredFileIdx = -1; + } + ngOnChanges(changes: SimpleChanges) { if (changes.files) { const fileWithHasMatch: FileWithHasMatch[] = changes.files.currentValue; @@ -56,6 +114,10 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { if (this.fileSelectSubscription) { this.fileSelectSubscription.unsubscribe(); } + + if (this.showFilesSubscription) { + this.showFilesSubscription.unsubscribe(); + } } hasActiveFile(): boolean { @@ -87,6 +149,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { this.activeFileIndex = idx; this.showFiles = false; this.selectFile.emit(file.file); + file.hasMatch = true; } hasFiles(): boolean { @@ -96,6 +159,30 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { toggleShowFiles(): void { if (this.hasFiles()) { this.showFiles = !this.showFiles; + this.showFilesSubject.next(this.showFiles); } } + + /** + * handles toggle of the dropdown, do NOT propagate change to emit toggle to parent component component for lock sync + * @param showFiles dropdown toggle status + */ + toggleShowFilesWithoutPropagation(showFiles: boolean): void { + if (this.hasFiles()) { + this.showFiles = showFiles; + } + } + + triggerMouseEnter(file: FileWithHasMatch, idx: number) { + this.dropdownHoverSubject.next({ idx: idx, file: file }); + } + + /** + * gets index of the file if it exists + * @param file The file to look up. + * @returns index if found, -1 otherwise + */ + private getIndexOf(file: FileWithHasMatch): number { + return this.files.findIndex((f) => f.file === file.file); + } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html index d58609f29b60..00cf96ec5037 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.html @@ -3,6 +3,8 @@ [isLockFilesEnabled]="this.isLockFilesEnabled" [fileSelectedSubject]="fileSelectedSubject" (selectFile)="handleFileSelect($event)" + [showFilesSubject]="showFilesSubject" + [dropdownHoverSubject]="dropdownHoverSubject" studentLogin="{{ plagiarismSubmission?.studentLogin }}" /> @if (cannotLoadFiles) { diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts index 3f1b5e29dd52..edf51ca92de9 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts @@ -29,7 +29,9 @@ export class TextSubmissionViewerComponent implements OnChanges { @Input() plagiarismSubmission: PlagiarismSubmission; @Input() hideContent: boolean; @Input() fileSelectedSubject!: Subject; - @Input() isLockFilesEnabled = false; + @Input() isLockFilesEnabled: boolean; + @Input() showFilesSubject!: Subject; + @Input() dropdownHoverSubject!: Subject; /** * Name of the currently selected file. diff --git a/src/main/webapp/i18n/de/plagiarism.json b/src/main/webapp/i18n/de/plagiarism.json index 3461f3818dfd..2e8f1d30798f 100644 --- a/src/main/webapp/i18n/de/plagiarism.json +++ b/src/main/webapp/i18n/de/plagiarism.json @@ -4,8 +4,6 @@ "plagiarismDetection": "Plagiatskontrolle", "confirm": "Bestätigen", "deny": "Ablehnen", - "lockSync": "Sync sperren", - "unlockSync": "Sync entsperren", "denyAfterConfirmModalTitle": "Wechsel von Bestätigen zu Ablehnen", "denyAfterConfirmModalText": "Bist du dir sicher, dass du die Entscheidung von \"Bestätigung des Plagiats\" in \"Ablehnung\" ändern möchtest? Dadurch wird der entsprechende Plagiatsfall einschließlich der Kommunikation mit dem/der Studierenden und des Urteils gelöscht und kann nicht rückgängig gemacht werden.", "warning": "Die Bestätigung/Ablehnung eines Plagiats geht beim Neuladen der Seite verloren.", diff --git a/src/main/webapp/i18n/en/plagiarism.json b/src/main/webapp/i18n/en/plagiarism.json index 149a6decf416..8f071d94b798 100644 --- a/src/main/webapp/i18n/en/plagiarism.json +++ b/src/main/webapp/i18n/en/plagiarism.json @@ -4,8 +4,6 @@ "plagiarismDetection": "Plagiarism Detection", "confirm": "Confirm", "deny": "Deny", - "lockSync": "Lock Sync", - "unlockSync": "Unlock Sync", "denyAfterConfirmModalTitle": "Change from confirm to deny", "denyAfterConfirmModalText": "Are you sure that you want to change the decision from confirming the plagiarism to denying it? This will delete the corresponding plagiarism case incl. the communication with the student and the verdict and cannot be undone.", "warning": "Confirmation/Denial of plagiarism will be lost when refreshing the page.", From 14fbd0af10c6e422d52975211a6669a93e933294 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Wed, 27 Nov 2024 14:36:07 +0100 Subject: [PATCH 13/26] fix minor indexing and add tests for coverage --- .../plagiarism-details.component.ts | 5 -- .../split-pane-header.component.ts | 4 +- .../split-pane-header.component.spec.ts | 79 ++++++++++++++++++- 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts index 60abc363abe2..a17fe6ff2421 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.ts @@ -18,9 +18,4 @@ export class PlagiarismDetailsComponent { * Subject to be passed into PlagiarismSplitViewComponent to control the split view. */ splitControlSubject: Subject = new Subject(); - - /** - * Boolean to determine whether file locking is enabled. - */ - isLockFilesEnabled: boolean = false; } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index a36f7f33a3c1..8ec2f2f67dbf 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -58,7 +58,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { let index; if (this.files[idx]?.file === file.file) { this.handleFileSelectWithoutPropagation(file, idx); - } else if ((index = this.getIndexOf(file)) > 0) { + } else if ((index = this.getIndexOf(file)) >= 0) { this.handleFileSelectWithoutPropagation(file, index); } else { this.showFiles = false; @@ -93,7 +93,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { let index; if (this.files[idx]?.file === file.file) { this.hoveredFileIdx = idx; - } else if ((index = this.getIndexOf(file)) > 0) { + } else if ((index = this.getIndexOf(file)) >= 0) { this.hoveredFileIdx = index; } else this.hoveredFileIdx = -1; } diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index d927cd1bcaa5..65f830b5f2f7 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -13,6 +13,7 @@ import { MockComponent, MockDirective, MockPipe } from 'ng-mocks'; import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; import { Subject } from 'rxjs'; import { TextSubmissionViewerComponent } from 'app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component'; +import { By } from '@angular/platform-browser'; describe('SplitPaneHeaderComponent', () => { let comp1: SplitPaneHeaderComponent; @@ -43,6 +44,8 @@ describe('SplitPaneHeaderComponent', () => { .compileComponents() .then(() => { const fileSelectedSubject = new Subject(); + const showFilesSubject = new Subject(); + const dropdownHoverSubject = new Subject(); fixture1 = TestBed.createComponent(SplitPaneHeaderComponent); comp1 = fixture1.componentInstance; fixture2 = TestBed.createComponent(SplitPaneHeaderComponent); @@ -52,11 +55,15 @@ describe('SplitPaneHeaderComponent', () => { comp1.studentLogin = 'ts10abc'; comp1.selectFile = new EventEmitter(); comp1.fileSelectedSubject = fileSelectedSubject; + comp1.showFilesSubject = showFilesSubject; + comp1.dropdownHoverSubject = dropdownHoverSubject; comp2.files = []; comp2.studentLogin = 'ts20abc'; comp2.selectFile = new EventEmitter(); comp2.fileSelectedSubject = fileSelectedSubject; + comp2.showFilesSubject = showFilesSubject; + comp2.dropdownHoverSubject = dropdownHoverSubject; }); }); it('resets the active file index on change', () => { @@ -152,6 +159,10 @@ describe('SplitPaneHeaderComponent', () => { const selectedFile = { idx: idx, file: files[idx] }; const lockFilesEnabled = true; + comp1.files = files; + comp2.files = files; + comp1.showFiles = true; + comp2.showFiles = true; comp1.isLockFilesEnabled = lockFilesEnabled; comp2.isLockFilesEnabled = lockFilesEnabled; @@ -168,7 +179,7 @@ describe('SplitPaneHeaderComponent', () => { expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledWith(selectedFile.file, selectedFile.idx); }); - it('should not sync file selection when lockFilesEnabled true', () => { + it('should not sync file selection when lockFilesEnabled false', () => { const idx = 0; const selectedFile = { idx: idx, file: files[idx] }; const lockFilesEnabled = false; @@ -185,4 +196,70 @@ describe('SplitPaneHeaderComponent', () => { expect(handleFileSelectWithoutPropagationSpy).not.toHaveBeenCalled(); }); + + it('should trigger dropdown hover subject on mouseenter on the first file element', () => { + comp1.files = files; + comp1.showFiles = true; + + fixture1.detectChanges(); + + const fileList = fixture1.debugElement.query(By.css('.split-pane-header-files')); + expect(fileList).toBeTruthy(); + + const fileItems = fileList.queryAll(By.css('.split-pane-header-file')); + expect(fileItems.length).toBeGreaterThan(0); + + const firstFileItem = fileItems[0]; + const triggerMouseEnterSpy = jest.spyOn(comp1 as any, 'triggerMouseEnter'); + + firstFileItem.triggerEventHandler('mouseenter', null); + expect(triggerMouseEnterSpy).toHaveBeenCalledWith(comp1.files[0], 0); + }); + + it('should create dropdownHoverSubject on ngOnInit', () => { + // Arrange + const mockFile = files[0]; + const mockIdx = 0; + + comp1.files = files; + comp1.isLockFilesEnabled = true; + jest.spyOn(comp1 as any, 'handleDropdownHover'); + + comp1.ngOnInit(); + + comp1.dropdownHoverSubject.next({ file: mockFile, idx: mockIdx }); + + expect(comp1['handleDropdownHover']).toHaveBeenCalledWith(mockFile, mockIdx); + expect(comp1.hoveredFileIdx).toBe(mockIdx); + }); + + it('should update showFiles when hasFiles returns true', () => { + comp1.hasFiles = jest.fn().mockReturnValue(true); + const initialShowFiles = comp1.showFiles; + + comp1.toggleShowFilesWithoutPropagation(true); + + expect(comp1.showFiles).toBeTrue(initialShowFiles); + }); + + it('should not update showFiles when hasFiles returns false', () => { + comp1.hasFiles = jest.fn().mockReturnValue(false); + const initialShowFiles = comp1.showFiles; + + comp1.toggleShowFilesWithoutPropagation(true); + + expect(comp1.showFiles).toBe(initialShowFiles); + }); + + it('should set hoveredFileIdx to -1 if file does not match and getIndexOf returns -1', () => { + const mockFile = { file: 'testFile', hasMatch: true }; + const mockIdx = files.length; + + comp1.files = files; + jest.spyOn(comp1 as any, 'getIndexOf').mockReturnValue(-1); + + (comp1 as any).handleDropdownHover(mockFile, mockIdx); + + expect(comp1.hoveredFileIdx).toBe(-1); + }); }); From ac6c46ae044e710f81ac79f134d4029bdaaa4945 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Thu, 28 Nov 2024 01:13:13 +0100 Subject: [PATCH 14/26] adapt split pane header to new client code guidelines and adapt tests --- .../split-pane-header.component.ts | 30 +++++++++--------- .../split-pane-header.component.spec.ts | 31 ++++++++++--------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 8ec2f2f67dbf..aa2355a71453 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; +import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges, input } from '@angular/core'; import { faChevronDown } from '@fortawesome/free-solid-svg-icons'; import { Subject, Subscription } from 'rxjs'; import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement'; @@ -19,10 +19,10 @@ export type FileWithHasMatch = { export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { @Input() files: FileWithHasMatch[]; @Input() studentLogin: string; - @Input() fileSelectedSubject!: Subject; - @Input() isLockFilesEnabled!: boolean; - @Input() showFilesSubject!: Subject; - @Input() dropdownHoverSubject!: Subject; + fileSelectedSubject = input>(); + isLockFilesEnabled = input(); + showFilesSubject = input>(); + dropdownHoverSubject = input>(); @Output() selectFile = new EventEmitter(); @@ -47,8 +47,8 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToFileSelection(): void { - this.fileSelectSubscription = this.fileSelectedSubject.subscribe((val) => { - if (this.isLockFilesEnabled) { + this.fileSelectSubscription = this.fileSelectedSubject()!.subscribe((val) => { + if (this.isLockFilesEnabled()) { this.handleLockedFileSelection(val.file, val.idx); } }); @@ -70,8 +70,8 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToShowFiles(): void { - this.showFilesSubscription = this.showFilesSubject.subscribe((showFiles) => { - if (this.isLockFilesEnabled || (!this.isLockFilesEnabled && !showFiles)) { + this.showFilesSubscription = this.showFilesSubject()!.subscribe((showFiles) => { + if (this.isLockFilesEnabled()! || (!this.isLockFilesEnabled()! && !showFiles)) { this.toggleShowFilesWithoutPropagation(showFiles); } }); @@ -82,8 +82,8 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToDropdownHover(): void { - this.dropdownHoverSubject.subscribe((val) => { - if (this.isLockFilesEnabled) { + this.dropdownHoverSubject()!.subscribe((val) => { + if (this.isLockFilesEnabled()) { this.handleDropdownHover(val.file, val.idx); } }); @@ -134,7 +134,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @param idx index of the file from the dropdown */ handleFileSelect(file: FileWithHasMatch, idx: number): void { - this.fileSelectedSubject.next({ idx: idx, file: file }); + this.fileSelectedSubject()!.next({ idx: idx, file: file }); this.activeFileIndex = idx; this.showFiles = false; this.selectFile.emit(file.file); @@ -153,13 +153,13 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } hasFiles(): boolean { - return this.files && this.files.length > 0; + return (this.files && this.files.length > 0) ?? false; } toggleShowFiles(): void { if (this.hasFiles()) { this.showFiles = !this.showFiles; - this.showFilesSubject.next(this.showFiles); + this.showFilesSubject()!.next(this.showFiles); } } @@ -174,7 +174,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } triggerMouseEnter(file: FileWithHasMatch, idx: number) { - this.dropdownHoverSubject.next({ idx: idx, file: file }); + this.dropdownHoverSubject()!.next({ idx: idx, file: file }); } /** diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 65f830b5f2f7..243faabaae0d 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -54,16 +54,18 @@ describe('SplitPaneHeaderComponent', () => { comp1.files = []; comp1.studentLogin = 'ts10abc'; comp1.selectFile = new EventEmitter(); - comp1.fileSelectedSubject = fileSelectedSubject; - comp1.showFilesSubject = showFilesSubject; - comp1.dropdownHoverSubject = dropdownHoverSubject; + fixture1.componentRef.setInput('fileSelectedSubject', fileSelectedSubject); + + fixture1.componentRef.setInput('fileSelectedSubject', fileSelectedSubject); + fixture1.componentRef.setInput('showFilesSubject', showFilesSubject); + fixture1.componentRef.setInput('dropdownHoverSubject', dropdownHoverSubject); comp2.files = []; comp2.studentLogin = 'ts20abc'; comp2.selectFile = new EventEmitter(); - comp2.fileSelectedSubject = fileSelectedSubject; - comp2.showFilesSubject = showFilesSubject; - comp2.dropdownHoverSubject = dropdownHoverSubject; + fixture2.componentRef.setInput('fileSelectedSubject', fileSelectedSubject); + fixture2.componentRef.setInput('showFilesSubject', showFilesSubject); + fixture2.componentRef.setInput('dropdownHoverSubject', dropdownHoverSubject); }); }); it('resets the active file index on change', () => { @@ -144,11 +146,11 @@ describe('SplitPaneHeaderComponent', () => { it('should emit selected file through fileSelectedSubject', () => { const selectedFile = { idx: 0, file: files[0] }; let emittedFile: TextPlagiarismFileElement | undefined; - comp1.fileSelectedSubject.subscribe((file) => { + comp1.fileSelectedSubject()!.subscribe((file) => { emittedFile = file; }); - comp1.fileSelectedSubject.next(selectedFile); + comp1.fileSelectedSubject()!.next(selectedFile); // Assert expect(emittedFile).toBe(selectedFile); @@ -163,8 +165,8 @@ describe('SplitPaneHeaderComponent', () => { comp2.files = files; comp1.showFiles = true; comp2.showFiles = true; - comp1.isLockFilesEnabled = lockFilesEnabled; - comp2.isLockFilesEnabled = lockFilesEnabled; + fixture1.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); + fixture2.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp2, 'handleFileSelectWithoutPropagation'); @@ -184,8 +186,8 @@ describe('SplitPaneHeaderComponent', () => { const selectedFile = { idx: idx, file: files[idx] }; const lockFilesEnabled = false; - comp1.isLockFilesEnabled = lockFilesEnabled; - comp2.isLockFilesEnabled = lockFilesEnabled; + fixture1.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); + fixture2.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp1, 'handleFileSelectWithoutPropagation'); fixture1.detectChanges(); @@ -222,12 +224,11 @@ describe('SplitPaneHeaderComponent', () => { const mockIdx = 0; comp1.files = files; - comp1.isLockFilesEnabled = true; + fixture1.componentRef.setInput('isLockFilesEnabled', true); jest.spyOn(comp1 as any, 'handleDropdownHover'); comp1.ngOnInit(); - - comp1.dropdownHoverSubject.next({ file: mockFile, idx: mockIdx }); + comp1.dropdownHoverSubject()!.next({ file: mockFile, idx: mockIdx }); expect(comp1['handleDropdownHover']).toHaveBeenCalledWith(mockFile, mockIdx); expect(comp1.hoveredFileIdx).toBe(mockIdx); From b2272f9e852e6178fa60260f0d030236571bf9c8 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Thu, 28 Nov 2024 01:18:39 +0100 Subject: [PATCH 15/26] fix test --- .../component/plagiarism/split-pane-header.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 243faabaae0d..5231b4aaee7f 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -240,7 +240,7 @@ describe('SplitPaneHeaderComponent', () => { comp1.toggleShowFilesWithoutPropagation(true); - expect(comp1.showFiles).toBeTrue(initialShowFiles); + expect(comp1.showFiles).not.toBe(initialShowFiles); }); it('should not update showFiles when hasFiles returns false', () => { From 24bc8895745840a6ca0db6a4818520ef8a344171 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Thu, 28 Nov 2024 01:29:52 +0100 Subject: [PATCH 16/26] fix test --- .../component/plagiarism/split-pane-header.component.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 5231b4aaee7f..45aeaa2d2afc 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -54,8 +54,6 @@ describe('SplitPaneHeaderComponent', () => { comp1.files = []; comp1.studentLogin = 'ts10abc'; comp1.selectFile = new EventEmitter(); - fixture1.componentRef.setInput('fileSelectedSubject', fileSelectedSubject); - fixture1.componentRef.setInput('fileSelectedSubject', fileSelectedSubject); fixture1.componentRef.setInput('showFilesSubject', showFilesSubject); fixture1.componentRef.setInput('dropdownHoverSubject', dropdownHoverSubject); From dfc8c281bd566aa7758dbf5a6f2b408e23b0c785 Mon Sep 17 00:00:00 2001 From: ajay_chauhan Date: Thu, 28 Nov 2024 01:31:01 +0100 Subject: [PATCH 17/26] fix test --- .../component/plagiarism/split-pane-header.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 45aeaa2d2afc..1483dc217402 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -176,7 +176,7 @@ describe('SplitPaneHeaderComponent', () => { fixture1.detectChanges(); fixture2.detectChanges(); - expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledWith(selectedFile.file, selectedFile.idx); + expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledExactlyOnceWith(selectedFile.file, selectedFile.idx); }); it('should not sync file selection when lockFilesEnabled false', () => { From 6c2a4c46ee77e8790a50eb74c5f418ccc7d43929 Mon Sep 17 00:00:00 2001 From: Ajayvir Singh <38434017+AjayvirS@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:10:50 +0100 Subject: [PATCH 18/26] ramona fixes Co-authored-by: Ramona Beinstingel <75392103+rabeatwork@users.noreply.github.com> --- .../split-pane-header/split-pane-header.component.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index aa2355a71453..37c74cd5555a 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -111,9 +111,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } ngOnDestroy(): void { - if (this.fileSelectSubscription) { - this.fileSelectSubscription.unsubscribe(); - } + this.fileSelectSubscription?.unsubscribe(); if (this.showFilesSubscription) { this.showFilesSubscription.unsubscribe(); From ea194cb9aaed21983d144cd1745b0db862f8ff09 Mon Sep 17 00:00:00 2001 From: Ajayvir Singh <38434017+AjayvirS@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:11:17 +0100 Subject: [PATCH 19/26] ramona fixes Co-authored-by: Ramona Beinstingel <75392103+rabeatwork@users.noreply.github.com> --- .../split-pane-header/split-pane-header.component.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 37c74cd5555a..fe545ea14363 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -113,9 +113,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { ngOnDestroy(): void { this.fileSelectSubscription?.unsubscribe(); - if (this.showFilesSubscription) { - this.showFilesSubscription.unsubscribe(); - } + this.showFilesSubscription?.unsubscribe(); } hasActiveFile(): boolean { From a0b53fad0fc679a46e98cc2f2099f0a5c7e7fb8e Mon Sep 17 00:00:00 2001 From: Ajayvir Singh <38434017+AjayvirS@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:11:44 +0100 Subject: [PATCH 20/26] ramona fixes Co-authored-by: Ramona Beinstingel <75392103+rabeatwork@users.noreply.github.com> --- .../split-pane-header/split-pane-header.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index fe545ea14363..281030dea6a4 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -149,7 +149,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } hasFiles(): boolean { - return (this.files && this.files.length > 0) ?? false; + return this.files?.length ? true : false; } toggleShowFiles(): void { From 38910dbae4763795c09042bbd400839c348ff75e Mon Sep 17 00:00:00 2001 From: "Ajayvir S." Date: Tue, 3 Dec 2024 18:09:49 +0100 Subject: [PATCH 21/26] ramona fixes --- .../plagiarism-details.component.html | 4 +- .../plagiarism-split-view.component.html | 12 +-- .../plagiarism-split-view.component.ts | 26 +----- .../split-pane-header.component.html | 6 +- .../split-pane-header.component.ts | 79 ++++++++----------- 5 files changed, 47 insertions(+), 80 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html index c66bd6ef1907..3fa33629ca83 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-details/plagiarism-details.component.html @@ -1,6 +1,6 @@ @if (comparison) {
    - - + +
    } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html index d7ea6b48f411..603fae72e0a4 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.html @@ -6,14 +6,14 @@ } @if (isProgrammingOrTextExercise) { } } @@ -34,14 +34,14 @@ } @if (isProgrammingOrTextExercise) { } } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts index 8405241b0a8b..c8e6fe103fa5 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/plagiarism-split-view.component.ts @@ -33,9 +33,9 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O @ViewChildren(SplitPaneDirective) panes!: QueryList; plagiarismComparison: PlagiarismComparison; - private fileSelectedSubject = new Subject(); - private showFilesSubject = new Subject(); - private dropdownHoverSubject = new Subject(); + fileSelectedSubject = new Subject(); + showFilesSubject = new Subject(); + dropdownHoverSubject = new Subject(); public split: Split.Instance; @@ -117,22 +117,6 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O } } - /** - * get the subject/listener for file selection - * @returns observable for fileselection - */ - getFileSelectedSubject() { - return this.fileSelectedSubject; - } - - /** - * get the subject/listener for checking dropdown toggle status of split-pane-header in text-viewer component - * @returns subject for showFile change - */ - getShowFilesSubject() { - return this.showFilesSubject; - } - parseTextMatches(plagComparison: PlagiarismComparison) { if (plagComparison.matches) { const matchesA = plagComparison.matches.map((match) => new SimpleMatch(match.startA, match.length)).sort((m1, m2) => m1.start - m2.start); @@ -208,10 +192,6 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O } } - getDropdownHoverSubject() { - return this.dropdownHoverSubject; - } - /** * Toggles the state of file locking and emits the new state to the parent component. */ diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html index e5a8c7b201d4..87054cf43bdb 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html @@ -1,5 +1,5 @@
    -
    +
    @if (hasActiveFile()) {
    @@ -15,8 +15,8 @@ class="split-pane-header-file" ngbDropdownItem (mouseenter)="triggerMouseEnter(file, idx)" - (click)="handleFileSelect(file, idx)" - [ngClass]="{ hover: idx === hoveredFileIdx }" + (click)="handleFileSelect(file, idx, true)" + [ngClass]="{ hover: idx === hoveredFileIndex }" > {{ file.file }} diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 281030dea6a4..5b1b64323626 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -31,10 +31,11 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { private fileSelectSubscription: Subscription; private showFilesSubscription: Subscription; + private dropdownHoverSubscription: Subscription; // Icons faChevronDown = faChevronDown; - hoveredFileIdx: number; + hoveredFileIndex: number; ngOnInit(): void { this.subscribeToFileSelection(); @@ -47,19 +48,18 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToFileSelection(): void { - this.fileSelectSubscription = this.fileSelectedSubject()!.subscribe((val) => { + this.fileSelectSubscription = this.fileSelectedSubject()!.subscribe((textPlagiarismElement) => { if (this.isLockFilesEnabled()) { - this.handleLockedFileSelection(val.file, val.idx); + this.handleLockedFileSelection(textPlagiarismElement.file, textPlagiarismElement.idx); } }); } private handleLockedFileSelection(file: FileWithHasMatch, idx: number): void { - let index; - if (this.files[idx]?.file === file.file) { - this.handleFileSelectWithoutPropagation(file, idx); - } else if ((index = this.getIndexOf(file)) >= 0) { - this.handleFileSelectWithoutPropagation(file, index); + const index = this.files[idx]?.file === file.file ? idx : this.getIndexOf(file); + + if (index >= 0) { + this.handleFileSelect(file, index, false); } else { this.showFiles = false; } @@ -72,7 +72,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { private subscribeToShowFiles(): void { this.showFilesSubscription = this.showFilesSubject()!.subscribe((showFiles) => { if (this.isLockFilesEnabled()! || (!this.isLockFilesEnabled()! && !showFiles)) { - this.toggleShowFilesWithoutPropagation(showFiles); + this.toggleShowFiles(false, showFiles); } }); } @@ -82,20 +82,17 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToDropdownHover(): void { - this.dropdownHoverSubject()!.subscribe((val) => { + this.dropdownHoverSubscription = this.dropdownHoverSubject()!.subscribe((textPlagiarismElement) => { if (this.isLockFilesEnabled()) { - this.handleDropdownHover(val.file, val.idx); + this.handleDropdownHover(textPlagiarismElement.file, textPlagiarismElement.idx); } }); } private handleDropdownHover(file: FileWithHasMatch, idx: number): void { - let index; - if (this.files[idx]?.file === file.file) { - this.hoveredFileIdx = idx; - } else if ((index = this.getIndexOf(file)) >= 0) { - this.hoveredFileIdx = index; - } else this.hoveredFileIdx = -1; + const index = this.files[idx]?.file === file.file ? idx : this.getIndexOf(file); + + this.hoveredFileIndex = index >= 0 ? index : -1; } ngOnChanges(changes: SimpleChanges) { @@ -111,9 +108,9 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } ngOnDestroy(): void { - this.fileSelectSubscription?.unsubscribe(); - - this.showFilesSubscription?.unsubscribe(); + this.fileSelectSubscription?.unsubscribe(); + this.showFilesSubscription?.unsubscribe(); + this.dropdownHoverSubscription?.unsubscribe(); } hasActiveFile(): boolean { @@ -128,44 +125,34 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * handles selection of file from dropdown, propagates change to fileslectionsubject component for lock sync * @param file to be selected * @param idx index of the file from the dropdown + * @param propagateChanges propagate changes to listeners subscribed to fileSelectedSubject */ - handleFileSelect(file: FileWithHasMatch, idx: number): void { - this.fileSelectedSubject()!.next({ idx: idx, file: file }); - this.activeFileIndex = idx; - this.showFiles = false; - this.selectFile.emit(file.file); - } - - /** - * handles selection of file from dropdown, do NOT propagates change to fileslectionsubject component for lock sync - * @param file to be selected - * @param idx index of the file from the dropdown - */ - handleFileSelectWithoutPropagation(file: FileWithHasMatch, idx: number) { + handleFileSelect(file: FileWithHasMatch, idx: number, propagateChanges: boolean): void { + if (propagateChanges) { + this.fileSelectedSubject()!.next({ idx: idx, file: file }); + file.hasMatch = true; + } this.activeFileIndex = idx; this.showFiles = false; this.selectFile.emit(file.file); - file.hasMatch = true; } hasFiles(): boolean { - return this.files?.length ? true : false; - } - - toggleShowFiles(): void { - if (this.hasFiles()) { - this.showFiles = !this.showFiles; - this.showFilesSubject()!.next(this.showFiles); - } + return !!this.files?.length; } /** - * handles toggle of the dropdown, do NOT propagate change to emit toggle to parent component component for lock sync - * @param showFiles dropdown toggle status + * Toggles the dropdown visibility and optionally propagates changes to the parent component. + * @param showFiles Optional toggle status; if undefined, the status will be toggled. + * @param propagateChanges Whether to propagate the change to the parent component. */ - toggleShowFilesWithoutPropagation(showFiles: boolean): void { + toggleShowFiles(propagateChanges: boolean, showFiles?: boolean): void { if (this.hasFiles()) { - this.showFiles = showFiles; + this.showFiles = showFiles !== undefined ? showFiles : !this.showFiles; + + if (propagateChanges) { + this.showFilesSubject()!.next(this.showFiles); + } } } From 442d81f7f47e197f0a531fd32952ff3d0ea4d1a9 Mon Sep 17 00:00:00 2001 From: "Ajayvir S." Date: Tue, 3 Dec 2024 23:12:00 +0100 Subject: [PATCH 22/26] ramona fix tests --- .../split-pane-header.component.spec.ts | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts index 1483dc217402..d4540192235f 100644 --- a/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts +++ b/src/test/javascript/spec/component/plagiarism/split-pane-header.component.spec.ts @@ -106,7 +106,7 @@ describe('SplitPaneHeaderComponent', () => { comp1.showFiles = true; jest.spyOn(comp1.selectFile, 'emit'); - comp1.handleFileSelect(files[idx], idx); + comp1.handleFileSelect(files[idx], idx, true); expect(comp1.activeFileIndex).toBe(idx); expect(comp1.showFiles).toBeFalse(); @@ -128,7 +128,7 @@ describe('SplitPaneHeaderComponent', () => { comp1.showFiles = false; comp1.files = files; - comp1.toggleShowFiles(); + comp1.toggleShowFiles(false); expect(comp1.showFiles).toBeTrue(); }); @@ -136,7 +136,7 @@ describe('SplitPaneHeaderComponent', () => { it('does not toggle "show files"', () => { comp1.showFiles = false; - comp1.toggleShowFiles(); + comp1.toggleShowFiles(false); expect(comp1.showFiles).toBeFalse(); }); @@ -166,17 +166,17 @@ describe('SplitPaneHeaderComponent', () => { fixture1.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); fixture2.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); - const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp2, 'handleFileSelectWithoutPropagation'); + const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp2, 'handleFileSelect'); fixture1.detectChanges(); fixture2.detectChanges(); - comp1.handleFileSelect(selectedFile.file, selectedFile.idx); + comp1.handleFileSelect(selectedFile.file, selectedFile.idx, true); fixture1.detectChanges(); fixture2.detectChanges(); - expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledExactlyOnceWith(selectedFile.file, selectedFile.idx); + expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledExactlyOnceWith(selectedFile.file, selectedFile.idx, false); }); it('should not sync file selection when lockFilesEnabled false', () => { @@ -187,14 +187,17 @@ describe('SplitPaneHeaderComponent', () => { fixture1.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); fixture2.componentRef.setInput('isLockFilesEnabled', lockFilesEnabled); - const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp1, 'handleFileSelectWithoutPropagation'); + const handleFileSelect = jest.spyOn(comp1, 'handleFileSelect'); + const handleFileSelectWithoutPropagationSpy = jest.spyOn(comp2, 'handleFileSelect'); + fixture1.detectChanges(); fixture2.detectChanges(); - comp1.handleFileSelect(selectedFile.file, selectedFile.idx); + comp1.handleFileSelect(selectedFile.file, selectedFile.idx, true); fixture1.detectChanges(); fixture2.detectChanges(); - expect(handleFileSelectWithoutPropagationSpy).not.toHaveBeenCalled(); + expect(handleFileSelectWithoutPropagationSpy).toHaveBeenCalledTimes(0); + expect(handleFileSelect).toHaveBeenCalledOnce(); }); it('should trigger dropdown hover subject on mouseenter on the first file element', () => { @@ -229,14 +232,14 @@ describe('SplitPaneHeaderComponent', () => { comp1.dropdownHoverSubject()!.next({ file: mockFile, idx: mockIdx }); expect(comp1['handleDropdownHover']).toHaveBeenCalledWith(mockFile, mockIdx); - expect(comp1.hoveredFileIdx).toBe(mockIdx); + expect(comp1.hoveredFileIndex).toBe(mockIdx); }); it('should update showFiles when hasFiles returns true', () => { comp1.hasFiles = jest.fn().mockReturnValue(true); const initialShowFiles = comp1.showFiles; - comp1.toggleShowFilesWithoutPropagation(true); + comp1.toggleShowFiles(false); expect(comp1.showFiles).not.toBe(initialShowFiles); }); @@ -245,7 +248,7 @@ describe('SplitPaneHeaderComponent', () => { comp1.hasFiles = jest.fn().mockReturnValue(false); const initialShowFiles = comp1.showFiles; - comp1.toggleShowFilesWithoutPropagation(true); + comp1.toggleShowFiles(false); expect(comp1.showFiles).toBe(initialShowFiles); }); @@ -259,6 +262,6 @@ describe('SplitPaneHeaderComponent', () => { (comp1 as any).handleDropdownHover(mockFile, mockIdx); - expect(comp1.hoveredFileIdx).toBe(-1); + expect(comp1.hoveredFileIndex).toBe(-1); }); }); From 1d6b25c3a62ff2254f7396063accf119eb8e6762 Mon Sep 17 00:00:00 2001 From: "Ajayvir S." Date: Tue, 10 Dec 2024 14:36:59 +0100 Subject: [PATCH 23/26] ramona changes --- .../split-pane-header/split-pane-header.component.scss | 4 ++-- .../split-pane-header/split-pane-header.component.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss index c6f405d8eb97..f4348ef2a551 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.scss @@ -51,6 +51,6 @@ } } -.split-pane-header-file.hover { - background-color: lightskyblue; +.dropdown-item.hover { + background-color: var(--data-table-dropdown-item-selected-background-color); } diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 5b1b64323626..fe5d534879ad 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -29,9 +29,9 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { public showFiles = false; public activeFileIndex = 0; - private fileSelectSubscription: Subscription; - private showFilesSubscription: Subscription; - private dropdownHoverSubscription: Subscription; + private fileSelectSubscription: Subscription | undefined; + private showFilesSubscription: Subscription | undefined; + private dropdownHoverSubscription: Subscription | undefined; // Icons faChevronDown = faChevronDown; @@ -70,7 +70,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToShowFiles(): void { - this.showFilesSubscription = this.showFilesSubject()!.subscribe((showFiles) => { + this.showFilesSubscription = this.showFilesSubject()?.subscribe((showFiles) => { if (this.isLockFilesEnabled()! || (!this.isLockFilesEnabled()! && !showFiles)) { this.toggleShowFiles(false, showFiles); } @@ -82,7 +82,7 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { * @private helper method */ private subscribeToDropdownHover(): void { - this.dropdownHoverSubscription = this.dropdownHoverSubject()!.subscribe((textPlagiarismElement) => { + this.dropdownHoverSubscription = this.dropdownHoverSubject()?.subscribe((textPlagiarismElement) => { if (this.isLockFilesEnabled()) { this.handleDropdownHover(textPlagiarismElement.file, textPlagiarismElement.idx); } From 993b38b86c7a257cb291ee7fb04d001c8efcd8c9 Mon Sep 17 00:00:00 2001 From: "Ajayvir S." Date: Tue, 10 Dec 2024 15:32:30 +0100 Subject: [PATCH 24/26] ramona changes --- .../split-pane-header/split-pane-header.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index fe5d534879ad..10abe1cedf46 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -29,9 +29,9 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { public showFiles = false; public activeFileIndex = 0; - private fileSelectSubscription: Subscription | undefined; - private showFilesSubscription: Subscription | undefined; - private dropdownHoverSubscription: Subscription | undefined; + private fileSelectSubscription?: Subscription; + private showFilesSubscription?: Subscription; + private dropdownHoverSubscription?: Subscription; // Icons faChevronDown = faChevronDown; From 8aa0c0f680f670c94ce31d5beec645d04077c2f2 Mon Sep 17 00:00:00 2001 From: Ajayvir Singh <38434017+AjayvirS@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:20:26 +0100 Subject: [PATCH 25/26] remove non null assertion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../split-pane-header/split-pane-header.component.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts index 10abe1cedf46..0a0b3b84d7ea 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.ts @@ -157,7 +157,10 @@ export class SplitPaneHeaderComponent implements OnChanges, OnInit, OnDestroy { } triggerMouseEnter(file: FileWithHasMatch, idx: number) { - this.dropdownHoverSubject()!.next({ idx: idx, file: file }); + const subject = this.dropdownHoverSubject(); + if (subject) { + subject.next({ idx, file }); + } } /** From e7a7a75510568c5e7d05553567dd0fb7c8c1c28c Mon Sep 17 00:00:00 2001 From: "Ajayvir S." Date: Tue, 17 Dec 2024 18:40:08 +0100 Subject: [PATCH 26/26] add auto close dropdown on mouseleave for better usability --- .../split-pane-header/split-pane-header.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html index 87054cf43bdb..556a5e8bf3bf 100644 --- a/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html +++ b/src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component.html @@ -9,7 +9,7 @@
    {{ studentLogin || ('artemisApp.plagiarism.unknownStudent' | artemisTranslate) }}
    @if (showFiles) { -
      +
        @for (file of files; track file; let idx = $index) {