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

Development: Merge exercise details calls into one DTO #8712

Merged
merged 20 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2cb7570
move additional calls in wrapper DTO
julian-christl Jun 4, 2024
736ad49
revert public
julian-christl Jun 4, 2024
61bdc63
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 4, 2024
9aacda5
fix tests
julian-christl Jun 4, 2024
7940908
fix client tests
julian-christl Jun 4, 2024
09ce85b
fix client test
julian-christl Jun 4, 2024
e84d768
fix more client tests
julian-christl Jun 4, 2024
8254f7d
Update src/test/javascript/spec/service/exercise.service.spec.ts
julian-christl Jun 5, 2024
6d151d4
Update src/main/webapp/app/exercises/shared/exercise/exercise.service.ts
julian-christl Jun 5, 2024
d59beb0
improve some code issues
julian-christl Jun 5, 2024
af14880
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 5, 2024
37b66fe
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 5, 2024
0c389dc
cr Johannes
julian-christl Jun 7, 2024
5a51140
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 7, 2024
b285fee
add new test
julian-christl Jun 7, 2024
a058d75
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 10, 2024
b375ae9
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 17, 2024
cee1649
Update src/main/java/de/tum/in/www1/artemis/service/plagiarism/Plagia…
julian-christl Jun 17, 2024
933c561
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
julian-christl Jun 19, 2024
5c7ed2b
Merge branch 'develop' into chore/exercises/optimize-exercise-details…
krusche Jun 21, 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 @@ -347,6 +347,7 @@ AND EXISTS (
FROM Exercise e
LEFT JOIN FETCH e.posts
LEFT JOIN FETCH e.categories
LEFT JOIN FETCH e.submissionPolicy
WHERE e.id = :exerciseId
""")
Optional<Exercise> findByIdWithDetailsForStudent(@Param("exerciseId") Long exerciseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import de.tum.in.www1.artemis.domain.Course;
import de.tum.in.www1.artemis.domain.Exercise;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.iris.IrisTemplate;
import de.tum.in.www1.artemis.domain.iris.settings.IrisChatSubSettings;
import de.tum.in.www1.artemis.domain.iris.settings.IrisCompetencyGenerationSubSettings;
Expand All @@ -27,6 +28,7 @@
import de.tum.in.www1.artemis.domain.iris.settings.IrisSubSettings;
import de.tum.in.www1.artemis.domain.iris.settings.IrisSubSettingsType;
import de.tum.in.www1.artemis.repository.iris.IrisSettingsRepository;
import de.tum.in.www1.artemis.service.AuthorizationCheckService;
import de.tum.in.www1.artemis.service.iris.IrisDefaultTemplateService;
import de.tum.in.www1.artemis.service.iris.dto.IrisCombinedSettingsDTO;
import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenAlertException;
Expand All @@ -50,11 +52,14 @@ public class IrisSettingsService {

private final IrisDefaultTemplateService irisDefaultTemplateService;

public IrisSettingsService(IrisSettingsRepository irisSettingsRepository, IrisSubSettingsService irisSubSettingsService,
IrisDefaultTemplateService irisDefaultTemplateService) {
private final AuthorizationCheckService authCheckService;

public IrisSettingsService(IrisSettingsRepository irisSettingsRepository, IrisSubSettingsService irisSubSettingsService, IrisDefaultTemplateService irisDefaultTemplateService,
AuthorizationCheckService authCheckService) {
this.irisSettingsRepository = irisSettingsRepository;
this.irisSubSettingsService = irisSubSettingsService;
this.irisDefaultTemplateService = irisDefaultTemplateService;
this.authCheckService = authCheckService;
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
}

private Optional<Integer> loadGlobalTemplateVersion() {
Expand Down Expand Up @@ -401,6 +406,17 @@ public IrisCombinedSettingsDTO getCombinedIrisSettingsFor(Exercise exercise, boo
irisSubSettingsService.combineCompetencyGenerationSettings(settingsList, minimal));
}

/**
* Check if we have to show minimal settings for an exercise. Editors can see the full settings, students only the reduced settings.
*
* @param exercise The exercise to check
* @param user The user to check
* @return Whether we have to show the user the minimal settings
*/
public boolean showMinimalSettings(Exercise exercise, User user) {
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
return !authCheckService.isAtLeastEditorForExercise(exercise, user);
}

/**
* Get the default Iris settings for a course.
* The default settings are used if no Iris settings for the course exist.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;
Expand All @@ -22,6 +23,7 @@
import de.tum.in.www1.artemis.repository.plagiarism.PlagiarismComparisonRepository;
import de.tum.in.www1.artemis.repository.plagiarism.PlagiarismSubmissionRepository;
import de.tum.in.www1.artemis.service.notifications.SingleUserNotificationService;
import de.tum.in.www1.artemis.web.rest.dto.plagiarism.PlagiarismCaseInfoDTO;
import de.tum.in.www1.artemis.web.rest.dto.plagiarism.PlagiarismVerdictDTO;

@Profile(PROFILE_CORE)
Expand Down Expand Up @@ -115,6 +117,27 @@ public void createOrAddToPlagiarismCasesForComparison(long plagiarismComparisonI
createOrAddToPlagiarismCaseForStudent(plagiarismComparison, plagiarismComparison.getSubmissionB(), false);
}

/**
* Get the plagiarism case for a student and exercise.
*
* @param exerciseId the ID of the exercise
* @param userId the ID of the student
* @return the plagiarism case for the student and exercise if it exists
*/
public Optional<PlagiarismCaseInfoDTO> getPlagiarismCaseInfoForExerciseAndUser(long exerciseId, long userId) {
return plagiarismCaseRepository.findByStudentIdAndExerciseIdWithPost(userId, exerciseId).map((plagiarismCase) -> {
// the student was notified if the plagiarism case is available (due to the nature of the query above)
// the following line is already checked in the SQL statement, but we want to ensure it 100%
if (plagiarismCase.getPost() != null) {
// Note: we only return the ID and verdict to tell the client there is a confirmed plagiarism case with student notification (post) and to support navigating to the
// detail page
// all other information might be irrelevant or sensitive and could lead to longer loading times
return new PlagiarismCaseInfoDTO(plagiarismCase.getId(), plagiarismCase.getVerdict(), plagiarismCase.isCreatedByContinuousPlagiarismControl());
}
return null;
});
}
julian-christl marked this conversation as resolved.
Show resolved Hide resolved

/**
* Create or add to a plagiarism case for a student defined via the submission involved in a plagiarism comparison.
* The following logic applies:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -29,10 +30,10 @@
import de.tum.in.www1.artemis.domain.enumeration.AssessmentType;
import de.tum.in.www1.artemis.domain.enumeration.TutorParticipationStatus;
import de.tum.in.www1.artemis.domain.exam.Exam;
import de.tum.in.www1.artemis.domain.hestia.ExerciseHint;
import de.tum.in.www1.artemis.domain.participation.StudentParticipation;
import de.tum.in.www1.artemis.domain.participation.TutorParticipation;
import de.tum.in.www1.artemis.domain.quiz.QuizExercise;
import de.tum.in.www1.artemis.domain.submissionpolicy.SubmissionPolicy;
import de.tum.in.www1.artemis.repository.ExampleSubmissionRepository;
import de.tum.in.www1.artemis.repository.ExerciseRepository;
import de.tum.in.www1.artemis.repository.GradingCriterionRepository;
Expand All @@ -50,8 +51,14 @@
import de.tum.in.www1.artemis.service.TutorParticipationService;
import de.tum.in.www1.artemis.service.exam.ExamAccessService;
import de.tum.in.www1.artemis.service.exam.ExamDateService;
import de.tum.in.www1.artemis.service.hestia.ExerciseHintService;
import de.tum.in.www1.artemis.service.iris.dto.IrisCombinedSettingsDTO;
import de.tum.in.www1.artemis.service.iris.settings.IrisSettingsService;
import de.tum.in.www1.artemis.service.plagiarism.PlagiarismCaseService;
import de.tum.in.www1.artemis.service.quiz.QuizBatchService;
import de.tum.in.www1.artemis.web.rest.dto.ExerciseDetailsDTO;
import de.tum.in.www1.artemis.web.rest.dto.StatsForDashboardDTO;
import de.tum.in.www1.artemis.web.rest.dto.plagiarism.PlagiarismCaseInfoDTO;
import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenException;
import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException;

Expand Down Expand Up @@ -93,11 +100,18 @@ public class ExerciseResource {

private final ExamAccessService examAccessService;

private final Optional<IrisSettingsService> irisSettingsService;

private final PlagiarismCaseService plagiarismCaseService;

private final ExerciseHintService exerciseHintService;

julian-christl marked this conversation as resolved.
Show resolved Hide resolved
public ExerciseResource(ExerciseService exerciseService, ExerciseDeletionService exerciseDeletionService, ParticipationService participationService,
UserRepository userRepository, ExamDateService examDateService, AuthorizationCheckService authCheckService, TutorParticipationService tutorParticipationService,
ExampleSubmissionRepository exampleSubmissionRepository, ProgrammingExerciseRepository programmingExerciseRepository,
GradingCriterionRepository gradingCriterionRepository, ExerciseRepository exerciseRepository, QuizBatchService quizBatchService,
ParticipationRepository participationRepository, ExamAccessService examAccessService) {
ParticipationRepository participationRepository, ExamAccessService examAccessService, Optional<IrisSettingsService> irisSettingsService,
PlagiarismCaseService plagiarismCaseService, ExerciseHintService exerciseHintService) {
this.exerciseService = exerciseService;
this.exerciseDeletionService = exerciseDeletionService;
this.participationService = participationService;
Expand All @@ -112,6 +126,9 @@ public ExerciseResource(ExerciseService exerciseService, ExerciseDeletionService
this.quizBatchService = quizBatchService;
this.participationRepository = participationRepository;
this.examAccessService = examAccessService;
this.irisSettingsService = irisSettingsService;
this.plagiarismCaseService = plagiarismCaseService;
this.exerciseHintService = exerciseHintService;
}

/**
Expand Down Expand Up @@ -289,7 +306,7 @@ public ResponseEntity<Void> reset(@PathVariable Long exerciseId) {
*/
@GetMapping("exercises/{exerciseId}/details")
@EnforceAtLeastStudent
public ResponseEntity<Exercise> getExerciseDetails(@PathVariable Long exerciseId) {
public ResponseEntity<ExerciseDetailsDTO> getExerciseDetails(@PathVariable Long exerciseId) {
User user = userRepository.getUserWithGroupsAndAuthorities();
Exercise exercise = exerciseService.findOneWithDetailsForStudents(exerciseId, user);

Expand Down Expand Up @@ -321,19 +338,24 @@ public ResponseEntity<Exercise> getExerciseDetails(@PathVariable Long exerciseId
quizExercise.setQuizBatches(null);
quizExercise.setQuizBatches(quizBatchService.getQuizBatchForStudentByLogin(quizExercise, user.getLogin()).stream().collect(Collectors.toSet()));
}
if (exercise instanceof ProgrammingExercise programmingExercise) {
// TODO: instead fetch the policy without programming exercise, should be faster
SubmissionPolicy policy = programmingExerciseRepository.findByIdWithSubmissionPolicyElseThrow(programmingExercise.getId()).getSubmissionPolicy();
programmingExercise.setSubmissionPolicy(policy);
}
// TODO: we should also check that the submissions do not contain sensitive data

// remove sensitive information for students
if (!isAtLeastTAForExercise) {
exercise.filterSensitiveInformation();
}

return ResponseEntity.ok(exercise);
IrisCombinedSettingsDTO irisSettings = irisSettingsService.map(service -> service.getCombinedIrisSettingsFor(exercise, service.showMinimalSettings(exercise, user)))
.orElse(null);
PlagiarismCaseInfoDTO plagiarismCaseInfo = plagiarismCaseService.getPlagiarismCaseInfoForExerciseAndUser(exercise.getId(), user.getId()).orElse(null);

if (exercise instanceof ProgrammingExercise programmingExercise) {
Set<ExerciseHint> activatedExerciseHints = exerciseHintService.getActivatedExerciseHints(programmingExercise, user);
Set<ExerciseHint> availableExerciseHints = exerciseHintService.getAvailableExerciseHints(programmingExercise, user);
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
return ResponseEntity.ok(new ExerciseDetailsDTO(exercise, irisSettings, plagiarismCaseInfo, availableExerciseHints, activatedExerciseHints));
}

return ResponseEntity.ok(new ExerciseDetailsDTO(exercise, irisSettings, plagiarismCaseInfo, null, null));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package de.tum.in.www1.artemis.web.rest.dto;

import java.util.Set;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.in.www1.artemis.domain.Exercise;
import de.tum.in.www1.artemis.domain.hestia.ExerciseHint;
import de.tum.in.www1.artemis.service.iris.dto.IrisCombinedSettingsDTO;
import de.tum.in.www1.artemis.web.rest.dto.plagiarism.PlagiarismCaseInfoDTO;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record ExerciseDetailsDTO(Exercise exercise, IrisCombinedSettingsDTO irisSettings, PlagiarismCaseInfoDTO plagiarismCaseInfo, Set<ExerciseHint> availableExerciseHints,
Set<ExerciseHint> activatedExerciseHints) {
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,7 @@ public ResponseEntity<IrisCombinedSettingsDTO> getProgrammingExerciseSettings(@P
var user = userRepository.getUserWithGroupsAndAuthorities();
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.STUDENT, exercise, user);

// Editors can see the full settings, students only the reduced settings
var getReduced = !authCheckService.isAtLeastEditorForExercise(exercise, user);
var combinedIrisSettings = irisSettingsService.getCombinedIrisSettingsFor(exercise, getReduced);
var combinedIrisSettings = irisSettingsService.getCombinedIrisSettingsFor(exercise, irisSettingsService.showMinimalSettings(exercise, user));
return ResponseEntity.ok(combinedIrisSettings);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,21 +193,7 @@ public ResponseEntity<PlagiarismCaseInfoDTO> getPlagiarismCaseForExerciseForStud
var user = userRepository.getUserWithGroupsAndAuthorities();
authenticationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, user);

var plagiarismCaseOptional = plagiarismCaseRepository.findByStudentIdAndExerciseIdWithPost(user.getId(), exerciseId);
if (plagiarismCaseOptional.isPresent()) {
// the student was notified if the plagiarism case is available (due to the nature of the query above)
var plagiarismCase = plagiarismCaseOptional.get();
// the following line is already checked in the SQL statement, but we want to ensure it 100%
if (plagiarismCase.getPost() != null) {
// Note: we only return the ID and verdict to tell the client there is a confirmed plagiarism case with student notification (post) and to support navigating to the
// detail page
// all other information might be irrelevant or sensitive and could lead to longer loading times
var plagiarismCaseInfoDTO = new PlagiarismCaseInfoDTO(plagiarismCase.getId(), plagiarismCase.getVerdict(), plagiarismCase.isCreatedByContinuousPlagiarismControl());
return ResponseEntity.ok(plagiarismCaseInfoDTO);
}
}
// in all other cases the response is empty
return ResponseEntity.ok(null);
return ResponseEntity.ok(plagiarismCaseService.getPlagiarismCaseInfoForExerciseAndUser(exerciseId, user.getId()).orElse(null));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class LearningPathContainerComponent implements OnInit {
loadExercise() {
this.exerciseService.getExerciseDetails(this.learningObjectId!).subscribe({
next: (exerciseResponse) => {
this.exercise = exerciseResponse.body!;
this.exercise = exerciseResponse.body!.exercise;
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
},
error: (errorResponse: HttpErrorResponse) => onError(this.alertService, errorResponse),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import { TextExercise } from 'app/entities/text-exercise.model';
import { FileUploadExercise } from 'app/entities/file-upload-exercise.model';
import { ArtemisMarkdownService } from 'app/shared/markdown.service';
import { SafeHtml } from '@angular/platform-browser';
import { PlagiarismCaseInfo } from 'app/exercises/shared/plagiarism/types/PlagiarismCaseInfo';
import { ExerciseHint } from 'app/entities/hestia/exercise-hint.model';
import { IrisExerciseSettings } from 'app/entities/iris/settings/iris-settings.model';

export type EntityResponseType = HttpResponse<Exercise>;
export type EntityArrayResponseType = HttpResponse<Exercise[]>;
Expand All @@ -30,6 +33,15 @@ export type ExampleSolutionInfo = {
exampleSolutionPublished: boolean;
};

export type EntityDetailsResponseType = HttpResponse<ExerciseDetailsType>;
export type ExerciseDetailsType = {
exercise: Exercise;
irisSettings?: IrisExerciseSettings;
plagiarismCaseInfo?: PlagiarismCaseInfo;
availableExerciseHints?: ExerciseHint[];
activatedExerciseHints?: ExerciseHint[];
};

export interface ExerciseServicable<T extends Exercise> {
create(exercise: T): Observable<HttpResponse<T>>;

Expand Down Expand Up @@ -146,16 +158,17 @@ export class ExerciseService {
* Get exercise details including all results for the currently logged-in user
* @param { number } exerciseId - Id of the exercise to get the repos from
*/
getExerciseDetails(exerciseId: number): Observable<EntityResponseType> {
return this.http.get<Exercise>(`${this.resourceUrl}/${exerciseId}/details`, { observe: 'response' }).pipe(
map((res: EntityResponseType) => {
this.processExerciseEntityResponse(res);

getExerciseDetails(exerciseId: number): Observable<EntityDetailsResponseType> {
return this.http.get<ExerciseDetailsType>(`${this.resourceUrl}/${exerciseId}/details`, { observe: 'response' }).pipe(
map((res: EntityDetailsResponseType) => {
if (res.body) {
res.body.exercise = ExerciseService.convertExerciseDatesFromServer(res.body.exercise)!;
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
ExerciseService.parseExerciseCategories(res.body.exercise);
// insert an empty list to avoid additional calls in case the list is empty on the server (because then it would be undefined in the client)
if (res.body.posts === undefined) {
res.body.posts = [];
if (res.body.exercise.posts === undefined) {
res.body.exercise.posts = [];
}
res.body.activatedExerciseHints?.forEach((hint) => this.entityTitleService.setTitle(EntityType.HINT, [hint?.id, exerciseId], hint?.title));
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
}
return res;
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Component, OnInit, Optional } from '@angular/core';
import { Exercise, ExerciseType } from 'app/entities/exercise.model';
import { Result } from 'app/entities/result.model';
import dayjs from 'dayjs/esm';
import { ExerciseService } from 'app/exercises/shared/exercise/exercise.service';
import { ExerciseDetailsType, ExerciseService } from 'app/exercises/shared/exercise/exercise.service';
import { ActivatedRoute } from '@angular/router';
import { HttpResponse } from '@angular/common/http';
import { ExerciseCacheService } from 'app/exercises/shared/exercise/exercise-cache.service';
Expand Down Expand Up @@ -35,8 +35,8 @@ export class StandaloneFeedbackComponent implements OnInit {
const participationId = parseInt(params['participationId'], 10);
const resultId = parseInt(params['resultId'], 10);

this.exerciseService.getExerciseDetails(exerciseId).subscribe((exerciseResponse: HttpResponse<Exercise>) => {
this.exercise = exerciseResponse.body!;
this.exerciseService.getExerciseDetails(exerciseId).subscribe((exerciseResponse: HttpResponse<ExerciseDetailsType>) => {
this.exercise = exerciseResponse.body!.exercise;
julian-christl marked this conversation as resolved.
Show resolved Hide resolved
const participation = this.exercise?.studentParticipations?.find((participation) => participation.id === participationId);
if (participation) {
participation.exercise = this.exercise;
Expand All @@ -50,7 +50,7 @@ export class StandaloneFeedbackComponent implements OnInit {
this.result = relevantResult;

// We set isBuilding here to false. It is the mobile applications responsibility to make the user aware if a participation is being built
const templateStatus = evaluateTemplateStatus(exerciseResponse.body!, participation, relevantResult, false);
const templateStatus = evaluateTemplateStatus(this.exercise, participation, relevantResult, false);
if (templateStatus == ResultTemplateStatus.MISSING) {
this.messageKey = 'artemisApp.result.notLatestSubmission';
} else {
Expand Down
Loading
Loading