Skip to content

Commit

Permalink
Bugfix/start exercise bindings (#501)
Browse files Browse the repository at this point in the history
* fixed binding of exercise details page and exercise list button

* added participation check to text exercises

* now updating participation status more reliably

* added missing websocket binding for text exercises

* added unsubscribe to participations

* added websocket update to exercise row, fixed two missing check errors

* fixed non updating quiz exercise elements

* renamed resume quiz to open quiz

* fixed view result for quiz exercises and multiple participations

* fixed rendering problem in result history, fixed server error for missing rated result for quiz exercises

* fixed exercise details page again, resetting cached participations even more on logout

* prevent wrong merge for participations

* reset websockets properly when user logs out

* show 'View submission' button instead of 'View result' before the assessment due date

* added missing translation strings for student question and answer update messages

* fixed race condition in websocket messages for first practice mode of quiz exercises
  • Loading branch information
maximilianmeier authored and Stephan Krusche committed May 28, 2019
1 parent 0b37b20 commit 6285ac3
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public interface ParticipationRepository extends JpaRepository<Participation, Lo

Optional<Participation> findByExerciseIdAndStudentLogin(Long exerciseId, String username);

Optional<Participation> findByInitializationStateAndExerciseIdAndStudentLogin(InitializationState initializationState, Long exerciseId, String username);

@Query("select distinct participation from Participation participation left join fetch participation.submissions where participation.exercise.id = :#{#exerciseId} and participation.student.login = :#{#username}")
Optional<Participation> findByExerciseIdAndStudentLoginWithEagerSubmissions(@Param("exerciseId") Long exerciseId, @Param("username") String username);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.stream.Collectors;

import org.slf4j.*;
import org.springframework.messaging.simp.SimpMessageSendingOperations;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand Down Expand Up @@ -35,14 +36,18 @@ public class ModelingSubmissionService {

private final ParticipationRepository participationRepository;

private final SimpMessageSendingOperations messagingTemplate;

public ModelingSubmissionService(ModelingSubmissionRepository modelingSubmissionRepository, ResultService resultService, ResultRepository resultRepository,
CompassService compassService, ParticipationService participationService, ParticipationRepository participationRepository) {
CompassService compassService, ParticipationService participationService, ParticipationRepository participationRepository,
SimpMessageSendingOperations messagingTemplate) {
this.modelingSubmissionRepository = modelingSubmissionRepository;
this.resultService = resultService;
this.resultRepository = resultRepository;
this.compassService = compassService;
this.participationService = participationService;
this.participationRepository = participationRepository;
this.messagingTemplate = messagingTemplate;
}

/**
Expand Down Expand Up @@ -185,6 +190,8 @@ public ModelingSubmission save(ModelingSubmission modelingSubmission, ModelingEx
notifyCompass(modelingSubmission, modelingExercise);
checkAutomaticResult(modelingSubmission, modelingExercise);
participation.setInitializationState(InitializationState.FINISHED);
messagingTemplate.convertAndSendToUser(participation.getStudent().getLogin(), "/topic/exercise/" + participation.getExercise().getId() + "/participation",
participation);
}
Participation savedParticipation = participationRepository.save(participation);
if (modelingSubmission.getId() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ else if (exercise instanceof QuizExercise || exercise instanceof ModelingExercis
public Participation participationForQuizWithResult(QuizExercise quizExercise, String username) {
if (quizExercise.isEnded()) {
// try getting participation from database first
Optional<Participation> optionalParticipation = findOneByExerciseIdAndStudentLoginAnyState(quizExercise.getId(), username);
Optional<Participation> optionalParticipation = findOneByExerciseIdAndStudentLoginAndFinished(quizExercise.getId(), username);

if (!optionalParticipation.isPresent()) {
log.error("Participation in quiz " + quizExercise.getTitle() + " not found for user " + username);
Expand Down Expand Up @@ -456,6 +456,19 @@ public Optional<Participation> findOneByExerciseIdAndStudentLoginAnyState(Long e
return participationRepository.findByExerciseIdAndStudentLogin(exerciseId, username);
}

/**
* Get one finished participation by its student and exercise.
*
* @param exerciseId the project key of the exercise
* @param username the username of the student
* @return the entity
*/
@Transactional(readOnly = true)
public Optional<Participation> findOneByExerciseIdAndStudentLoginAndFinished(Long exerciseId, String username) {
log.debug("Request to get Participation for User {} for Exercise with id: {}", username, exerciseId);
return participationRepository.findByInitializationStateAndExerciseIdAndStudentLogin(InitializationState.FINISHED, exerciseId, username);
}

/**
* Get one participation (in any state) by its student and exercise with eager submissions.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.stream.Collectors;

import org.springframework.http.HttpStatus;
import org.springframework.messaging.simp.SimpMessageSendingOperations;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.server.ResponseStatusException;
Expand All @@ -33,12 +34,15 @@ public class TextSubmissionService {

private final ResultRepository resultRepository;

private final SimpMessageSendingOperations messagingTemplate;

public TextSubmissionService(TextSubmissionRepository textSubmissionRepository, ParticipationRepository participationRepository, ParticipationService participationService,
ResultRepository resultRepository) {
ResultRepository resultRepository, SimpMessageSendingOperations messagingTemplate) {
this.textSubmissionRepository = textSubmissionRepository;
this.participationRepository = participationRepository;
this.participationService = participationService;
this.resultRepository = resultRepository;
this.messagingTemplate = messagingTemplate;
}

/**
Expand Down Expand Up @@ -93,6 +97,9 @@ public TextSubmission save(TextSubmission textSubmission, TextExercise textExerc

if (textSubmission.isSubmitted()) {
participation.setInitializationState(InitializationState.FINISHED);

messagingTemplate.convertAndSendToUser(participation.getStudent().getLogin(), "/topic/exercise/" + participation.getExercise().getId() + "/participation",
participation);
}
Participation savedParticipation = participationRepository.save(participation);
if (textSubmission.getId() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,6 @@ public ResponseEntity<Exercise> getResultsForCurrentStudent(@PathVariable Long e
exercise.setParticipations(new HashSet<>());

for (Participation participation : participations) {
// Removing not needed properties
participation.setStudent(null);

participation.setResults(exercise.findResultsFilteredForStudents(participation));
exercise.addParticipation(participation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ else if (quizExercise.isSubmissionAllowed()) {
quizExercise = quizExerciseService.findOneWithQuestions(quizExercise.getId());
Participation participation = participationService.participationForQuizWithResult(quizExercise, username);
// avoid problems due to bidirectional associations between submission and result during serialization
if (participation == null) {
return null;
}
for (Result result : participation.getResults()) {
if (result.getSubmission() != null) {
result.getSubmission().setResult(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.security.Principal;
import java.time.ZonedDateTime;

import org.hibernate.Hibernate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -99,7 +100,13 @@ public ResponseEntity<Result> submitForPractice(@PathVariable Long courseId, @Pa
quizExercise.setQuizPointStatistic(null);
quizExercise.setCourse(null);

messagingTemplate.convertAndSend("/topic/participation/" + result.getParticipation().getId() + "/newResults", result);
if (Hibernate.isInitialized(participation.getResults()) && participation.getResults().size() == 0) {
participation.addResult(result);
messagingTemplate.convertAndSendToUser(principal.getName(), "/topic/exercise/" + quizExercise.getId() + "/participation", participation);
}
else {
messagingTemplate.convertAndSend("/topic/participation/" + result.getParticipation().getId() + "/newResults", result);
}
// return result with quizSubmission, participation and quiz exercise (including the solution)
return ResponseEntity.ok(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ export class ParticipationWebsocketService {
participations.forEach(participation => {
this.removeParticipation(participation.id, participation.exercise.id);
});
this.cachedParticipations = new Map<number, Participation>();
this.resultObservables = new Map<number, BehaviorSubject<Result>>();
this.participationObservable = null;
}

updateParticipation(participation: Participation, exercise?: Exercise) {
Expand Down Expand Up @@ -145,7 +148,11 @@ export class ParticipationWebsocketService {
const participationObservable = this.jhiWebsocketService.receive(participationTopic);
participationObservable.subscribe((participationMessage: Participation) => {
this.addParticipation(participationMessage);
this.participationObservable.next(participationMessage);
if (!this.participationObservable) {
this.participationObservable = new BehaviorSubject<Participation>(participationMessage);
} else {
this.participationObservable.next(participationMessage);
}
});
this.openWebsocketConnections.set(`${PARTICIPATION_WEBSOCKET}${exerciseId}`, participationTopic);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,14 @@ export class ParticipationService {
combinedParticipation.exercise = participations[0].exercise;

participations.forEach(participation => {
combinedParticipation.results = combinedParticipation.results ? combinedParticipation.results.concat(participation.results) : participation.results;
combinedParticipation.submissions = combinedParticipation.submissions
? combinedParticipation.submissions.concat(participation.submissions)
: participation.submissions;
if (participation.results) {
combinedParticipation.results = combinedParticipation.results ? combinedParticipation.results.concat(participation.results) : participation.results;
}
if (participation.submissions) {
combinedParticipation.submissions = combinedParticipation.submissions
? combinedParticipation.submissions.concat(participation.submissions)
: participation.submissions;
}
});
}
return combinedParticipation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<div class="result-divider"></div>
</div>
<div class="result-history-element" *ngFor="let result of results">
<div class="result-score" [ngbTooltip]="result.completionDate | date: 'dd/MM/yy HH:mm'" placement="top" container="body">
<div *ngIf="!!result" class="result-score" [ngbTooltip]="result.completionDate | date: 'dd/MM/yy HH:mm'" placement="top" container="body">
<div class="result-score-icon" [ngClass]="resultClass(result)">
<fa-icon [icon]="resultIcon(result)"></fa-icon>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class UpdatingResultComponent implements OnInit, OnChanges, OnDestroy {
subscribeForNewResults(exercise: Exercise) {
this.accountService.identity().then(user => {
// only subscribe for the currently logged in user or if the participation is a template/solution participation and the student is at least instructor
const isInstructorInCourse = this.participation.student == null && this.accountService.isAtLeastInstructorInCourse(exercise.course);
const isInstructorInCourse = this.participation.student == null && exercise.course && this.accountService.isAtLeastInstructorInCourse(exercise.course);
const isSameUser = this.participation.student && user.id === this.participation.student.id;
const exerciseNotOver = exercise.dueDate == null || (moment(exercise.dueDate).isValid() && moment(exercise.dueDate).isAfter(moment()));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Component, HostBinding, Input, OnInit } from '@angular/core';
import { Component, HostBinding, Input, OnInit, OnDestroy } from '@angular/core';
import { Exercise, ExerciseCategory, ExerciseService, ExerciseType, ParticipationStatus, getIcon, getIconTooltip } from 'app/entities/exercise';
import { JhiAlertService } from 'ng-jhipster';
import { QuizExercise } from 'app/entities/quiz-exercise';
import { InitializationState, Participation, ParticipationService } from 'app/entities/participation';
import { InitializationState, Participation, ParticipationService, ParticipationWebsocketService } from 'app/entities/participation';
import * as moment from 'moment';
import { Subscription } from 'rxjs/Subscription';

import { Moment } from 'moment';
import { Course } from 'app/entities/course';
import { AccountService, WindowRef } from 'app/core';
Expand All @@ -16,7 +18,7 @@ import { ProgrammingExercise } from 'app/entities/programming-exercise';
templateUrl: './course-exercise-row.component.html',
styleUrls: ['./course-exercise-row.scss'],
})
export class CourseExerciseRowComponent implements OnInit {
export class CourseExerciseRowComponent implements OnInit, OnDestroy {
readonly QUIZ = ExerciseType.QUIZ;
readonly PROGRAMMING = ExerciseType.PROGRAMMING;
readonly MODELING = ExerciseType.MODELING;
Expand All @@ -32,6 +34,8 @@ export class CourseExerciseRowComponent implements OnInit {
public exerciseCategories: ExerciseCategory[];
isAfterAssessmentDueDate: boolean;

participationUpdateListener: Subscription;

constructor(
private accountService: AccountService,
private jhiAlertService: JhiAlertService,
Expand All @@ -41,9 +45,26 @@ export class CourseExerciseRowComponent implements OnInit {
private httpClient: HttpClient,
private router: Router,
private route: ActivatedRoute,
private participationWebsocketService: ParticipationWebsocketService,
) {}

ngOnInit() {
const cachedParticipations = this.participationWebsocketService.getAllParticipationsForExercise(this.exercise.id);
if (cachedParticipations && cachedParticipations.length > 0) {
this.exercise.participations = cachedParticipations;
}
this.participationWebsocketService.addExerciseForNewParticipation(this.exercise.id);
this.participationUpdateListener = this.participationWebsocketService.subscribeForParticipationChanges().subscribe((changedParticipation: Participation) => {
if (changedParticipation && this.exercise && changedParticipation.exercise.id === this.exercise.id) {
this.exercise.participations =
this.exercise.participations && this.exercise.participations.length > 0
? this.exercise.participations.map(el => {
return el.id === changedParticipation.id ? changedParticipation : el;
})
: [changedParticipation];
this.participationStatus(this.exercise);
}
});
this.exercise.participationStatus = this.participationStatus(this.exercise);
if (this.exercise.participations.length > 0) {
this.exercise.participations[0].exercise = this.exercise;
Expand All @@ -61,6 +82,12 @@ export class CourseExerciseRowComponent implements OnInit {
this.exerciseCategories = this.exerciseService.convertExerciseCategoriesFromServer(this.exercise);
}

ngOnDestroy() {
if (this.participationUpdateListener) {
this.participationUpdateListener.unsubscribe();
}
}

getUrgentClass(date: Moment): string {
if (!date) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
<!-- HEADER INFORMATION END -->
<!-- EXERCISE ACTIONS START -->
<div class="tab-bar pl-2 pr-2 mb-2 justify-content-between">
<jhi-exercise-details-student-actions [courseId]="courseId" [exercise]="exercise"> </jhi-exercise-details-student-actions>
<jhi-exercise-details-student-actions [courseId]="courseId" [exercise]="exercise" [ratedResult]="showResults ? currentResult : null">
</jhi-exercise-details-student-actions>
<div class="col-auto d-none d-md-flex align-items-center" *ngIf="exercise.isAtLeastInstructor">
<span class="mr-1">{{ 'arTeMiSApp.courseOverview.exerciseDetails.instructorActions.title' | translate }}</span>
<div class="btn-group">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export class CourseExerciseDetailsComponent implements OnInit, OnDestroy {
if (cachedParticipations && cachedParticipations.length > 0) {
this.exerciseService.find(this.exerciseId).subscribe((exerciseResponse: HttpResponse<Exercise>) => {
this.exercise = exerciseResponse.body;
this.exercise.participations = cachedParticipations.filter((participation: Participation) => participation.student.id === this.currentUser.id);
this.exercise.participations = cachedParticipations.filter(
(participation: Participation) => participation.student && participation.student.id === this.currentUser.id,
);
this.mergeResultsAndSubmissionsForParticipations();
this.isAfterAssessmentDueDate = !this.exercise.assessmentDueDate || moment().isAfter(this.exercise.assessmentDueDate);
this.exerciseCategories = this.exerciseService.convertExerciseCategoriesFromServer(this.exercise);
Expand Down Expand Up @@ -123,11 +125,12 @@ export class CourseExerciseDetailsComponent implements OnInit, OnDestroy {
}
this.participationUpdateListener = this.participationWebsocketService.subscribeForParticipationChanges().subscribe((changedParticipation: Participation) => {
if (changedParticipation && this.exercise && changedParticipation.exercise.id === this.exercise.id) {
this.exercise.participations = this.exercise.participations
? this.exercise.participations.map(el => {
return el.id === changedParticipation.id ? changedParticipation : el;
})
: [changedParticipation];
this.exercise.participations =
this.exercise.participations && this.exercise.participations.length > 0
? this.exercise.participations.map(el => {
return el.id === changedParticipation.id ? changedParticipation : el;
})
: [changedParticipation];
this.mergeResultsAndSubmissionsForParticipations();
}
});
Expand Down Expand Up @@ -169,7 +172,7 @@ export class CourseExerciseDetailsComponent implements OnInit, OnDestroy {
}

get currentResult(): Result {
if (!this.exercise.participations || !this.exercise.participations[0].results) {
if (!this.exercise.participations || !this.exercise.participations[0] || !this.exercise.participations[0].results) {
return null;
}
const results = this.exercise.participations[0].results;
Expand Down
Loading

0 comments on commit 6285ac3

Please sign in to comment.