Skip to content

Commit

Permalink
Programming exercises: Fix issues with hints (ls1intum#4260)
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan Krusche authored Nov 6, 2021
1 parent d604b8e commit daa4e15
Show file tree
Hide file tree
Showing 16 changed files with 132 additions and 146 deletions.
2 changes: 1 addition & 1 deletion docs/admin/accessRights.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ General Functionality
+-------------------------+------------+--------+--------------------+---------+
| Scores |||| |
+-------------------------+------------+--------+--------------------+---------+
| Participations |||| |
| Participation |||| |
+-------------------------+------------+--------+--------------------+---------+
| Submissions ||| | |
+-------------------------+------------+--------+--------------------+---------+
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package de.tum.in.www1.artemis.web.rest;

import static de.tum.in.www1.artemis.web.rest.util.ResponseUtil.*;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.Set;
Expand All @@ -19,6 +17,8 @@
import de.tum.in.www1.artemis.repository.*;
import de.tum.in.www1.artemis.security.Role;
import de.tum.in.www1.artemis.service.AuthorizationCheckService;
import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenException;
import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException;
import tech.jhipster.web.util.HeaderUtil;

/**
Expand Down Expand Up @@ -59,20 +59,20 @@ public ExerciseHintResource(ExerciseHintRepository exerciseHintRepository, Autho
* @throws URISyntaxException if the Location URI syntax is incorrect.
*/
@PostMapping("/exercise-hints")
@PreAuthorize("hasRole('TA')")
@PreAuthorize("hasRole('EDITOR')")
public ResponseEntity<ExerciseHint> createExerciseHint(@RequestBody ExerciseHint exerciseHint) throws URISyntaxException {
log.debug("REST request to save ExerciseHint : {}", exerciseHint);
if (exerciseHint.getExercise() == null) {
return badRequest();
throw new BadRequestAlertException("An exercise hint can only be created if the exercise is defined", ENTITY_NAME, "idnull");
}
// Reload the exercise from the database as we can't trust data from the client
Exercise exercise = exerciseRepository.findByIdElseThrow(exerciseHint.getExercise().getId());

// Hints for exam exercises are not supported at the moment
if (exercise.isExamExercise()) {
return forbidden();
throw new AccessForbiddenException("Exercise hints for exams are currently not supported");
}
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, exercise, null);
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, exercise, null);
ExerciseHint result = exerciseHintRepository.save(exerciseHint);
return ResponseEntity.created(new URI("/api/exercise-hints/" + result.getId()))
.headers(HeaderUtil.createEntityCreationAlert(applicationName, true, ENTITY_NAME, result.getId().toString())).body(result);
Expand All @@ -88,23 +88,22 @@ public ResponseEntity<ExerciseHint> createExerciseHint(@RequestBody ExerciseHint
* or with status {@code 500 (Internal Server Error)} if the exerciseHint couldn't be updated.
*/
@PutMapping("/exercise-hints/{exerciseHintId}")
@PreAuthorize("hasRole('TA')")
@PreAuthorize("hasRole('EDITOR')")
public ResponseEntity<ExerciseHint> updateExerciseHint(@RequestBody ExerciseHint exerciseHint, @PathVariable Long exerciseHintId) {
log.debug("REST request to update ExerciseHint : {}", exerciseHint);
if (exerciseHint.getId() == null || !exerciseHintId.equals(exerciseHint.getId()) || exerciseHint.getExercise() == null) {
return badRequest();
throw new BadRequestAlertException("An exercise hint can only be changed if it has an ID and if the exercise is not null", ENTITY_NAME, "idnull");
}
var hintBeforeSaving = exerciseHintRepository.findByIdElseThrow(exerciseHintId);
// Reload the exercise from the database as we can't trust data from the client
Exercise exercise = exerciseRepository.findByIdElseThrow(exerciseHint.getExercise().getId());

// Hints for exam exercises are not supported at the moment
if (exercise.isExamExercise()) {
return forbidden();
}
if (!authCheckService.isAtLeastTeachingAssistantForExercise(exercise) || !authCheckService.isAtLeastTeachingAssistantForExercise(hintBeforeSaving.getExercise())) {
return forbidden();
throw new AccessForbiddenException("Exercise hints for exams are currently not supported");
}
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, hintBeforeSaving.getExercise(), null);
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, exerciseHint.getExercise(), null);
ExerciseHint result = exerciseHintRepository.save(exerciseHint);
return ResponseEntity.ok().headers(HeaderUtil.createEntityUpdateAlert(applicationName, true, ENTITY_NAME, exerciseHint.getId().toString())).body(result);
}
Expand All @@ -129,14 +128,11 @@ public ResponseEntity<String> getHintTitle(@PathVariable Long hintId) {
* @return the {@link ResponseEntity} with status {@code 200 (OK)} and with body the exerciseHint, or with status {@code 404 (Not Found)}.
*/
@GetMapping("/exercise-hints/{exerciseHintId}")
@PreAuthorize("hasRole('TA')")
@PreAuthorize("hasRole('EDITOR')")
public ResponseEntity<ExerciseHint> getExerciseHint(@PathVariable Long exerciseHintId) {
log.debug("REST request to get ExerciseHint : {}", exerciseHintId);
var exerciseHint = exerciseHintRepository.findByIdElseThrow(exerciseHintId);

if (!authCheckService.isAtLeastTeachingAssistantForExercise(exerciseHint.getExercise())) {
return forbidden();
}
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, exerciseHint.getExercise(), null);
return ResponseEntity.ok().body(exerciseHint);
}

Expand All @@ -163,11 +159,11 @@ public ResponseEntity<Set<ExerciseHint>> getExerciseHintsForExercise(@PathVariab
* @return the {@link ResponseEntity} with status {@code 204 (NO_CONTENT)}.
*/
@DeleteMapping("/exercise-hints/{exerciseHintId}")
@PreAuthorize("hasRole('TA')")
@PreAuthorize("hasRole('EDITOR')")
public ResponseEntity<Void> deleteExerciseHint(@PathVariable Long exerciseHintId) {
log.debug("REST request to delete ExerciseHint : {}", exerciseHintId);
var exerciseHint = exerciseHintRepository.findByIdElseThrow(exerciseHintId);
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, exerciseHint.getExercise(), null);
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, exerciseHint.getExercise(), null);
exerciseHintRepository.deleteById(exerciseHintId);
return ResponseEntity.noContent().headers(HeaderUtil.createEntityDeletionAlert(applicationName, true, ENTITY_NAME, exerciseHintId.toString())).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { OrganizationManagementService } from 'app/admin/organization-management
import { Organization } from 'app/entities/organization.model';

@Injectable({ providedIn: 'root' })
export class OrganizationMgmtResolve implements Resolve<any> {
export class OrganizationMgmtResolve implements Resolve<Organization> {
constructor(private organizationManagementService: OrganizationManagementService) {}

resolve(route: ActivatedRouteSnapshot) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import { SystemNotification } from 'app/entities/system-notification.model';
import { SystemNotificationService } from 'app/shared/notification/system-notification/system-notification.service';
import { SystemNotificationManagementComponent } from 'app/admin/system-notification-management/system-notification-management.component';
import { SystemNotificationManagementDetailComponent } from 'app/admin/system-notification-management/system-notification-management-detail.component';
import { HttpResponse } from '@angular/common/http';
import { filter, map } from 'rxjs/operators';

@Injectable({ providedIn: 'root' })
export class SystemNotificationManagementResolve implements Resolve<any> {
export class SystemNotificationManagementResolve implements Resolve<SystemNotification> {
constructor(private service: SystemNotificationService) {}

/**
Expand All @@ -16,7 +18,10 @@ export class SystemNotificationManagementResolve implements Resolve<any> {
*/
resolve(route: ActivatedRouteSnapshot) {
if (route.params['id']) {
return this.service.find(parseInt(route.params['id'], 10));
return this.service.find(parseInt(route.params['id'], 10)).pipe(
filter((response: HttpResponse<SystemNotification>) => response.ok),
map((response: HttpResponse<SystemNotification>) => response.body!),
);
}
return new SystemNotification();
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/app/exam/manage/exam-management.route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class ExamResolve implements Resolve<Exam> {
if (courseId && examId) {
return this.examManagementService.find(courseId, examId, withStudents, withExerciseGroups).pipe(
filter((response: HttpResponse<Exam>) => response.ok),
map((exam: HttpResponse<Exam>) => exam.body!),
map((response: HttpResponse<Exam>) => response.body!),
);
}
return of(new Exam());
Expand Down Expand Up @@ -131,7 +131,7 @@ export class StudentExamResolve implements Resolve<StudentExam> {
if (courseId && examId && studentExamId) {
return this.studentExamService.find(courseId, examId, studentExamId).pipe(
filter((response: HttpResponse<StudentExam>) => response.ok),
map((studentExam: HttpResponse<StudentExam>) => studentExam.body!),
map((response: HttpResponse<StudentExam>) => response.body!),
);
}
return of(new StudentExam());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { Result } from 'app/entities/result.model';
})
export class ProgrammingExerciseEditableInstructionComponent implements AfterViewInit, OnChanges, OnDestroy {
participationValue: Participation;
exerciseValue: ProgrammingExercise;
programmingExercise: ProgrammingExercise;

exerciseTestCases: string[] = [];
exerciseHints: ExerciseHint[];
Expand Down Expand Up @@ -62,7 +62,7 @@ export class ProgrammingExerciseEditableInstructionComponent implements AfterVie
@Input() forceRender: Observable<void>;
@Input()
get exercise() {
return this.exerciseValue;
return this.programmingExercise;
}
@Input()
get participation() {
Expand All @@ -80,11 +80,18 @@ export class ProgrammingExerciseEditableInstructionComponent implements AfterVie
}

set exercise(exercise: ProgrammingExercise) {
if (this.exerciseValue && exercise.problemStatement !== this.exerciseValue.problemStatement) {
if (this.programmingExercise && exercise.problemStatement !== this.programmingExercise.problemStatement) {
this.unsavedChanges = true;
}
this.exerciseValue = exercise;
this.exerciseChange.emit(this.exerciseValue);
this.programmingExercise = exercise;
const isExamExercise = !!this.programmingExercise.exerciseGroup;
if (isExamExercise) {
// Note: Exam exercises do not include hints at the moment, therefore, the markdown editor should not offer this command
this.domainCommands = [this.katexCommand, this.taskCommand, this.testCaseCommand];
} else {
this.domainCommands = [this.katexCommand, this.taskCommand, this.testCaseCommand, this.taskHintCommand];
}
this.exerciseChange.emit(this.programmingExercise);
}

set unsavedChanges(hasChanges: boolean) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ <h2 id="jhi-exercise-hint-heading" jhiTranslate="artemisApp.exerciseHint.home.cr
</div>
<div class="form-group">
<label class="form-control-label" jhiTranslate="artemisApp.exerciseHint.title" for="field_title">Title</label>
<input type="text" required minlength="5" class="form-control" name="title" id="field_title" [(ngModel)]="exerciseHint.title" />
<input type="text" required minlength="3" class="form-control" name="title" id="field_title" [(ngModel)]="exerciseHint.title" />
</div>
<div class="form-group hint-form__editor-wrapper">
<jhi-markdown-editor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ import { Authority } from 'app/shared/constants/authority.constants';
import { exerciseTypes } from 'app/entities/exercise.model';

@Injectable({ providedIn: 'root' })
export class ExerciseHintResolve implements Resolve<ExerciseHint | null> {
export class ExerciseHintResolve implements Resolve<ExerciseHint> {
constructor(private service: ExerciseHintService) {}

/**
* Resolves the route into an exercise hint id and fetches it from the server
* @param route Route which to resolve
*/
resolve(route: ActivatedRouteSnapshot): Observable<ExerciseHint | null> {
resolve(route: ActivatedRouteSnapshot) {
const id = route.params['hintId'] ? route.params['hintId'] : undefined;
if (id) {
return this.service.find(id).pipe(
filter((response: HttpResponse<ExerciseHint>) => response.ok),
map((exerciseHint: HttpResponse<ExerciseHint>) => exerciseHint.body),
map((exerciseHint: HttpResponse<ExerciseHint>) => exerciseHint.body!),
);
}
return of(new ExerciseHint());
Expand All @@ -41,7 +41,7 @@ export const exerciseHintRoute: Routes = [
exerciseHint: ExerciseHintResolve,
},
data: {
authorities: [Authority.ADMIN],
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'artemisApp.exerciseHint.home.title',
},
canActivate: [UserRouteAccessService],
Expand All @@ -55,7 +55,7 @@ export const exerciseHintRoute: Routes = [
exerciseHint: ExerciseHintResolve,
},
data: {
authorities: [Authority.ADMIN],
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'artemisApp.exerciseHint.home.title',
},
canActivate: [UserRouteAccessService],
Expand All @@ -69,7 +69,7 @@ export const exerciseHintRoute: Routes = [
exerciseHint: ExerciseHintResolve,
},
data: {
authorities: [Authority.ADMIN],
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'artemisApp.exerciseHint.home.title',
},
canActivate: [UserRouteAccessService],
Expand All @@ -80,7 +80,7 @@ export const exerciseHintRoute: Routes = [
path: ':courseId/' + exerciseType + '-exercises/:exerciseId/hints',
component: ExerciseHintComponent,
data: {
authorities: [Authority.ADMIN],
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'artemisApp.exerciseHint.home.title',
},
canActivate: [UserRouteAccessService],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { HttpClient, HttpResponse } from '@angular/common/http';
import { Observable } from 'rxjs';

import { ExerciseHint } from 'app/entities/exercise-hint.model';
import { ExerciseService } from 'app/exercises/shared/exercise/exercise.service';

export type ExerciseHintResponse = HttpResponse<ExerciseHint>;

Expand Down Expand Up @@ -35,20 +36,24 @@ export interface IExerciseHintService {
* Deletes an exercise hint
* @param id Id of exercise hint to delete
*/
delete(id: number): Observable<HttpResponse<any>>;
delete(id: number): Observable<HttpResponse<void>>;
}

@Injectable({ providedIn: 'root' })
export class ExerciseHintService implements IExerciseHintService {
public resourceUrl = SERVER_API_URL + 'api/exercise-hints';

constructor(protected http: HttpClient) {}
constructor(protected http: HttpClient, private exerciseService: ExerciseService) {}

/**
* Creates an exercise hint
* @param exerciseHint Exercise hint to create
*/
create(exerciseHint: ExerciseHint): Observable<ExerciseHintResponse> {
exerciseHint.exercise = this.exerciseService.convertDateFromClient(exerciseHint.exercise!);
if (exerciseHint.exercise.categories) {
exerciseHint.exercise.categories = this.exerciseService.stringifyExerciseCategories(exerciseHint.exercise);
}
return this.http.post<ExerciseHint>(this.resourceUrl, exerciseHint, { observe: 'response' });
}

Expand All @@ -57,15 +62,17 @@ export class ExerciseHintService implements IExerciseHintService {
* @param exerciseHint Exercise hint to update
*/
update(exerciseHint: ExerciseHint): Observable<ExerciseHintResponse> {
exerciseHint.exercise = this.exerciseService.convertDateFromClient(exerciseHint.exercise!);
exerciseHint.exercise.categories = this.exerciseService.stringifyExerciseCategories(exerciseHint.exercise);
return this.http.put<ExerciseHint>(`${this.resourceUrl}/${exerciseHint.id}`, exerciseHint, { observe: 'response' });
}

/**
* Finds an exercise hint
* @param id Id of exercise hint to find
* @param exerciseHintId Id of exercise hint to find
*/
find(id: number): Observable<ExerciseHintResponse> {
return this.http.get<ExerciseHint>(`${this.resourceUrl}/${id}`, { observe: 'response' });
find(exerciseHintId: number): Observable<ExerciseHintResponse> {
return this.http.get<ExerciseHint>(`${this.resourceUrl}/${exerciseHintId}`, { observe: 'response' });
}

/**
Expand All @@ -79,18 +86,18 @@ export class ExerciseHintService implements IExerciseHintService {
/**
* Fetches the title of the hint with the given id
*
* @param hintId the id of the hint
* @param exerciseHintId the id of the hint
* @return the title of the hint in an HttpResponse, or an HttpErrorResponse on error
*/
getTitle(hintId: number): Observable<HttpResponse<string>> {
return this.http.get(`${this.resourceUrl}/${hintId}/title`, { observe: 'response', responseType: 'text' });
getTitle(exerciseHintId: number): Observable<HttpResponse<string>> {
return this.http.get(`${this.resourceUrl}/${exerciseHintId}/title`, { observe: 'response', responseType: 'text' });
}

/**
* Deletes an exercise hint
* @param id Id of exercise hint to delete
* @param exerciseHintId Id of exercise hint to delete
*/
delete(id: number): Observable<HttpResponse<any>> {
return this.http.delete<any>(`${this.resourceUrl}/${id}`, { observe: 'response' });
delete(exerciseHintId: number): Observable<HttpResponse<void>> {
return this.http.delete<void>(`${this.resourceUrl}/${exerciseHintId}`, { observe: 'response' });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ export class ExerciseService {
}

/**
* Parsses the exercise categories JSON string into ExerciseCategory objects.
* @param exercise - the exericse
* Parses the exercise categories JSON string into ExerciseCategory objects.
* @param exercise - the exercise
*/
parseExerciseCategories(exercise: Exercise) {
if (exercise.categories) {
Expand All @@ -335,7 +335,7 @@ export class ExerciseService {
* @param { string[] } categories that are converted to categories
*/
convertExerciseCategoriesAsStringFromServer(categories: string[]): ExerciseCategory[] {
return categories.map((el) => JSON.parse(el));
return categories.map((category) => JSON.parse(category));
}

/**
Expand Down
Loading

0 comments on commit daa4e15

Please sign in to comment.