-
Notifications
You must be signed in to change notification settings - Fork 291
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
General:
Make Request Feedback a standalone component
#9323
Changes from all commits
ab5af3c
9711dcc
d0f9539
3e2a994
6493b0e
42f6b39
d8d0c9b
7bd2101
653816d
dabd872
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ | |
import jakarta.annotation.Nullable; | ||
import jakarta.validation.constraints.NotNull; | ||
|
||
import org.apache.velocity.exception.ResourceNotFoundException; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Value; | ||
|
@@ -382,7 +381,7 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc | |
throw new BadRequestAlertException("Not intended for the use in exams", "participation", "preconditions not met"); | ||
} | ||
if (exercise.getDueDate() != null && now().isAfter(exercise.getDueDate())) { | ||
throw new BadRequestAlertException("The due date is over", "participation", "preconditions not met"); | ||
throw new BadRequestAlertException("The due date is over", "participation", "feedbackRequestAfterDueDate", true); | ||
} | ||
if (exercise instanceof ProgrammingExercise) { | ||
((ProgrammingExercise) exercise).validateSettingsForFeedbackRequest(); | ||
|
@@ -393,7 +392,7 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc | |
StudentParticipation participation = (exercise instanceof ProgrammingExercise) | ||
? programmingExerciseParticipationService.findStudentParticipationByExerciseAndStudentId(exercise, principal.getName()) | ||
: studentParticipationRepository.findByExerciseIdAndStudentLogin(exercise.getId(), principal.getName()) | ||
.orElseThrow(() -> new ResourceNotFoundException("Participation not found")); | ||
.orElseThrow(() -> new BadRequestAlertException("Participation not found", "participation", "noSubmissionExists", true)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use When a participation is not found, it's appropriate to throw an |
||
|
||
checkAccessPermissionOwner(participation, user); | ||
participation = studentParticipationRepository.findByIdWithResultsElseThrow(participation.getId()); | ||
|
@@ -406,15 +405,15 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc | |
} | ||
else if (exercise instanceof ProgrammingExercise) { | ||
if (participation.findLatestLegalResult() == null) { | ||
throw new BadRequestAlertException("User has not reached the conditions to submit a feedback request", "participation", "preconditions not met"); | ||
throw new BadRequestAlertException("You need to submit at least once and have the build results", "participation", "noSubmissionExists", true); | ||
undernagruzez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
// Check if feedback has already been requested | ||
var currentDate = now(); | ||
var participationIndividualDueDate = participation.getIndividualDueDate(); | ||
if (participationIndividualDueDate != null && currentDate.isAfter(participationIndividualDueDate)) { | ||
throw new BadRequestAlertException("Request has already been sent", "participation", "already sent"); | ||
throw new BadRequestAlertException("Request has already been sent", "participation", "feedbackRequestAlreadySent", true); | ||
} | ||
|
||
// Process feedback request | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -111,7 +111,7 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici | |||||
var submissionOptional = programmingExerciseParticipationService.findProgrammingExerciseParticipationWithLatestSubmissionAndResult(participation.getId()) | ||||||
.findLatestSubmission(); | ||||||
if (submissionOptional.isEmpty()) { | ||||||
throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmission"); | ||||||
throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmissionÊxists"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo in error code "noSubmissionÊxists" The error code in the Apply this diff to correct the typo: -throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmissionÊxists");
+throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmissionExists"); 📝 Committable suggestion
Suggested change
|
||||||
} | ||||||
var submission = submissionOptional.get(); | ||||||
|
||||||
|
@@ -225,15 +225,10 @@ private void checkRateLimitOrThrow(ProgrammingExerciseStudentParticipation parti | |||||
|
||||||
List<Result> athenaResults = participation.getResults().stream().filter(result -> result.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA).toList(); | ||||||
|
||||||
long countOfAthenaResultsInProcessOrSuccessful = athenaResults.stream().filter(result -> result.isSuccessful() == null || result.isSuccessful() == Boolean.TRUE).count(); | ||||||
|
||||||
long countOfSuccessfulRequests = athenaResults.stream().filter(result -> result.isSuccessful() == Boolean.TRUE).count(); | ||||||
|
||||||
if (countOfAthenaResultsInProcessOrSuccessful >= 3) { | ||||||
throw new BadRequestAlertException("Cannot send additional AI feedback requests now. Try again later!", "participation", "preconditions not met"); | ||||||
} | ||||||
if (countOfSuccessfulRequests >= 20) { | ||||||
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", "maxAthenaResultsReached", true); | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -110,6 +110,7 @@ export class ExerciseDetailsStudentActionsComponent implements OnInit, OnChanges | |||||||||||||||||||
this.profileService.getProfileInfo().subscribe((profileInfo) => { | ||||||||||||||||||||
this.localVCEnabled = profileInfo.activeProfiles?.includes(PROFILE_LOCALVC); | ||||||||||||||||||||
this.athenaEnabled = profileInfo.activeProfiles?.includes(PROFILE_ATHENA); | ||||||||||||||||||||
|
||||||||||||||||||||
// The online IDE is only available with correct SpringProfile and if it's enabled for this exercise | ||||||||||||||||||||
if (profileInfo.activeProfiles?.includes(PROFILE_THEIA) && this.programmingExercise) { | ||||||||||||||||||||
this.theiaEnabled = true; | ||||||||||||||||||||
|
@@ -135,9 +136,6 @@ export class ExerciseDetailsStudentActionsComponent implements OnInit, OnChanges | |||||||||||||||||||
}); | ||||||||||||||||||||
} else if (this.exercise.type === ExerciseType.MODELING) { | ||||||||||||||||||||
this.editorLabel = 'openModelingEditor'; | ||||||||||||||||||||
this.profileService.getProfileInfo().subscribe((profileInfo) => { | ||||||||||||||||||||
this.athenaEnabled = profileInfo.activeProfiles?.includes(PROFILE_ATHENA); | ||||||||||||||||||||
}); | ||||||||||||||||||||
} else if (this.exercise.type === ExerciseType.TEXT) { | ||||||||||||||||||||
this.editorLabel = 'openTextEditor'; | ||||||||||||||||||||
this.profileService.getProfileInfo().subscribe((profileInfo) => { | ||||||||||||||||||||
|
@@ -257,6 +255,7 @@ export class ExerciseDetailsStudentActionsComponent implements OnInit, OnChanges | |||||||||||||||||||
}); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
// this method will be removed once text and modeling support new component | ||||||||||||||||||||
requestFeedback() { | ||||||||||||||||||||
if (!this.assureConditionsSatisfied()) return; | ||||||||||||||||||||
if (this.exercise.type === ExerciseType.PROGRAMMING) { | ||||||||||||||||||||
|
@@ -341,6 +340,7 @@ export class ExerciseDetailsStudentActionsComponent implements OnInit, OnChanges | |||||||||||||||||||
* 3. There is no already pending feedback request. | ||||||||||||||||||||
* @returns {boolean} `true` if all conditions are satisfied, otherwise `false`. | ||||||||||||||||||||
*/ | ||||||||||||||||||||
// this method will be removed once text and modeling support new component | ||||||||||||||||||||
assureConditionsSatisfied(): boolean { | ||||||||||||||||||||
this.updateParticipations(); | ||||||||||||||||||||
if (this.exercise.type === ExerciseType.PROGRAMMING) { | ||||||||||||||||||||
|
@@ -388,27 +388,12 @@ export class ExerciseDetailsStudentActionsComponent implements OnInit, OnChanges | |||||||||||||||||||
|
||||||||||||||||||||
hasAthenaResultForlatestSubmission(): boolean { | ||||||||||||||||||||
if (this.gradedParticipation?.submissions && this.gradedParticipation?.results) { | ||||||||||||||||||||
const sortedSubmissions = this.gradedParticipation.submissions.slice().sort((a, b) => { | ||||||||||||||||||||
const dateA = this.getDateValue(a.submissionDate) ?? -Infinity; | ||||||||||||||||||||
const dateB = this.getDateValue(b.submissionDate) ?? -Infinity; | ||||||||||||||||||||
return dateB - dateA; | ||||||||||||||||||||
}); | ||||||||||||||||||||
|
||||||||||||||||||||
return this.gradedParticipation.results.some((result) => result.submission?.id === sortedSubmissions[0]?.id); | ||||||||||||||||||||
// submissions.results is always undefined so this is neccessary | ||||||||||||||||||||
return ( | ||||||||||||||||||||
this.gradedParticipation.submissions.last()?.id === | ||||||||||||||||||||
this.gradedParticipation?.results.filter((result) => result.assessmentType == AssessmentType.AUTOMATIC_ATHENA).first()?.submission?.id | ||||||||||||||||||||
); | ||||||||||||||||||||
Comment on lines
+391
to
+395
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential runtime errors due to non-standard JavaScript arrays do not have Apply this diff to fix the issue: -return (
- this.gradedParticipation.submissions.last()?.id ===
- this.gradedParticipation?.results.filter((result) => result.assessmentType == AssessmentType.AUTOMATIC_ATHENA).first()?.submission?.id
-);
+const latestSubmissionId = this.gradedParticipation.submissions?.[this.gradedParticipation.submissions.length - 1]?.id;
+const firstAthenaResultSubmissionId = this.gradedParticipation?.results.filter((result) => result.assessmentType === AssessmentType.AUTOMATIC_ATHENA)?.[0]?.submission?.id;
+return latestSubmissionId === firstAthenaResultSubmissionId; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
} | ||||||||||||||||||||
return false; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
private getDateValue = (date: any): number => { | ||||||||||||||||||||
if (dayjs.isDayjs(date)) { | ||||||||||||||||||||
return date.valueOf(); | ||||||||||||||||||||
} | ||||||||||||||||||||
if (date instanceof Date) { | ||||||||||||||||||||
return date.valueOf(); | ||||||||||||||||||||
} | ||||||||||||||||||||
if (typeof date === 'string') { | ||||||||||||||||||||
return new Date(date).valueOf(); | ||||||||||||||||||||
} | ||||||||||||||||||||
return -Infinity; // fallback for null, undefined, or invalid dates | ||||||||||||||||||||
}; | ||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
@if (!isExamExercise) { | ||
@if (athenaEnabled) { | ||
@if (exercise().type === ExerciseType.TEXT) { | ||
<button | ||
class="btn btn-primary" | ||
(click)="requestFeedback()" | ||
[class.btn-sm]="smallButtons()" | ||
[disabled]="!participation?.submissions?.last()?.submitted || isGeneratingFeedback()" | ||
[id]="'request-feedback-' + exercise().id" | ||
[ngbTooltip]="'artemisApp.exerciseActions.requestAutomaticFeedbackTooltip' | artemisTranslate" | ||
> | ||
<fa-icon [icon]="faPenSquare" [fixedWidth]="true" /> | ||
<span class="d-none d-md-inline" jhiTranslate="artemisApp.exerciseActions.requestAutomaticFeedback"></span> | ||
</button> | ||
} @else { | ||
<a | ||
class="btn btn-primary" | ||
(click)="requestFeedback()" | ||
[class.btn-sm]="smallButtons()" | ||
[id]="'request-feedback-' + exercise().id" | ||
[ngbTooltip]="'artemisApp.exerciseActions.requestAutomaticFeedbackTooltip' | artemisTranslate" | ||
> | ||
<fa-icon [icon]="faPenSquare" [fixedWidth]="true" /> | ||
<span class="d-none d-md-inline" jhiTranslate="artemisApp.exerciseActions.requestAutomaticFeedback"></span> | ||
</a> | ||
undernagruzez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} @else { | ||
<a | ||
class="btn btn-primary" | ||
(click)="requestFeedback()" | ||
[class.btn-sm]="smallButtons()" | ||
[id]="'request-feedback-' + exercise().id" | ||
[ngbTooltip]="'artemisApp.exerciseActions.requestManualFeedbackTooltip' | artemisTranslate" | ||
> | ||
<fa-icon [icon]="faPenSquare" [fixedWidth]="true" /> | ||
<span class="d-none d-md-inline" jhiTranslate="artemisApp.exerciseActions.requestManualFeedback"></span> | ||
</a> | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using
AccessForbiddenException
instead ofBadRequestAlertException
Requesting feedback after the due date is a forbidden action. Using
AccessForbiddenException
, which results in a 403 Forbidden status code, would more accurately represent the access restriction compared toBadRequestAlertException
(400 Bad Request).