Skip to content

Commit

Permalink
Merge branch 'develop' into bugfix/communication/show-only-accepted-c…
Browse files Browse the repository at this point in the history
…ategories
  • Loading branch information
cremertim authored Oct 26, 2024
2 parents 4bfbc7e + 6e1c562 commit e1ec400
Show file tree
Hide file tree
Showing 36 changed files with 490 additions and 305 deletions.
6 changes: 3 additions & 3 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ module.exports = {
coverageThreshold: {
global: {
// TODO: in the future, the following values should increase to at least 90%
statements: 87.52,
branches: 73.63,
functions: 82.15,
statements: 87.53,
branches: 73.62,
functions: 82.13,
lines: 87.58,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ public final class Constants {

public static final long MAX_NUMBER_OF_LOCKED_SUBMISSIONS_PER_TUTOR = 10;

// Note: The values in input.constants.ts (client) need to be the same
public static final long MAX_FILE_SIZE_COMMUNICATION = 5 * 1024 * 1024; // 5 MB

// Note: The values in input.constants.ts (client) need to be the same
public static final long MAX_SUBMISSION_FILE_SIZE = 8 * 1024 * 1024; // 8 MB

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public static Path getMarkdownFilePath() {
return Path.of(fileUploadPath, "markdown");
}

public static Path getMarkdownFilePathForConversation(long courseId, long conversationId) {
return getMarkdownFilePath().resolve("communication").resolve(String.valueOf(courseId)).resolve(String.valueOf(conversationId));
}

/**
* Convert the given public file url to its corresponding local path
*
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/de/tum/cit/aet/artemis/core/service/FileService.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,32 @@ public URI handleSaveFile(MultipartFile file, boolean keepFilename, boolean mark
return URI.create(markdown ? MARKDOWN_FILE_SUBPATH : DEFAULT_FILE_SUBPATH).resolve(currentFilename);
}

/**
* Handles the saving of a file in a conversation.
*
* @param file The file to be uploaded.
* @param courseId The ID of the course.
* @param conversationId The ID of the conversation.
* @return The URI of the saved file.
*/
public URI handleSaveFileInConversation(MultipartFile file, Long courseId, Long conversationId) {
// TODO: Improve the access check. The course is already checked, but the user might not be a member of the conversation. The course may not belong to the conversation
String filename = checkAndSanitizeFilename(file.getOriginalFilename());

validateExtension(filename, true);

final String filenamePrefix = "Markdown_";
final Path path = FilePathService.getMarkdownFilePathForConversation(courseId, conversationId);

String fileName = generateFilename(filenamePrefix, filename, false); // TODO: keep?
Path filePath = path.resolve(fileName);

copyFile(file, filePath);

String currentFilename = filePath.getFileName().toString();
return URI.create("/api/files/courses/" + courseId + "/conversations/" + conversationId + "/").resolve(currentFilename);
}

/**
* Saves a file to the given path using a generated filename.
*
Expand Down
45 changes: 45 additions & 0 deletions src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.server.ResponseStatusException;

import de.tum.cit.aet.artemis.core.config.Constants;
import de.tum.cit.aet.artemis.core.domain.Course;
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.core.exception.AccessForbiddenException;
Expand All @@ -52,6 +54,7 @@
import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastStudent;
import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastTutor;
import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastEditorInCourse;
import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastStudentInCourse;
import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService;
import de.tum.cit.aet.artemis.core.service.FilePathService;
import de.tum.cit.aet.artemis.core.service.FileService;
Expand Down Expand Up @@ -163,6 +166,48 @@ public ResponseEntity<String> saveMarkdownFile(@RequestParam(value = "file") Mul
return ResponseEntity.created(new URI(responsePath)).body(responseBody);
}

/**
* POST /files/courses/{courseId}/conversations/{conversationId} : Upload a new file for use in a conversation.
*
* @param file The file to save. The size must not exceed Constants.MAX_FILE_SIZE_COMMUNICATION.
* @param courseId The ID of the course the conversation belongs to.
* @param conversationId The ID of the conversation the file is used in.
* @return The path of the file.
* @throws URISyntaxException If the response path can't be converted into a URI.
*/
@PostMapping("files/courses/{courseId}/conversations/{conversationId}")
@EnforceAtLeastStudentInCourse
public ResponseEntity<String> saveMarkdownFileForConversation(@RequestParam(value = "file") MultipartFile file, @PathVariable Long courseId, @PathVariable Long conversationId)
throws URISyntaxException {
log.debug("REST request to upload file for markdown in conversation: {} for conversation {} in course {}", file.getOriginalFilename(), conversationId, courseId);
if (file.getSize() > Constants.MAX_FILE_SIZE_COMMUNICATION) {
throw new ResponseStatusException(HttpStatus.PAYLOAD_TOO_LARGE, "The file is too large. Maximum file size is " + Constants.MAX_FILE_SIZE_COMMUNICATION + " bytes.");
}
String responsePath = fileService.handleSaveFileInConversation(file, courseId, conversationId).toString();

// return path for getting the file
String responseBody = "{\"path\":\"" + responsePath + "\"}";

return ResponseEntity.created(new URI(responsePath)).body(responseBody);
}

/**
* GET /files/courses/{courseId}/conversations/{conversationId}/{filename} : Get the markdown file with the given filename for the given conversation.
*
* @param courseId The ID of the course the conversation belongs to.
* @param conversationId The ID of the conversation the file is used in.
* @param filename The filename of the file to get.
* @return The requested file, or 404 if the file doesn't exist. The response will enable caching.
*/
@GetMapping("files/courses/{courseId}/conversations/{conversationId}/{filename}")
@EnforceAtLeastStudentInCourse
public ResponseEntity<byte[]> getMarkdownFileForConversation(@PathVariable Long courseId, @PathVariable Long conversationId, @PathVariable String filename) {
// TODO: Improve the access check
log.debug("REST request to get file for markdown in conversation: File {} for conversation {} in course {}", filename, conversationId, courseId);
sanitizeFilenameElseThrow(filename);
return buildFileResponse(FilePathService.getMarkdownFilePathForConversation(courseId, conversationId), filename, true);
}

/**
* GET /files/markdown/:filename : Get the markdown file with the given filename
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,19 @@
<div class="row justify-content-start align-items-center">
@if (predecessorLearningObject(); as predecessorLearningObject) {
<div class="col-md-auto">
<button id="previous-button" (click)="selectLearningObject(predecessorLearningObject)" type="button" class="btn btn-secondary">
<button
id="previous-button"
(click)="selectLearningObject(predecessorLearningObject, false)"
type="button"
class="btn btn-secondary d-flex justify-content-center"
>
<div class="me-1 loading-icon-container">
@if (isLoadingPredecessor()) {
<fa-icon [icon]="faSpinner" animation="spin" />
} @else {
<fa-icon [icon]="faChevronLeft" />
}
</div>
<span jhiTranslate="artemisApp.learningPath.navigation.previousButton"></span>
</button>
</div>
Expand All @@ -13,18 +25,16 @@
</div>
</div>
<div ngbDropdown class="col-4 dropdown" (openChange)="setIsDropdownOpen($event)" #navOverview="ngbDropdown">
@if (!isLoading()) {
<div type="button" class="row justify-content-center align-items-center h-100" id="navigation-overview" ngbDropdownToggle>
@if (currentLearningObject()) {
<span class="col-md-auto fw-bold">
{{ currentLearningObject()?.name }}
</span>
} @else {
<span class="col-md-auto fw-bold" jhiTranslate="artemisApp.learningPath.navigation.recapLabel"></span>
}
<fa-icon [icon]="faChevronDown" class="col-md-auto ps-0" />
</div>
}
<div type="button" class="row justify-content-center align-items-center h-100" id="navigation-overview" ngbDropdownToggle>
@if (currentLearningObject()) {
<span class="col-md-auto fw-bold">
{{ currentLearningObject()?.name }}
</span>
} @else {
<span class="col-md-auto fw-bold" jhiTranslate="artemisApp.learningPath.navigation.recapLabel"></span>
}
<fa-icon [icon]="faChevronDown" class="col-md-auto ps-0" />
</div>
<div ngbDropdownMenu class="mt-3 col p-0" aria-labelledby="navigation-overview">
@if (isDropdownOpen()) {
<jhi-learning-path-nav-overview (onLearningObjectSelected)="navOverview.close()" [learningPathId]="learningPathId()" />
Expand All @@ -36,8 +46,15 @@
@if (successorLearningObject(); as successorLearningObject) {
<span class="col text-end pe-3 text-truncate text-secondary">{{ successorLearningObject.name }}</span>
<div class="col-md-auto">
<button id="next-button" (click)="selectLearningObject(successorLearningObject)" type="button" class="btn btn-primary">
<button id="next-button" (click)="selectLearningObject(successorLearningObject, true)" type="button" class="btn btn-primary d-flex justify-content-center">
<span jhiTranslate="artemisApp.learningPath.navigation.nextButton"></span>
<div class="ms-1 loading-icon-container">
@if (isLoadingSuccessor()) {
<fa-icon [icon]="faSpinner" animation="spin" />
} @else {
<fa-icon [icon]="faChevronRight" />
}
</div>
</button>
</div>
} @else if (currentLearningObject() && !successorLearningObject()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@
width: 500px;
box-shadow: 0 8px 12px 0 var(--lecture-unit-card-shadow);
}

.loading-icon-container {
width: 15px;
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Component, InputSignal, Signal, WritableSignal, computed, effect, inject, input, signal } from '@angular/core';
import { Component, computed, effect, inject, input, signal, untracked } from '@angular/core';
import { LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model';
import { CommonModule } from '@angular/common';
import { NgbAccordionModule, NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap';
import { FontAwesomeModule } from '@fortawesome/angular-fontawesome';
import { IconDefinition, faCheckCircle, faChevronDown, faFlag } from '@fortawesome/free-solid-svg-icons';
import { faCheckCircle, faChevronDown, faChevronLeft, faChevronRight, faFlag, faSpinner } from '@fortawesome/free-solid-svg-icons';
import { LearningPathNavOverviewComponent } from 'app/course/learning-paths/components/learning-path-nav-overview/learning-path-nav-overview.component';
import { ArtemisSharedModule } from 'app/shared/shared.module';
import { LearningPathNavigationService } from 'app/course/learning-paths/services/learning-path-navigation.service';
Expand All @@ -16,33 +16,44 @@ import { LearningPathNavigationService } from 'app/course/learning-paths/service
styleUrl: './learning-path-student-nav.component.scss',
})
export class LearningPathNavComponent {
protected readonly faChevronDown: IconDefinition = faChevronDown;
protected readonly faCheckCircle: IconDefinition = faCheckCircle;
protected readonly faFlag: IconDefinition = faFlag;
protected readonly faChevronDown = faChevronDown;
protected readonly faCheckCircle = faCheckCircle;
protected readonly faFlag = faFlag;
protected readonly faSpinner = faSpinner;
protected readonly faChevronLeft = faChevronLeft;
protected readonly faChevronRight = faChevronRight;

private learningPathNavigationService: LearningPathNavigationService = inject(LearningPathNavigationService);
private learningPathNavigationService = inject(LearningPathNavigationService);

readonly learningPathId: InputSignal<number> = input.required<number>();
readonly learningPathId = input.required<number>();

readonly isLoading: WritableSignal<boolean> = this.learningPathNavigationService.isLoading;
readonly isLoading = this.learningPathNavigationService.isLoading;
readonly isLoadingPredecessor = signal<boolean>(false);
readonly isLoadingSuccessor = signal<boolean>(false);

readonly learningPathProgress: Signal<number> = computed(() => this.learningPathNavigationService.learningPathNavigation()?.progress ?? 0);
readonly predecessorLearningObject: Signal<LearningPathNavigationObjectDTO | undefined> = computed(
() => this.learningPathNavigationService.learningPathNavigation()?.predecessorLearningObject,
);
readonly currentLearningObject: Signal<LearningPathNavigationObjectDTO | undefined> = computed(() => this.learningPathNavigationService.currentLearningObject());
readonly successorLearningObject: Signal<LearningPathNavigationObjectDTO | undefined> = computed(
() => this.learningPathNavigationService.learningPathNavigation()?.successorLearningObject,
);
private readonly learningPathNavigation = this.learningPathNavigationService.learningPathNavigation;
readonly learningPathProgress = computed(() => this.learningPathNavigation()?.progress ?? 0);
readonly predecessorLearningObject = computed(() => this.learningPathNavigation()?.predecessorLearningObject);
readonly currentLearningObject = computed(() => this.learningPathNavigation()?.currentLearningObject);
readonly successorLearningObject = computed(() => this.learningPathNavigation()?.successorLearningObject);

readonly isDropdownOpen: WritableSignal<boolean> = signal(false);
readonly isDropdownOpen = signal<boolean>(false);

constructor() {
effect(async () => await this.learningPathNavigationService.loadLearningPathNavigation(this.learningPathId()), { allowSignalWrites: true });
effect(
() => {
const learningPathId = this.learningPathId();
untracked(() => this.learningPathNavigationService.loadLearningPathNavigation(learningPathId));
},
{ allowSignalWrites: true },
);
}

async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO): Promise<void> {
async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO, isSuccessor: boolean): Promise<void> {
const loadingSpinner = isSuccessor ? this.isLoadingSuccessor : this.isLoadingPredecessor;
loadingSpinner.set(true);
await this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), selectedLearningObject);
loadingSpinner.set(false);
}

completeLearningPath(): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { Injectable, Signal, WritableSignal, computed, inject, signal } from '@angular/core';
import { Injectable, computed, inject, signal } from '@angular/core';
import { LearningPathNavigationDTO, LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model';
import { AlertService } from 'app/core/util/alert.service';
import { LearningPathApiService } from 'app/course/learning-paths/services/learning-path-api.service';

@Injectable({ providedIn: 'root' })
export class LearningPathNavigationService {
private readonly learningPathApiService: LearningPathApiService = inject(LearningPathApiService);
private readonly alertService: AlertService = inject(AlertService);
private readonly learningPathApiService = inject(LearningPathApiService);
private readonly alertService = inject(AlertService);

readonly isLoading: WritableSignal<boolean> = signal(false);
readonly isLoading = signal<boolean>(false);

readonly learningPathNavigation: WritableSignal<LearningPathNavigationDTO | undefined> = signal(undefined);
readonly currentLearningObject: Signal<LearningPathNavigationObjectDTO | undefined> = computed(() => this.learningPathNavigation()?.currentLearningObject);
readonly learningPathNavigation = signal<LearningPathNavigationDTO | undefined>(undefined);
readonly currentLearningObject = computed(() => this.learningPathNavigation()?.currentLearningObject);

readonly isCurrentLearningObjectCompleted: WritableSignal<boolean> = signal(false);
readonly isCurrentLearningObjectCompleted = signal<boolean>(false);

async loadLearningPathNavigation(learningPathId: number): Promise<void> {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ <h5 class="mb-0 fw-medium" jhiTranslate="artemisApp.conversationsLayout.threadSi
<div class="message-input mx-3">
@if (!readOnlyMode) {
<jhi-message-reply-inline-input
[activeConversation]="conversation"
[posting]="createdAnswerPost"
(onCreate)="createdAnswerPost = createEmptyAnswerPost()"
(valueChange)="scrollEditorIntoView()"
Expand Down
2 changes: 2 additions & 0 deletions src/main/webapp/app/shared/constants/input.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export const DATE_TIME_FORMAT = 'YYYY-MM-DDTHH:mm';

/** Maximum file size: 20 MB (Spring-Boot interprets MB as 1024^2 bytes) **/
export const MAX_FILE_SIZE = 20 * 1024 * 1024;
/** Maximum file size for communication: 5 MB **/
export const MAX_FILE_SIZE_COMMUNICATION = 5 * 1024 * 1024;
/** Maximum submission file size: 4 MB **/
export const MAX_SUBMISSION_FILE_SIZE = 8 * 1024 * 1024;
/** Maximum text exercise submission character length: 30.000 **/
Expand Down
Loading

0 comments on commit e1ec400

Please sign in to comment.