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

Text Exercises: Replace feedback modal with inline feedback view #9310

Closed
Closed
Show file tree
Hide file tree
Changes from 60 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
cd72413
Add new router link to pick submission, improve error handling and ui…
Sep 12, 2024
10dd7a4
minor fixes
Sep 12, 2024
31bac76
revert result history to basic old way and fix some tests
Sep 12, 2024
2f0e6ad
remove no longer used history results scrollable option
Sep 12, 2024
4183099
fix cannot find getAll in test
Sep 12, 2024
7a949e9
details
Sep 12, 2024
2338dcd
ngtooltip -> tooltip
Sep 12, 2024
ee18bbd
ngtooltip -> tooltip
Sep 12, 2024
82c5491
clean up code and more fixed tests
Sep 13, 2024
cef96ed
mock getAll function in tests
Sep 13, 2024
a9c3e68
more tests
Sep 13, 2024
629d964
more tests
Sep 13, 2024
53bc43f
remove unused imports
Sep 13, 2024
a641a7c
reduce codacy complexity
Sep 13, 2024
136928a
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 13, 2024
8b98e0f
more tests, cleaner timeline
Sep 13, 2024
bec70f4
last is possibly undefined
Sep 13, 2024
eb7549d
improve coverage further
Sep 13, 2024
9f581d9
more result component tests
Sep 13, 2024
6bf027f
fix is possibly undefined in tests
Sep 13, 2024
e5fd0ba
and another one (tests)
Sep 13, 2024
8e3828a
even more tests
Sep 13, 2024
f0a0192
add translations and make submit add the submission to the participation
Sep 14, 2024
baa0c0c
handle possibly undefined submissions array
Sep 14, 2024
9961e30
revert passing results on submit
Sep 14, 2024
6a449eb
keep results but no need to push submission, make smoother result com…
Sep 14, 2024
b5082ac
fix test
Sep 14, 2024
3e10d01
unreferenced feedback exists too
Sep 14, 2024
ed965a8
assessment view exclude exam mode
Sep 14, 2024
4cdaf53
and not or
Sep 14, 2024
18024f7
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 14, 2024
43df5cb
only show ai feedback button if it is enabled
Sep 15, 2024
38c0592
improve translations and labels for feedback option
Sep 15, 2024
9ed844e
Merge branch 'feature/text-exercises/preliminary-feedback-view' of ht…
Sep 15, 2024
246e25c
show athena results even before the assessment due date
Sep 15, 2024
ecfd8e2
add preliminary tag, and default to latest submission if given submis…
Sep 15, 2024
b518281
allow feedback requests if date is not set
Sep 16, 2024
81f885d
address feedback on PR, hide ai button after deadline, show timeline …
Sep 17, 2024
312cc5a
dont send tutor result before assessment due date
Sep 17, 2024
8aeb55e
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
SimonEntholzer Sep 17, 2024
0817502
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
FelixTJDietrich Sep 19, 2024
3beb2db
fix result ratings and typos
Sep 19, 2024
4deae4e
Merge branch 'feature/text-exercises/preliminary-feedback-view' of ht…
Sep 19, 2024
2a12293
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 19, 2024
2ba858b
add new tests for rating component and text exercise resource with at…
Sep 19, 2024
ca3fa04
branch coverage
Sep 20, 2024
cc5a0d8
no left side optional chaining
Sep 20, 2024
1a70ef6
add tests for html elements in text editor
Sep 20, 2024
a490cfe
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 20, 2024
e5bed25
revert changes to text feedback tests
Sep 20, 2024
2cd13dc
Merge branch 'feature/text-exercises/preliminary-feedback-view' of ht…
Sep 20, 2024
aa9e64e
test more branches
Sep 20, 2024
b55ff4d
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
FelixTJDietrich Sep 20, 2024
b4dc5fb
copmonent -> comp
Sep 20, 2024
19693da
fix test
Sep 20, 2024
0f067a7
Merge branch 'feature/text-exercises/preliminary-feedback-view' of ht…
Sep 20, 2024
fc48fec
fix issue where athena result needs to be deleted on submit causing i…
Sep 20, 2024
81c9355
add translations for preconditions not met server side
Sep 21, 2024
ee18df7
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 21, 2024
9748390
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 23, 2024
3906d8f
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
FelixTJDietrich Sep 27, 2024
396ed6a
resolve conflicts
Sep 30, 2024
2317fb6
more conflict resolve
Sep 30, 2024
b7d8908
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
Sep 30, 2024
a612795
Merge branch 'develop' into feature/text-exercises/preliminary-feedba…
EneaGore Sep 30, 2024
ef36e39
Merge branch 'feature/text-exercises/preliminary-feedback-view' of ht…
Sep 30, 2024
2ec239d
fix style
Sep 30, 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 @@ -220,11 +220,11 @@ public List<Result> getManualResults() {
/**
* This method is necessary to ignore Athena results in the assessment view
*
* @return non athena automatic results including null results
* @return non athena automatic results excluding null results
*/
@JsonIgnore
public List<Result> getNonAthenaResults() {
return results.stream().filter(result -> result == null || !result.isAthenaAutomatic()).collect(Collectors.toCollection(ArrayList::new));
return results.stream().filter(result -> result != null && !result.isAthenaAutomatic()).collect(Collectors.toCollection(ArrayList::new));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ private List<SubmissionWithComplaintDTO> getSubmissionsWithComplaintsFromComplai
// add each submission with its complaint to the DTO
submissions.stream().filter(submission -> submission.getResultWithComplaint() != null).forEach(submission -> {
// get the complaint which belongs to the submission
submission.setResults(submission.getNonAthenaResults());
Complaint complaintOfSubmission = complaintMap.get(submission.getResultWithComplaint().getId());
prepareComplaintAndSubmission(complaintOfSubmission, submission);
submissionWithComplaintDTOs.add(new SubmissionWithComplaintDTO(submission, complaintOfSubmission));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected ResponseEntity<List<Submission>> getAllSubmissions(Long exerciseId, bo
submission.getParticipation().setExercise(null);
}
// Important for exercises with Athena results
if (assessedByTutor) {
if (assessedByTutor && !examMode) {
submission.setResults(submission.getNonAthenaResults());
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ else if (exercise instanceof ProgrammingExercise) {
// Process feedback request
StudentParticipation updatedParticipation;
if (exercise instanceof TextExercise) {
updatedParticipation = textExerciseFeedbackService.handleNonGradedFeedbackRequest(exercise.getId(), participation, (TextExercise) exercise);
updatedParticipation = textExerciseFeedbackService.handleNonGradedFeedbackRequest(participation, (TextExercise) exercise);
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
}
else {
updatedParticipation = programmingExerciseCodeReviewFeedbackService.handleNonGradedFeedbackRequest(exercise.getId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;

import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
Expand All @@ -21,11 +24,10 @@
import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService;
import de.tum.cit.aet.artemis.athena.service.AthenaFeedbackSuggestionsService;
import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException;
import de.tum.cit.aet.artemis.core.exception.InternalServerErrorException;
import de.tum.cit.aet.artemis.exercise.domain.participation.Participation;
import de.tum.cit.aet.artemis.exercise.domain.participation.StudentParticipation;
import de.tum.cit.aet.artemis.exercise.service.ParticipationService;
import de.tum.cit.aet.artemis.exercise.service.SubmissionService;
import de.tum.cit.aet.artemis.text.domain.TextBlock;
import de.tum.cit.aet.artemis.text.domain.TextExercise;
import de.tum.cit.aet.artemis.text.domain.TextSubmission;

Expand All @@ -35,8 +37,6 @@ public class TextExerciseFeedbackService {

private static final Logger log = LoggerFactory.getLogger(TextExerciseFeedbackService.class);

public static final String NON_GRADED_FEEDBACK_SUGGESTION = "NonGradedFeedbackSuggestion:";

private final Optional<AthenaFeedbackSuggestionsService> athenaFeedbackSuggestionsService;

private final ResultWebsocketService resultWebsocketService;
Expand All @@ -49,14 +49,18 @@ public class TextExerciseFeedbackService {

private final ResultRepository resultRepository;

private final TextBlockService textBlockService;

public TextExerciseFeedbackService(Optional<AthenaFeedbackSuggestionsService> athenaFeedbackSuggestionsService, SubmissionService submissionService,
ResultService resultService, ResultRepository resultRepository, ResultWebsocketService resultWebsocketService, ParticipationService participationService) {
ResultService resultService, ResultRepository resultRepository, ResultWebsocketService resultWebsocketService, ParticipationService participationService,
TextBlockService textBlockService) {
this.athenaFeedbackSuggestionsService = athenaFeedbackSuggestionsService;
this.submissionService = submissionService;
this.resultService = resultService;
this.resultRepository = resultRepository;
this.resultWebsocketService = resultWebsocketService;
this.participationService = participationService;
this.textBlockService = textBlockService;
}

private void checkRateLimitOrThrow(StudentParticipation participation) {
Expand All @@ -66,20 +70,19 @@ private void checkRateLimitOrThrow(StudentParticipation participation) {
long countOfAthenaResults = athenaResults.size();

if (countOfAthenaResults >= 10) {
throw new BadRequestAlertException("Maximum number of AI feedback requests reached.", "participation", "preconditions not met");
throw new BadRequestAlertException("Maximum number of AI feedback requests reached.", "participation", "preconditionsNotMet");
EneaGore marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
* Handles the request for generating feedback for a text exercise.
* Unlike programming exercises a tutor is not notified if Athena is not available.
*
* @param exerciseId the id of the text exercise.
* @param participation the student participation associated with the exercise.
* @param textExercise the text exercise object.
* @return StudentParticipation updated text exercise for an AI assessment
*/
public StudentParticipation handleNonGradedFeedbackRequest(Long exerciseId, StudentParticipation participation, TextExercise textExercise) {
public StudentParticipation handleNonGradedFeedbackRequest(StudentParticipation participation, TextExercise textExercise) {
EneaGore marked this conversation as resolved.
Show resolved Hide resolved
if (this.athenaFeedbackSuggestionsService.isPresent()) {
this.checkRateLimitOrThrow(participation);
CompletableFuture.runAsync(() -> this.generateAutomaticNonGradedFeedback(participation, textExercise));
Expand All @@ -103,50 +106,81 @@ public void generateAutomaticNonGradedFeedback(StudentParticipation participatio
if (submissionOptional.isEmpty()) {
throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmission");
}
var submission = submissionOptional.get();
TextSubmission textSubmission = (TextSubmission) submissionOptional.get();
EneaGore marked this conversation as resolved.
Show resolved Hide resolved

Result automaticResult = new Result();
automaticResult.setAssessmentType(AssessmentType.AUTOMATIC_ATHENA);
automaticResult.setRated(true);
automaticResult.setScore(0.0);
automaticResult.setSuccessful(null);
automaticResult.setSubmission(submission);
automaticResult.setSubmission(textSubmission);
automaticResult.setParticipation(participation);
try {
this.resultWebsocketService.broadcastNewResult((Participation) participation, automaticResult);
// This broadcast signals the client that feedback is being generated, does not save empty result
this.resultWebsocketService.broadcastNewResult(participation, automaticResult);

log.debug("Submission id: {}", textSubmission.getId());

log.debug("Submission id: {}", submission.getId());
var athenaResponse = this.athenaFeedbackSuggestionsService.orElseThrow().getTextFeedbackSuggestions(textExercise, textSubmission, true);

var athenaResponse = this.athenaFeedbackSuggestionsService.orElseThrow().getTextFeedbackSuggestions(textExercise, (TextSubmission) submission, false);
Set<TextBlock> textBlocks = new HashSet<>();
List<Feedback> feedbacks = new ArrayList<>();

List<Feedback> feedbacks = athenaResponse.stream().filter(individualFeedbackItem -> individualFeedbackItem.description() != null).map(individualFeedbackItem -> {
athenaResponse.stream().filter(individualFeedbackItem -> individualFeedbackItem.description() != null).forEach(individualFeedbackItem -> {
var textBlock = new TextBlock();
var feedback = new Feedback();

feedback.setText(individualFeedbackItem.title());
feedback.setDetailText(individualFeedbackItem.description());
feedback.setHasLongFeedbackText(false);
feedback.setType(FeedbackType.AUTOMATIC);
feedback.setCredits(individualFeedbackItem.credits());
return feedback;
}).toList();

if (textSubmission.getText() != null && individualFeedbackItem.indexStart() != null && individualFeedbackItem.indexEnd() != null) {
textBlock.setStartIndex(individualFeedbackItem.indexStart());
textBlock.setEndIndex(individualFeedbackItem.indexEnd());
textBlock.setSubmission(textSubmission);
textBlock.setTextFromSubmission();
textBlock.automatic();
textBlock.computeId();
feedback.setReference(textBlock.getId());
textBlock.setFeedback(feedback);
log.debug(textBlock.toString());

textBlocks.add(textBlock);
}
feedbacks.add(feedback);
});

double totalFeedbacksScore = 0.0;
for (Feedback feedback : feedbacks) {
totalFeedbacksScore += feedback.getCredits();
}
totalFeedbacksScore = totalFeedbacksScore / textExercise.getMaxPoints() * 100;
automaticResult.setSuccessful(true);
automaticResult.setCompletionDate(ZonedDateTime.now());
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved

automaticResult.setScore(Math.clamp(totalFeedbacksScore, 0, 100));

// For Athena automatic results successful = true will mean that the generation was successful
// undefined in progress and false it failed
automaticResult.setSuccessful(true);

automaticResult = this.resultRepository.save(automaticResult);
resultService.storeFeedbackInResult(automaticResult, feedbacks, true);
submissionService.saveNewResult(submission, automaticResult);
this.resultWebsocketService.broadcastNewResult((Participation) participation, automaticResult);
textBlockService.saveAll(textBlocks);
textSubmission.setBlocks(textBlocks);
submissionService.saveNewResult(textSubmission, automaticResult);
// This broadcast signals the client that feedback generation succeeded, result is saved in this case only
this.resultWebsocketService.broadcastNewResult(participation, automaticResult);
}
catch (Exception e) {
log.error("Could not generate feedback", e);
throw new InternalServerErrorException("Something went wrong... AI Feedback could not be generated");
// Broadcast the failed result but don't save, note that successful = false is normally used to indicate a score < 100
// but since we do not differentiate for athena feedback we use it to indicate a failed generation
automaticResult.setSuccessful(false);
automaticResult.setCompletionDate(null);
participation.addResult(automaticResult); // for proper change detection
EneaGore marked this conversation as resolved.
Show resolved Hide resolved
// This broadcast signals the client that feedback generation failed, does not save empty result
this.resultWebsocketService.broadcastNewResult(participation, automaticResult);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -413,44 +414,49 @@ public ResponseEntity<StudentParticipation> getDataForTextEditor(@PathVariable L
participation.setResults(new HashSet<>(results));
}

Optional<Submission> optionalSubmission = participation.findLatestSubmission();
if (!ExerciseDateService.isAfterAssessmentDueDate(textExercise)) {
// We want to have the preliminary feedback before the assessment due date too
Set<Result> athenaResults = participation.getResults().stream().filter(result -> result.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA)
.collect(Collectors.toSet());
participation.setResults(athenaResults);
}
EneaGore marked this conversation as resolved.
Show resolved Hide resolved

Set<Submission> submissions = participation.getSubmissions();
participation.setSubmissions(new HashSet<>());

if (optionalSubmission.isPresent()) {
TextSubmission textSubmission = (TextSubmission) optionalSubmission.get();
for (Submission submission : submissions) {
if (submission != null) {
TextSubmission textSubmission = (TextSubmission) submission;
EneaGore marked this conversation as resolved.
Show resolved Hide resolved

// set reference to participation to null, since we are already inside a participation
textSubmission.setParticipation(null);
// set reference to participation to null, since we are already inside a participation
textSubmission.setParticipation(null);
EneaGore marked this conversation as resolved.
Show resolved Hide resolved

if (!ExerciseDateService.isAfterAssessmentDueDate(textExercise)) {
// We want to have the preliminary feedback before the assessment due date too
List<Result> athenaResults = participation.getResults().stream().filter(result -> result.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA).toList();
textSubmission.setResults(athenaResults);
Set<Result> athenaResultsSet = new HashSet<Result>(athenaResults);
participation.setResults(athenaResultsSet);
}
if (!ExerciseDateService.isAfterAssessmentDueDate(textExercise)) {
// We want to have the preliminary feedback before the assessment due date too
List<Result> athenaResults = submission.getResults().stream().filter(result -> result.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA).toList();
textSubmission.setResults(athenaResults);
}

Result result = textSubmission.getLatestResult();
if (result != null) {
// Load TextBlocks for the Submission. They are needed to display the Feedback in the client.
final var textBlocks = textBlockRepository.findAllBySubmissionId(textSubmission.getId());
textSubmission.setBlocks(textBlocks);
Result result = textSubmission.getLatestResult();
if (result != null) {
// Load TextBlocks for the Submission. They are needed to display the Feedback in the client.
final var textBlocks = textBlockRepository.findAllBySubmissionId(textSubmission.getId());
textSubmission.setBlocks(textBlocks);

if (textSubmission.isSubmitted() && result.getCompletionDate() != null) {
List<Feedback> assessments = feedbackRepository.findByResult(result);
result.setFeedbacks(assessments);
}
if (textSubmission.isSubmitted() && result.getCompletionDate() != null) {
List<Feedback> assessments = feedbackRepository.findByResult(result);
result.setFeedbacks(assessments);
}

if (!authCheckService.isAtLeastTeachingAssistantForExercise(textExercise, user)) {
result.filterSensitiveInformation();
}
if (!authCheckService.isAtLeastTeachingAssistantForExercise(textExercise, user)) {
result.filterSensitiveInformation();
}

// only send the one latest result to the client
textSubmission.setResults(List.of(result));
participation.setResults(Set.of(result));
// only send the one latest result to the client
textSubmission.setResults(List.of(result));
}
participation.addSubmission(textSubmission);
}

participation.addSubmission(textSubmission);
}

if (!(authCheckService.isAtLeastInstructorForExercise(textExercise, user) || participation.isOwnedBy(user))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ export class HeaderParticipationPageComponent implements OnInit, OnChanges {
this.exerciseStatusBadge = hasExerciseDueDatePassed(this.exercise, this.participation) ? 'bg-danger' : 'bg-success';
this.exerciseCategories = this.exercise.categories || [];
this.dueDate = getExerciseDueDate(this.exercise, this.participation);
if (this.participation?.results?.[0]?.rated) {
if (this.participation?.results?.last()?.rated) {
this.achievedPoints = roundValueSpecifiedByCourseSettings(
// eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain
(this.participation.results?.[0].score! * this.exercise.maxPoints!) / 100,
(this.participation.results?.last()?.score! * this.exercise.maxPoints!) / 100,
getCourseFromExercise(this.exercise),
);
EneaGore marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ export class ExerciseScoresComponent implements OnInit, OnDestroy {
// the result of the first correction round will be at index 0,
// the result of a complaints or the second correction at index 1.
participation.results?.sort((result1, result2) => (result1.id ?? 0) - (result2.id ?? 0));

const resultsWithoutAthena = participation.results?.filter((result) => result.assessmentType !== AssessmentType.AUTOMATIC_ATHENA);
if (resultsWithoutAthena?.length != 0) {
if (resultsWithoutAthena?.[0].submission) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
id="allowFeedbackRequests"
(change)="toggleFeedbackRequests($event)"
/>
<label class="form-control-label" for="allowFeedbackRequests" jhiTranslate="artemisApp.programmingExercise.timeline.allowFeedbackRequests"></label>
<jhi-help-icon placement="right auto" [text]="'artemisApp.programmingExercise.timeline.allowFeedbackRequestsTooltip'" />
<label class="form-control-label" for="allowFeedbackRequests" jhiTranslate="artemisApp.textExercise.allowPreliminaryAthenaFeedbackRequests"></label>
<jhi-help-icon placement="right auto" [text]="'artemisApp.textExercise.allowPreliminaryAthenaFeedbackRequestsTooltip'" />
FelixTJDietrich marked this conversation as resolved.
Show resolved Hide resolved
</div>
}
@if (!!this.exercise.feedbackSuggestionModule) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export class FeedbackComponent implements OnInit, OnChanges {
faXmark = faXmark;
faCircleNotch = faCircleNotch;
faExclamationTriangle = faExclamationTriangle;

private showTestDetails = false;
isLoading = false;
loadingFailed = false;
Expand Down
Loading
Loading