Skip to content

Commit

Permalink
Plagiarism checks: Improve file selection in comparison (#9789)
Browse files Browse the repository at this point in the history
  • Loading branch information
AjayvirS authored Dec 20, 2024
1 parent 794f57d commit a73231a
Show file tree
Hide file tree
Showing 12 changed files with 383 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,24 @@
<jhi-modeling-submission-viewer [exercise]="exercise" [plagiarismSubmission]="getModelingSubmissionA()" [hideContent]="false" />
}
@if (isProgrammingOrTextExercise) {
<jhi-text-submission-viewer [exercise]="exercise" [matches]="matchesA" [plagiarismSubmission]="getTextSubmissionA()" [hideContent]="false" />
<jhi-text-submission-viewer
[fileSelectedSubject]="this.fileSelectedSubject"
[exercise]="exercise"
[matches]="matchesA"
[plagiarismSubmission]="getTextSubmissionA()"
[isLockFilesEnabled]="isLockFilesEnabled"
[hideContent]="false"
[dropdownHoverSubject]="this.dropdownHoverSubject"
[showFilesSubject]="this.showFilesSubject"
/>
}
}
</div>
<div class="split-pane-header-color">
<button class="btn btn-sm" (click)="toggleLockFiles()" [class.active]="isLockFilesEnabled" data-qa="lock-files-toggle-button">
<fa-icon [icon]="isLockFilesEnabled ? faLock : faUnlock" aria-label="Synchronize files while navigating on both sides"></fa-icon>
</button>
</div>
<div class="plagiarism-split-pane" jhiPane>
@if (plagiarismComparison) {
@if (isModelingExercise) {
Expand All @@ -20,10 +34,14 @@
}
@if (isProgrammingOrTextExercise) {
<jhi-text-submission-viewer
[fileSelectedSubject]="this.fileSelectedSubject"
[exercise]="exercise"
[matches]="matchesB"
[plagiarismSubmission]="getTextSubmissionB()"
[isLockFilesEnabled]="isLockFilesEnabled"
[hideContent]="forStudent && dayjs(exercise.dueDate).isAfter(dayjs())"
[dropdownHoverSubject]="this.dropdownHoverSubject"
[showFilesSubject]="this.showFilesSubject"
/>
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@
position: relative;
}
}

.split-pane-header-color {
background-color: var(--bs-body-bg);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AfterViewInit, Component, Directive, ElementRef, Input, OnChanges, OnInit, QueryList, SimpleChanges, ViewChildren } from '@angular/core';
import { AfterViewInit, Component, Directive, ElementRef, Input, OnChanges, OnDestroy, OnInit, QueryList, SimpleChanges, ViewChildren } from '@angular/core';
import * as Split from 'split.js';
import { Subject } from 'rxjs';
import { PlagiarismComparison } from 'app/exercises/shared/plagiarism/types/PlagiarismComparison';
Expand All @@ -10,6 +10,8 @@ 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 { 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 {
Expand All @@ -21,7 +23,7 @@ export class SplitPaneDirective {
styleUrls: ['./plagiarism-split-view.component.scss'],
templateUrl: './plagiarism-split-view.component.html',
})
export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, OnInit {
export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, OnInit, OnDestroy {
@Input() comparison: PlagiarismComparison<TextSubmissionElement | ModelingSubmissionElement>;
@Input() exercise: Exercise;
@Input() splitControlSubject: Subject<string>;
Expand All @@ -31,6 +33,9 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O
@ViewChildren(SplitPaneDirective) panes!: QueryList<SplitPaneDirective>;

plagiarismComparison: PlagiarismComparison<TextSubmissionElement | ModelingSubmissionElement>;
fileSelectedSubject = new Subject<TextPlagiarismFileElement>();
showFilesSubject = new Subject<boolean>();
dropdownHoverSubject = new Subject<TextPlagiarismFileElement>();

public split: Split.Instance;

Expand All @@ -39,13 +44,16 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O

public matchesA: Map<string, FromToElement[]>;
public matchesB: Map<string, FromToElement[]>;
isLockFilesEnabled = false;

readonly dayjs = dayjs;
protected readonly faLock: IconDefinition = faLock;
protected readonly faUnlock: IconDefinition = faUnlock;

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);
Expand Down Expand Up @@ -84,6 +92,12 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O
}
}

ngOnDestroy() {
this.fileSelectedSubject.complete();
this.showFilesSubject.complete();
this.dropdownHoverSubject.complete();
}

/**
* Swaps fields of A with fields of B in-place.
* More specifically, swaps submissionA with submissionB and startA with startB in matches.
Expand Down Expand Up @@ -177,4 +191,11 @@ export class PlagiarismSplitViewComponent implements AfterViewInit, OnChanges, O
}
}
}

/**
* Toggles the state of file locking and emits the new state to the parent component.
*/
toggleLockFiles() {
this.isLockFilesEnabled = !this.isLockFilesEnabled;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="split-pane-header" ngbDropdown>
<div class="split-pane-header-top" data-toggle="dropdown" [ngClass]="{ active: showFiles, clickable: hasFiles() }" (click)="toggleShowFiles()">
<div class="split-pane-header-top" data-toggle="dropdown" [ngClass]="{ active: showFiles, clickable: hasFiles() }" (click)="toggleShowFiles(true)">
@if (hasActiveFile()) {
<div class="split-pane-header-file-name">
<fa-icon class="file-arrow-down" [icon]="faChevronDown" />
Expand All @@ -9,9 +9,15 @@
<div>{{ studentLogin || ('artemisApp.plagiarism.unknownStudent' | artemisTranslate) }}</div>
</div>
@if (showFiles) {
<ul class="split-pane-header-files">
<ul (mouseleave)="toggleShowFiles(true, false)" class="split-pane-header-files">
@for (file of files; track file; let idx = $index) {
<li class="split-pane-header-file" ngbDropdownItem (click)="handleFileSelect(file, idx)">
<li
class="split-pane-header-file"
ngbDropdownItem
(mouseenter)="triggerMouseEnter(file, idx)"
(click)="handleFileSelect(file, idx, true)"
[ngClass]="{ hover: idx === hoveredFileIndex }"
>
<span [class.split-pane-header-file-with-match]="file.hasMatch">{{ file.file }}</span>
</li>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,7 @@
overflow: scroll;
}
}

.dropdown-item.hover {
background-color: var(--data-table-dropdown-item-selected-background-color);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Component, EventEmitter, Input, OnChanges, 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';

/**
* A file name that additionally stores if a plagiarism match has been found for it.
Expand All @@ -14,16 +16,84 @@ 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;
fileSelectedSubject = input<Subject<TextPlagiarismFileElement>>();
isLockFilesEnabled = input<boolean>();
showFilesSubject = input<Subject<boolean>>();
dropdownHoverSubject = input<Subject<TextPlagiarismFileElement>>();

@Output() selectFile = new EventEmitter<string>();

public showFiles = false;
public activeFileIndex = 0;

private fileSelectSubscription?: Subscription;
private showFilesSubscription?: Subscription;
private dropdownHoverSubscription?: Subscription;

// Icons
faChevronDown = faChevronDown;
hoveredFileIndex: 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((textPlagiarismElement) => {
if (this.isLockFilesEnabled()) {
this.handleLockedFileSelection(textPlagiarismElement.file, textPlagiarismElement.idx);
}
});
}

private handleLockedFileSelection(file: FileWithHasMatch, idx: number): void {
const index = this.files[idx]?.file === file.file ? idx : this.getIndexOf(file);

if (index >= 0) {
this.handleFileSelect(file, index, false);
} 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.toggleShowFiles(false, showFiles);
}
});
}

/**
* subscribes to listening onto mouse enter changes in dropdown in component instance
* @private helper method
*/
private subscribeToDropdownHover(): void {
this.dropdownHoverSubscription = this.dropdownHoverSubject()?.subscribe((textPlagiarismElement) => {
if (this.isLockFilesEnabled()) {
this.handleDropdownHover(textPlagiarismElement.file, textPlagiarismElement.idx);
}
});
}

private handleDropdownHover(file: FileWithHasMatch, idx: number): void {
const index = this.files[idx]?.file === file.file ? idx : this.getIndexOf(file);

this.hoveredFileIndex = index >= 0 ? index : -1;
}

ngOnChanges(changes: SimpleChanges) {
if (changes.files) {
Expand All @@ -37,6 +107,12 @@ export class SplitPaneHeaderComponent implements OnChanges {
}
}

ngOnDestroy(): void {
this.fileSelectSubscription?.unsubscribe();
this.showFilesSubscription?.unsubscribe();
this.dropdownHoverSubscription?.unsubscribe();
}

hasActiveFile(): boolean {
return this.hasFiles() && this.activeFileIndex < this.files.length;
}
Expand All @@ -45,19 +121,54 @@ export class SplitPaneHeaderComponent implements OnChanges {
return this.files[this.activeFileIndex].file;
}

handleFileSelect(file: FileWithHasMatch, idx: number): void {
/**
* 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, 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);
}

hasFiles(): boolean {
return this.files && this.files.length > 0;
return !!this.files?.length;
}

toggleShowFiles(): void {
/**
* 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.
*/
toggleShowFiles(propagateChanges: boolean, showFiles?: boolean): void {
if (this.hasFiles()) {
this.showFiles = !this.showFiles;
this.showFiles = showFiles !== undefined ? showFiles : !this.showFiles;

if (propagateChanges) {
this.showFilesSubject()!.next(this.showFiles);
}
}
}

triggerMouseEnter(file: FileWithHasMatch, idx: number) {
const subject = this.dropdownHoverSubject();
if (subject) {
subject.next({ idx, 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);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
<jhi-split-pane-header [files]="files" (selectFile)="handleFileSelect($event)" studentLogin="{{ plagiarismSubmission?.studentLogin }}" />
<jhi-split-pane-header
[files]="files"
[isLockFilesEnabled]="this.isLockFilesEnabled"
[fileSelectedSubject]="fileSelectedSubject"
(selectFile)="handleFileSelect($event)"
[showFilesSubject]="showFilesSubject"
[dropdownHoverSubject]="dropdownHoverSubject"
studentLogin="{{ plagiarismSubmission?.studentLogin }}"
/>
@if (cannotLoadFiles) {
<div class="text-submission-viewer text-warning">
<div class="text-submission-viewer-warning">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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';
import { TextPlagiarismFileElement } from 'app/exercises/shared/plagiarism/types/text/TextPlagiarismFileElement';

type FilesWithType = { [p: string]: FileType };

Expand All @@ -26,6 +28,10 @@ export class TextSubmissionViewerComponent implements OnChanges {
@Input() matches: Map<string, FromToElement[]>;
@Input() plagiarismSubmission: PlagiarismSubmission<TextSubmissionElement>;
@Input() hideContent: boolean;
@Input() fileSelectedSubject!: Subject<TextPlagiarismFileElement>;
@Input() isLockFilesEnabled: boolean;
@Input() showFilesSubject!: Subject<boolean>;
@Input() dropdownHoverSubject!: Subject<TextPlagiarismFileElement>;

/**
* Name of the currently selected file.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { FileWithHasMatch } from 'app/exercises/shared/plagiarism/plagiarism-split-view/split-pane-header/split-pane-header.component';

export interface TextPlagiarismFileElement {
idx: number;
file: FileWithHasMatch;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +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 { MockDirective } from 'ng-mocks';
import { TranslateDirective } from 'app/shared/language/translate.directive';

describe('Plagiarism Header Component', () => {
let comp: PlagiarismHeaderComponent;
Expand All @@ -22,7 +24,7 @@ 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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, FromToElement[]>());

const matches: PlagiarismMatch[] = [
{ startA: 0, startB: 0, length: 5 },
Expand Down
Loading

0 comments on commit a73231a

Please sign in to comment.