Skip to content

Commit

Permalink
Improvements for tutor dashboard (#291)
Browse files Browse the repository at this point in the history
* display massage indicating that submission is locked
* delete corresponding submission when deleting example submission
* fix more efficient loading of submitted submissions without result
* prevent errors when no feedbacks exist
* load next optimal submission upon starting new assessment to prevent opening assessment editor for locked submission
* prevent error when there are no complaints
* disable Compass
* handle case when starting assessment in exercise dashboard but no submissions waiting for assessment available
* fix 'assess next submission' button when diagram type is not supported by Compass
* fix redirect when no submission is loaded
* adjust transactions to prevent example solution and explanation from getting removed from the exercise
  • Loading branch information
SiggZ authored and Stephan Krusche committed Apr 17, 2019
1 parent 4ace695 commit 0cff4fa
Show file tree
Hide file tree
Showing 21 changed files with 284 additions and 187 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class ExampleSubmission implements Serializable {
@ManyToOne
private Exercise exercise;

@OneToOne
@OneToOne(cascade = CascadeType.REMOVE)
@JoinColumn(unique = true)
private Submission submission;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public class Participation implements Serializable {
// objects would cause more issues (Subclasses don't work properly for Proxy objects)
// and the gain from fetching lazy here is minimal
@ManyToOne
@JsonIgnoreProperties("participations")
@JsonView(QuizView.Before.class)
private Exercise exercise;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ public interface ParticipationRepository extends JpaRepository<Participation, Lo
@Query("select distinct participation from Participation participation left join fetch participation.results where participation.exercise.id = :#{#exerciseId} and participation.student.id = :#{#studentId}")
List<Participation> findByExerciseIdAndStudentIdWithEagerResults(@Param("exerciseId") Long exerciseId, @Param("studentId") Long studentId);

@Query("select distinct participation from Participation participation left join fetch participation.submissions where participation.exercise.id = :#{#exerciseId}")
List<Participation> findByExerciseIdWithEagerSubmissions(@Param("exerciseId") Long exerciseId);

// TODO: this call does not work at the moment
@Query("select distinct participation from Participation participation left join fetch participation.submissions submission where participation.exercise.id = :#{#exerciseId} and submission.submitted = true and submission.result is null")
@Query("select distinct participation from Participation participation left join fetch participation.submissions submission left join fetch submission.result result where participation.exercise.id = :#{#exerciseId} and submission.submitted = true and result is null")
List<Participation> findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(@Param("exerciseId") Long exerciseId);

@Query("select distinct participation from Participation participation left join fetch participation.results where participation.id = :#{#participationId}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public class ModelingSubmissionService {

private final ModelingSubmissionRepository modelingSubmissionRepository;

private final ResultService resultService;

private final ResultRepository resultRepository;

private final CompassService compassService;
Expand All @@ -33,9 +35,10 @@ public class ModelingSubmissionService {

private final ParticipationRepository participationRepository;

public ModelingSubmissionService(ModelingSubmissionRepository modelingSubmissionRepository, ResultRepository resultRepository, CompassService compassService,
ParticipationService participationService, ParticipationRepository participationRepository) {
public ModelingSubmissionService(ModelingSubmissionRepository modelingSubmissionRepository, ResultService resultService, ResultRepository resultRepository, CompassService compassService,
ParticipationService participationService, ParticipationRepository participationRepository) {
this.modelingSubmissionRepository = modelingSubmissionRepository;
this.resultService = resultService;
this.resultRepository = resultRepository;
this.compassService = compassService;
this.participationService = participationService;
Expand Down Expand Up @@ -69,11 +72,30 @@ public List<ModelingSubmission> getModelingSubmissions(Long exerciseId, boolean
return submissions;
}

@Transactional
public ModelingSubmission getLockedModelingSubmission(Long submissionId, ModelingExercise modelingExercise) {
ModelingSubmission modelingSubmission = findOneWithEagerResultAndFeedback(submissionId);
lockSubmission(modelingSubmission, modelingExercise);
return modelingSubmission;
}

@Transactional
public ModelingSubmission getLockedModelingSubmissionWithoutResult(ModelingExercise modelingExercise) {
ModelingSubmission modelingSubmission = getModelingSubmissionWithoutResult(modelingExercise)
.orElseThrow(() -> new EntityNotFoundException("Modeling submission for exercise " + modelingExercise.getId() + " could not be found"));
lockSubmission(modelingSubmission, modelingExercise);
return modelingSubmission;
}

/**
* Given an exercise, find a random modeling submission for that exercise which still doesn't have any result. We relay for the randomness to `findAny()`, which return any
* element of the stream. While it is not mathematically random, it is not deterministic https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#findAny--
* Given an exercise, find a modeling submission for that exercise which still doesn't have any result.
* If the diagram type is supported by Compass we get the next optimal submission from Compass, i.e. the submission
* for which an assessment means the most knowledge gain for the automatic assessment mechanism.
* If it's not supported by Compass we just get a random submission without assessment. We relay for the randomness
* to `findAny()`, which return any element of the stream. While it is not mathematically random, it is not
* deterministic https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#findAny--
*
* @param modelingExercise the modeling exercise for which we want to get a modeling submission
* @param modelingExercise the modeling exercise for which we want to get a modeling submission without result
* @return a modeling submission without any result
*/
@Transactional(readOnly = true)
Expand All @@ -86,16 +108,9 @@ public Optional<ModelingSubmission> getModelingSubmissionWithoutResult(ModelingE
}
}
// otherwise return any submission that is not assessed
// TODO: optimize performance
return this.participationService.findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(modelingExercise.getId()).stream()
.peek(participation -> participation.getExercise().setParticipations(null))
// Map to Latest Submission
.map(Participation::findLatestModelingSubmission).filter(Optional::isPresent).map(Optional::get)
// It needs to be submitted to be ready for assessment
.filter(Submission::isSubmitted).filter(modelingSubmission -> {
Result result = resultRepository.findDistinctBySubmissionId(modelingSubmission.getId()).orElse(null);
return result == null;
}).findAny();
return participationService.findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(modelingExercise.getId()).stream()
// map to latest submission
.map(Participation::findLatestModelingSubmission).filter(Optional::isPresent).map(Optional::get).findAny();
}

/**
Expand Down Expand Up @@ -131,11 +146,17 @@ public List<ModelingSubmission> getAllModelingSubmissionsByTutorForExercise(Long
*
* @param modelingSubmission the submission to notifyCompass
* @param modelingExercise the exercise to notifyCompass in
* @param participation the participation where the result should be saved
* @param username the name of the corresponding user
* @return the modelingSubmission entity
*/
@Transactional(rollbackFor = Exception.class)
public ModelingSubmission save(ModelingSubmission modelingSubmission, ModelingExercise modelingExercise, Participation participation) {
public ModelingSubmission save(ModelingSubmission modelingSubmission, ModelingExercise modelingExercise, String username) {

Optional<Participation> optionalParticipation = participationService.findOneByExerciseIdAndStudentLoginAnyState(modelingExercise.getId(), username);
if (!optionalParticipation.isPresent()) {
throw new EntityNotFoundException("No participation found for " + username + " in exercise " + modelingExercise.getId());
}
Participation participation = optionalParticipation.get();

// update submission properties
modelingSubmission.setSubmissionDate(ZonedDateTime.now());
Expand Down Expand Up @@ -167,12 +188,35 @@ else if (modelingExercise.getDueDate() != null && !modelingExercise.isEnded()) {
return modelingSubmission;
}

/**
* Soft lock the submission to prevent other tutors from receiving and assessing it.
* We remove the model from the models waiting for assessment in Compass to prevent other tutors from retrieving it
* in the first place.
* Additionally, we set the assessor and save the result to soft lock the assessment in the client, i.e. the client
* will not allow tutors to assess a model when an assessor is already assigned. If no result exists for this
* submission we create one first.
*
* @param modelingSubmission the submission to lock
* @param modelingExercise the exercise to which the submission belongs to (needed for Compass)
*/
private void lockSubmission(ModelingSubmission modelingSubmission, ModelingExercise modelingExercise) {
if (modelingSubmission.getResult() == null) {
setNewResult(modelingSubmission);
}
if (modelingSubmission.getResult().getAssessor() == null) {
if (compassService.isSupported(modelingExercise.getDiagramType())) {
compassService.removeModelWaitingForAssessment(modelingExercise.getId(), modelingSubmission.getId());
}
resultService.setAssessor(modelingSubmission.getResult());
}
}

/**
* Creates and sets new Result object in given submission and stores changes to the database.
*
* @param submission
*/
public void setNewResult(ModelingSubmission submission) {
private void setNewResult(ModelingSubmission submission) {
Result result = new Result();
result.setSubmission(submission);
submission.setResult(result);
Expand All @@ -193,13 +237,19 @@ public void notifyCompass(ModelingSubmission modelingSubmission, ModelingExercis
}
}

public ModelingSubmission findOne(Long id) {
return modelingSubmissionRepository.findById(id)
.orElseThrow(() -> new EntityNotFoundException("Modeling submission with id \"" + id + "\" does not exist"));
}

public ModelingSubmission findOneWithEagerResult(Long id) {
return modelingSubmissionRepository.findByIdWithEagerResult(id).orElseThrow(() -> new EntityNotFoundException("Modeling submission with id \"" + id + "\" does not exist"));
return modelingSubmissionRepository.findByIdWithEagerResult(id)
.orElseThrow(() -> new EntityNotFoundException("Modeling submission with id \"" + id + "\" does not exist"));
}

public ModelingSubmission findOneWithEagerResultAndFeedback(Long id) {
return modelingSubmissionRepository.findByIdWithEagerResultAndFeedback(id)
.orElseThrow(() -> new EntityNotFoundException("Modeling submission with id \"" + id + "\" does not exist"));
.orElseThrow(() -> new EntityNotFoundException("Modeling submission with id \"" + id + "\" does not exist"));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,9 +469,7 @@ public List<Participation> findByExerciseIdAndStudentIdWithEagerResults(Long exe

@Transactional(readOnly = true)
public List<Participation> findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(Long exerciseId) {
// TODO optimize performance
List<Participation> participations = participationRepository.findByExerciseIdWithEagerSubmissions(exerciseId);
return participations;
return participationRepository.findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(exerciseId);
}

@Transactional(readOnly = true)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package de.tum.in.www1.artemis.service;

import java.security.Principal;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.server.ResponseStatusException;

import de.tum.in.www1.artemis.domain.*;
import de.tum.in.www1.artemis.domain.enumeration.InitializationState;
Expand Down Expand Up @@ -37,6 +40,30 @@ public TextSubmissionService(TextSubmissionRepository textSubmissionRepository,
this.resultRepository = resultRepository;
}

/**
* Handles text submissions sent from the client and saves them in the database.
*
* @param textSubmission the text submission that should be saved
* @param textExercise the corresponding text exercise
* @param principal the user principal
* @return the saved text submission
*/
@Transactional
public TextSubmission handleTextSubmission(TextSubmission textSubmission, TextExercise textExercise, Principal principal) {
if (textSubmission.isExampleSubmission() == Boolean.TRUE) {
textSubmission = save(textSubmission);
}
else {
Optional<Participation> optionalParticipation = participationService.findOneByExerciseIdAndStudentLoginAnyState(textExercise.getId(), principal.getName());
if (!optionalParticipation.isPresent()) {
throw new ResponseStatusException(HttpStatus.FAILED_DEPENDENCY, "No participation found for " + principal.getName() + " in exercise " + textExercise.getId());
}
Participation participation = optionalParticipation.get();
textSubmission = save(textSubmission, textExercise, participation);
}
return textSubmission;
}

/**
* Saves the given submission. Furthermore, the submission is added to the AutomaticSubmissionService, if not submitted yet. Is used for creating and updating text submissions.
* If it is used for a submit action, Compass is notified about the new model. Rolls back if inserting fails - occurs for concurrent createTextSubmission() calls.
Expand Down Expand Up @@ -106,16 +133,9 @@ public TextSubmission save(TextSubmission textSubmission) {
*/
@Transactional(readOnly = true)
public Optional<TextSubmission> getTextSubmissionWithoutResult(TextExercise textExercise) {
// TODO: optimize performance
return this.participationService.findByExerciseIdWithEagerSubmittedSubmissionsWithoutResults(textExercise.getId()).stream()
.peek(participation -> participation.getExercise().setParticipations(null))
// Map to Latest Submission
.map(Participation::findLatestTextSubmission).filter(Optional::isPresent).map(Optional::get)
// It needs to be submitted to be ready for assessment
.filter(Submission::isSubmitted).filter(textSubmission -> {
Result result = resultRepository.findDistinctBySubmissionId(textSubmission.getId()).orElse(null);
return result == null;
}).findAny();
.map(Participation::findLatestTextSubmission).filter(Optional::isPresent).map(Optional::get).findAny();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ public CompassService(ResultRepository resultRepository, ModelingExerciseReposit

public boolean isSupported(DiagramType diagramType) {
// at the moment, we only support class diagrams
return diagramType == DiagramType.ClassDiagram;
// TODO CZ: enable class diagrams again
// return diagramType == DiagramType.ClassDiagram;
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -18,8 +17,6 @@
import de.tum.in.www1.artemis.domain.*;
import de.tum.in.www1.artemis.domain.modeling.ModelingExercise;
import de.tum.in.www1.artemis.domain.modeling.ModelingSubmission;
import de.tum.in.www1.artemis.repository.ModelingSubmissionRepository;
import de.tum.in.www1.artemis.repository.ResultRepository;
import de.tum.in.www1.artemis.service.*;
import de.tum.in.www1.artemis.service.compass.CompassService;
import de.tum.in.www1.artemis.service.compass.conflict.Conflict;
Expand Down Expand Up @@ -54,14 +51,11 @@ public class ModelingAssessmentResource extends AssessmentResource {

private final ModelingSubmissionService modelingSubmissionService;

private final ModelingSubmissionRepository modelingSubmissionRepository;

private final ExampleSubmissionService exampleSubmissionService;

public ModelingAssessmentResource(AuthorizationCheckService authCheckService, UserService userService, CompassService compassService,
ModelingExerciseService modelingExerciseService, AuthorizationCheckService authCheckService1, CourseService courseService,
ModelingAssessmentService modelingAssessmentService, ModelingSubmissionService modelingSubmissionService, ModelingSubmissionRepository modelingSubmissionRepository,
ExampleSubmissionService exampleSubmissionService, ResultRepository resultRepository) {
ModelingAssessmentService modelingAssessmentService, ModelingSubmissionService modelingSubmissionService, ExampleSubmissionService exampleSubmissionService) {
super(authCheckService, userService);
this.compassService = compassService;
this.modelingExerciseService = modelingExerciseService;
Expand All @@ -70,37 +64,6 @@ public ModelingAssessmentResource(AuthorizationCheckService authCheckService, Us
this.exampleSubmissionService = exampleSubmissionService;
this.modelingAssessmentService = modelingAssessmentService;
this.modelingSubmissionService = modelingSubmissionService;
this.modelingSubmissionRepository = modelingSubmissionRepository;
}

@DeleteMapping("/exercises/{exerciseId}/optimal-model-submissions")
@PreAuthorize("hasAnyRole('TA', 'INSTRUCTOR', 'ADMIN')")
public ResponseEntity<String> resetOptimalModels(@PathVariable Long exerciseId) {
ModelingExercise modelingExercise = modelingExerciseService.findOne(exerciseId);
checkAuthorization(modelingExercise);
if (compassService.isSupported(modelingExercise.getDiagramType())) {
compassService.resetModelsWaitingForAssessment(exerciseId);
}
return ResponseEntity.noContent().build();
}

// TODO MJ add api documentation (returns list of submission ids as array)
@GetMapping("/exercises/{exerciseId}/optimal-model-submissions")
@PreAuthorize("hasAnyRole('TA', 'INSTRUCTOR', 'ADMIN')")
@Transactional
public ResponseEntity<Long[]> getNextOptimalModelSubmissions(@PathVariable Long exerciseId) {
ModelingExercise modelingExercise = modelingExerciseService.findOne(exerciseId);
checkAuthorization(modelingExercise);
if (compassService.isSupported(modelingExercise.getDiagramType())) {
Set<Long> optimalModelSubmissions = compassService.getModelsWaitingForAssessment(exerciseId);
if (optimalModelSubmissions.isEmpty()) {
return ResponseEntity.ok(new Long[] {}); // empty
}
return ResponseEntity.ok(optimalModelSubmissions.toArray(new Long[] {}));
}
else {
return ResponseEntity.ok(new Long[] {}); // empty
}
}

@GetMapping("/modeling-submissions/{submissionId}/partial-assessment")
Expand Down
Loading

0 comments on commit 0cff4fa

Please sign in to comment.