Skip to content

Commit

Permalink
Programming exercises: Remove the legacy README.md handling (#9220)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pzdr7
pzdr7 authored Aug 21, 2024
1 parent fc5e849 commit b75396d
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 132 deletions.
11 changes: 1 addition & 10 deletions ...ercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts
View file
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
26 changes: 2 additions & 24 deletions ...ises/programming/shared/instructions-render/programming-exercise-instruction.component.ts
View file
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
105 changes: 104 additions & 1 deletion src/test/javascript/spec/component/code-editor/code-editor-file-browser.component.spec.ts
View file
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

0 comments on commit b75396d

Please sign in to comment.