Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plagiarism checks: Improve file selection in comparison #9789

Merged
merged 47 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
346499e
add ui for toggling and translation for labels
AjayvirS Nov 14, 2024
9d7a69f
add working implementation
AjayvirS Nov 15, 2024
6dedb68
add code quality changes and comments
AjayvirS Nov 15, 2024
0d37874
fix failing test
AjayvirS Nov 15, 2024
e8d46fb
attempt fix tests
AjayvirS Nov 18, 2024
f02813f
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 18, 2024
7f268d2
attempt fix more tests
AjayvirS Nov 18, 2024
bbdb814
Merge remote-tracking branch 'origin/feature/plagiarism-checks/same-f…
AjayvirS Nov 18, 2024
efb80bd
fix last failing test
AjayvirS Nov 18, 2024
55c5a4e
add mock translate directive to fix test
AjayvirS Nov 18, 2024
f573a95
add coderabbit suggestions
AjayvirS Nov 18, 2024
8206899
add tests for feature
AjayvirS Nov 18, 2024
68b65ec
fix test
AjayvirS Nov 18, 2024
a106166
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 18, 2024
52d28fb
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 20, 2024
f0e0bfd
adapt functionality to handle cases where files are not identically n…
AjayvirS Nov 22, 2024
f5b3748
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 22, 2024
9c88a73
Merge branch 'feature/plagiarism-checks/same-file-comparison-view' of…
AjayvirS Nov 26, 2024
e6cef0a
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 26, 2024
14fbd0a
fix minor indexing and add tests for coverage
AjayvirS Nov 27, 2024
6976196
xMerge branch 'develop' into feature/plagiarism-checks/same-file-comp…
AjayvirS Nov 27, 2024
ac6c46a
adapt split pane header to new client code guidelines and adapt tests
AjayvirS Nov 28, 2024
b2272f9
fix test
AjayvirS Nov 28, 2024
613dc23
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Nov 28, 2024
24bc889
fix test
AjayvirS Nov 28, 2024
dfc8c28
fix test
AjayvirS Nov 28, 2024
16325a8
Merge remote-tracking branch 'origin/feature/plagiarism-checks/same-f…
AjayvirS Nov 28, 2024
aa7b290
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Dec 3, 2024
6c2a4c4
ramona fixes
AjayvirS Dec 3, 2024
ea194cb
ramona fixes
AjayvirS Dec 3, 2024
a0b53fa
ramona fixes
AjayvirS Dec 3, 2024
d7a636a
Merge branch 'feature/plagiarism-checks/same-file-comparison-view' of…
AjayvirS Dec 3, 2024
38910db
ramona fixes
AjayvirS Dec 3, 2024
442d81f
ramona fix tests
AjayvirS Dec 3, 2024
3f4a980
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
MarkusPaulsen Dec 10, 2024
cd3ede4
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
MarkusPaulsen Dec 10, 2024
1d6b25c
ramona changes
AjayvirS Dec 10, 2024
596e0a6
Merge remote-tracking branch 'origin/feature/plagiarism-checks/same-f…
AjayvirS Dec 10, 2024
8e9a155
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Dec 10, 2024
993b38b
ramona changes
AjayvirS Dec 10, 2024
9973ade
Merge remote-tracking branch 'origin/feature/plagiarism-checks/same-f…
AjayvirS Dec 10, 2024
8aa0c0f
remove non null assertion
AjayvirS Dec 11, 2024
f53e92a
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
sachmii Dec 11, 2024
38b0505
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Dec 11, 2024
5a95c27
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
AjayvirS Dec 17, 2024
e7a7a75
add auto close dropdown on mouseleave for better usability
AjayvirS Dec 17, 2024
a85d119
Merge branch 'develop' into feature/plagiarism-checks/same-file-compa…
Strohgelaender Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>();
AjayvirS marked this conversation as resolved.
Show resolved Hide resolved
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';
AjayvirS marked this conversation as resolved.
Show resolved Hide resolved
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()) {
AjayvirS marked this conversation as resolved.
Show resolved Hide resolved
this.handleLockedFileSelection(textPlagiarismElement.file, textPlagiarismElement.idx);
}
});
}

AjayvirS marked this conversation as resolved.
Show resolved Hide resolved
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>;
AjayvirS marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
Loading