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

Programming exercises: Remove the legacy README.md handling #9220

Merged
merged 7 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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 @@ -515,8 +515,6 @@ export class CodeEditorFileBrowserComponent implements OnInit, OnChanges, AfterV
fromPairs(
toPairs(files)
.filter(([fileName, fileType]) => this.allowHiddenFiles || CodeEditorFileBrowserComponent.shouldDisplayFile(fileName, fileType))
// Filter Readme file that was historically in the student's assignment repo
.filter(([value]) => !value.includes('README.md'))
// Filter root folder
.filter(([value]) => value),
),
Expand All @@ -527,14 +525,7 @@ export class CodeEditorFileBrowserComponent implements OnInit, OnChanges, AfterV

loadFilesWithInformationAboutChange(): Observable<{ [fileName: string]: boolean }> {
return this.repositoryFileService.getFilesWithInformationAboutChange().pipe(
rxMap((files) =>
fromPairs(
toPairs(files)
.filter(([filename]) => this.allowHiddenFiles || CodeEditorFileBrowserComponent.shouldDisplayFile(filename, FileType.FILE))
// Filter Readme file that was historically in the student's assignment repo
.filter(([value]) => !value.includes('README.md')),
),
),
rxMap((files) => fromPairs(toPairs(files).filter(([filename]) => this.allowHiddenFiles || CodeEditorFileBrowserComponent.shouldDisplayFile(filename, FileType.FILE)))),
catchError(() => throwError(() => new Error('couldNotBeRetrieved'))),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { TaskArray } from 'app/exercises/programming/shared/instructions-render/
import { Participation } from 'app/entities/participation/participation.model';
import { Feedback } from 'app/entities/feedback.model';
import { ResultService } from 'app/exercises/shared/result/result.service';
import { RepositoryFileService } from 'app/exercises/shared/result/repository.service';
import { problemStatementHasChanged } from 'app/exercises/shared/exercise/exercise.utils';
import { ProgrammingExerciseParticipationService } from 'app/exercises/programming/manage/services/programming-exercise-participation.service';
import { Result } from 'app/entities/result.model';
Expand All @@ -51,7 +50,7 @@ export class ProgrammingExerciseInstructionComponent implements OnChanges, OnDes
@Input() public participation: Participation;
@Input() generateHtmlEvents: Observable<void>;
@Input() personalParticipation: boolean;
// If there are no instructions available (neither in the exercise problemStatement nor the legacy README.md) emits an event
// Emits an event if the instructions are not available via the problemStatement
@Output()
public onNoInstructionsAvailable = new EventEmitter();

Expand Down Expand Up @@ -94,7 +93,6 @@ export class ProgrammingExerciseInstructionComponent implements OnChanges, OnDes
constructor(
public viewContainerRef: ViewContainerRef,
private resultService: ResultService,
private repositoryFileService: RepositoryFileService,
private participationWebsocketService: ParticipationWebsocketService,
private programmingExerciseTaskWrapper: ProgrammingExerciseTaskExtensionWrapper,
private programmingExercisePlantUmlWrapper: ProgrammingExercisePlantUmlExtensionWrapper,
Expand Down Expand Up @@ -155,7 +153,7 @@ export class ProgrammingExerciseInstructionComponent implements OnChanges, OnDes
// If the exercise is not loaded, the instructions can't be loaded and so there is no point in loading the results, etc, yet.
if (!this.isLoading && this.exercise && this.participation && (this.isInitial || participationHasChanged)) {
this.isLoading = true;
return this.loadInstructions().pipe(
return of(this.exercise.problemStatement).pipe(
// If no instructions can be loaded, abort pipe and hide the instruction panel
tap((problemStatement) => {
if (!problemStatement) {
Expand Down Expand Up @@ -291,26 +289,6 @@ export class ProgrammingExerciseInstructionComponent implements OnChanges, OnDes
);
}

/**
* Loads the instructions for the programming exercise.
* We added the problemStatement later, historically the instructions where a file in the student's repository
* This is why we now prefer the problemStatement and if it doesn't exist try to load the readme.
*/
loadInstructions(): Observable<string | undefined> {
if (this.exercise.problemStatement) {
return of(this.exercise.problemStatement);
} else {
if (!this.participation.id) {
return of(undefined);
}
return this.repositoryFileService.get(this.participation.id, 'README.md').pipe(
catchError(() => of(undefined)),
// Old readme files contain chars instead of our domain command tags - replace them when loading the file
map((fileObj) => fileObj && fileObj.fileContent.replace(new RegExp(/✅/, 'g'), '[task]')),
);
}
}

private renderMarkdown(): void {
// Highlight differences between previous and current markdown
if (
Expand Down
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { MockCodeEditorConflictStateService } from '../../helpers/mocks/service/
import { TranslatePipeMock } from '../../helpers/mocks/service/mock-translate.service';
import { TreeviewModule } from 'app/exercises/programming/shared/code-editor/treeview/treeview.module';
import { TreeviewItem } from 'app/exercises/programming/shared/code-editor/treeview/models/treeview-item';
import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap';

describe('CodeEditorFileBrowserComponent', () => {
let comp: CodeEditorFileBrowserComponent;
Expand Down Expand Up @@ -149,6 +150,25 @@ describe('CodeEditorFileBrowserComponent', () => {
expect(renderedFiles).toHaveLength(3);
});

it('should toggle tree compression', () => {
comp.repositoryFiles = {
file1: FileType.FILE,
};
const treeNode = {
folder: '',
file: 'file1',
children: [],
text: 'file1',
value: 'file1',
};
jest.spyOn(comp, 'buildTree').mockReturnValue([treeNode]);
const transformTreeToTreeViewItemStub = jest.spyOn(comp, 'transformTreeToTreeViewItem').mockReturnValue([new TreeviewItem(treeNode)]);
comp.compressFolders = false;
comp.toggleTreeCompress();
expect(comp.compressFolders).toBeTrue();
expect(transformTreeToTreeViewItemStub).toHaveBeenCalledExactlyOnceWith([treeNode]);
});

it('should create compressed treeviewItems with nested folder structure', () => {
comp.repositoryFiles = {
folder: FileType.FOLDER,
Expand Down Expand Up @@ -205,7 +225,6 @@ describe('CodeEditorFileBrowserComponent', () => {
};
const forbiddenFiles = {
'danger.bin': FileType.FILE,
'README.md': FileType.FILE,
'.hidden': FileType.FILE,
'.': FileType.FOLDER,
};
Expand Down Expand Up @@ -310,6 +329,42 @@ describe('CodeEditorFileBrowserComponent', () => {
expect(renderedFiles).toHaveLength(0);
});

it('should select the correct file based on the user selection', () => {
const fileToSelect = 'folder/file1';
const otherFile = 'folder2/file2';
comp.repositoryFiles = {
folder: FileType.FOLDER,
'folder/file1': FileType.FILE,
folder2: FileType.FOLDER,
'folder/file2': FileType.FILE,
};
comp.filesTreeViewItem = [
new TreeviewItem({
checked: false,
text: fileToSelect,
value: fileToSelect,
children: [],
} as any),
new TreeviewItem({
checked: false,
text: otherFile,
value: otherFile,
children: [],
}),
];
comp.selectedFile = undefined;
const nodeFirstFile = comp.filesTreeViewItem[0];
comp.handleNodeSelected(nodeFirstFile);
expect(nodeFirstFile.checked).toBeTrue();
expect(comp.selectedFile).toBe(fileToSelect);
// Deselect the current file.
const nodeSecondFile = comp.filesTreeViewItem[1];
comp.handleNodeSelected(nodeSecondFile);
expect(nodeFirstFile.checked).toBeFalse();
expect(nodeSecondFile.checked).toBeTrue();
expect(comp.selectedFile).toBe(otherFile);
});

it('should set node to checked if its file gets selected and update ui', () => {
const selectedFile = 'folder/file1';
const repositoryFiles = {
Expand Down Expand Up @@ -505,6 +560,26 @@ describe('CodeEditorFileBrowserComponent', () => {
expect(createFileStub).not.toHaveBeenCalled();
});

it('should manage the root file/folder it is currently creating', () => {
comp.setCreatingFileInRoot(FileType.FILE);
expect(comp.creatingFile).toEqual(['', FileType.FILE]);
comp.setCreatingFileInRoot(FileType.FOLDER);
expect(comp.creatingFile).toEqual(['', FileType.FOLDER]);
comp.clearCreatingFile();
expect(comp.creatingFile).toBeUndefined();
});

it('should manage the file/folder it is currently creating within another folder', () => {
const folder = 'folder';
const item = { value: folder } as TreeviewItem<string>;
comp.setCreatingFile({ item, fileType: FileType.FILE });
expect(comp.creatingFile).toEqual([folder, FileType.FILE]);
comp.setCreatingFile({ item, fileType: FileType.FOLDER });
expect(comp.creatingFile).toEqual([folder, FileType.FOLDER]);
comp.clearCreatingFile();
expect(comp.creatingFile).toBeUndefined();
});

it('should update repository file entry on rename', fakeAsync(() => {
const fileName = 'file1';
const afterRename = 'newFileName';
Expand Down Expand Up @@ -761,6 +836,18 @@ describe('CodeEditorFileBrowserComponent', () => {
expect(renamingInput).toBeNull();
}));

it('should manage the file it is currently renaming', () => {
comp.repositoryFiles = {
'folder/file1': FileType.FILE,
folder: FileType.FOLDER,
};
const item = { value: 'folder/file1', text: 'file1' } as TreeviewItem<string>;
comp.setRenamingFile(item);
expect(comp.renamingFile).toEqual(['folder/file1', 'file1', FileType.FILE]);
comp.clearRenamingFile();
expect(comp.renamingFile).toBeUndefined();
});

it('should disable action buttons if there is a git conflict', () => {
const repositoryContent: { [fileName: string]: string } = {};
getStatusStub.mockReturnValue(of({ repositoryStatus: CommitState.CONFLICT }));
Expand Down Expand Up @@ -806,6 +893,7 @@ describe('CodeEditorFileBrowserComponent', () => {
const filteredChangeInformation = {
'Class.java': true,
'Document.md': false,
'README.md': true,
};
const getFilesWithChangeInfoStub = jest.fn().mockReturnValue(of(changeInformation));
codeEditorRepositoryFileService.getFilesWithInformationAboutChange = getFilesWithChangeInfoStub;
Expand All @@ -821,6 +909,21 @@ describe('CodeEditorFileBrowserComponent', () => {
loadFiles.unsubscribe();
}));

it('should open a modal when trying to delete a file', () => {
comp.repositoryFiles = {
'folder/file1': FileType.FILE,
folder: FileType.FOLDER,
};
const item = { value: 'folder/file1', text: 'file1' } as TreeviewItem<string>;
const modalRef = { componentInstance: { parent: undefined, fileNameToDelete: undefined, fileType: undefined } } as NgbModalRef;
const openModalStub = jest.spyOn(comp.modalService, 'open').mockReturnValue(modalRef);
comp.openDeleteFileModal(item);
expect(openModalStub).toHaveBeenCalledOnce();
expect(modalRef.componentInstance.parent).toBe(comp);
expect(modalRef.componentInstance.fileNameToDelete).toBe('folder/file1');
expect(modalRef.componentInstance.fileType).toBe(FileType.FILE);
});

describe('getFolderBadges', () => {
// Mock fileBadges data
const mockFileBadges = {
Expand Down
Loading
Loading