Skip to content

Commit ee6d6e3

Browse files
committed
Programming exercises: Avoid unnecessary not authorized alert for instructors when using edit in editor
1 parent 451bd2a commit ee6d6e3

File tree

6 files changed

+25
-24
lines changed

6 files changed

+25
-24
lines changed

src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryProgrammingExerciseParticipationResource.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
44

5-
import java.util.ArrayList;
65
import java.util.List;
76
import java.util.Map;
87
import java.util.Objects;
@@ -406,18 +405,17 @@ public ResponseEntity<RepositoryStatusDTO> getStatus(@PathVariable Long particip
406405
}
407406

408407
/**
409-
* GET /repository/:participationId/buildlogs : get the build log for the "participationId" repository.
408+
* GET /participations/:participationId/buildlogs : get the build log for the "participationId" repository.
410409
*
411410
* @param participationId to identify the repository with.
412411
* @param resultId an optional result ID to get the build logs for the submission that the result belongs to. If the result ID is not specified, the latest submission is
413412
* used.
414413
* @return the ResponseEntity with status 200 (OK) and with body the result, or with status 404 (Not Found)
415414
*/
416-
// TODO: rename to participation/{participationId}/buildlogs
417-
@GetMapping(value = "repository/{participationId}/buildlogs", produces = MediaType.APPLICATION_JSON_VALUE)
415+
@GetMapping(value = "participations/{participationId}/buildlogs", produces = MediaType.APPLICATION_JSON_VALUE)
418416
@EnforceAtLeastStudent
419417
public ResponseEntity<List<BuildLogEntry>> getBuildLogs(@PathVariable Long participationId, @RequestParam(name = "resultId") Optional<Long> resultId) {
420-
log.debug("REST request to get build log : {}", participationId);
418+
log.debug("REST request to get build logs for participation {}", participationId);
421419

422420
ProgrammingExerciseParticipation participation = participationService.findProgrammingExerciseParticipationWithLatestSubmissionAndResult(participationId);
423421
participationAuthCheckService.checkCanAccessParticipationElseThrow(participation);
@@ -436,16 +434,16 @@ public ResponseEntity<List<BuildLogEntry>> getBuildLogs(@PathVariable Long parti
436434
}
437435
else if (programmingSubmission == null) {
438436
// Can't return build logs if a submission doesn't exist yet
439-
return ResponseEntity.ok(new ArrayList<>());
437+
return ResponseEntity.ok(List.of());
440438
}
441439

442-
// Do not return build logs if the build hasn't failed
440+
// Empty build logs are returned if the submission is not build failed
443441
if (!programmingSubmission.isBuildFailed()) {
444-
throw new AccessForbiddenException("Build logs cannot be retrieved when the build hasn't failed!");
442+
return ResponseEntity.ok(List.of());
445443
}
446444

447445
// Load the logs from the database
448446
List<BuildLogEntry> buildLogs = buildLogService.getLatestBuildLogs(programmingSubmission);
449-
return new ResponseEntity<>(buildLogs, HttpStatus.OK);
447+
return ResponseEntity.ok(buildLogs);
450448
}
451449
}

src/main/webapp/app/programming/shared/services/build-log.service.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Build Log Service', () => {
88
let service: BuildLogService;
99
let httpMock: HttpTestingController;
1010

11-
const resourceUrl = 'api/programming/repository/42/buildlogs';
11+
const resourceUrl = 'api/programming/participations/42/buildlogs';
1212

1313
beforeEach(() => {
1414
TestBed.configureTestingModule({

src/main/webapp/app/programming/shared/services/build-log.service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export interface IBuildLogService {
1111
export class BuildLogService implements IBuildLogService {
1212
private http = inject(HttpClient);
1313

14-
private assignmentResourceUrl = 'api/programming/repository';
14+
private participationsResourceUrl = 'api/programming/participations';
1515

1616
/**
1717
* Retrieves the build logs for a given participation and optionally, a given result.
@@ -23,6 +23,6 @@ export class BuildLogService implements IBuildLogService {
2323
if (resultId) {
2424
params = params.set('resultId', resultId);
2525
}
26-
return this.http.get<BuildLogEntry[]>(`${this.assignmentResourceUrl}/${participationId}/buildlogs`, { params });
26+
return this.http.get<BuildLogEntry[]>(`${this.participationsResourceUrl}/${participationId}/buildlogs`, { params });
2727
}
2828
}

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultLocalVCJenkinsIntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ private Result assertBuildError(Long participationId, String userLogin) throws E
195195

196196
userUtilService.changeUser(userLogin);
197197
// Assert that the build logs can be retrieved from the REST API from the database
198-
var receivedLogs = request.get("/api/programming/repository/" + participationId + "/buildlogs", HttpStatus.OK, List.class);
198+
var receivedLogs = request.get("/api/programming/participations/" + participationId + "/buildlogs", HttpStatus.OK, List.class);
199199
assertThat(receivedLogs).isNotNull().isNotEmpty();
200200

201201
return result;

src/test/java/de/tum/cit/aet/artemis/programming/RepositoryIntegrationTest.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ class RepositoryIntegrationTest extends AbstractProgrammingIntegrationLocalCILoc
118118

119119
private final String studentRepoBaseUrl = "/api/programming/repository/";
120120

121+
private final String participationsBaseUrl = "/api/programming/participations/";
122+
121123
private final String filesContentBaseUrl = "/api/programming/repository-files-content/";
122124

123125
private ProgrammingExercise programmingExercise;
@@ -942,15 +944,16 @@ void testGetStatus() throws Exception {
942944
@Test
943945
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
944946
void testBuildLogsNoSubmission() throws Exception {
945-
var receivedLogs = request.get(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, List.class);
947+
var receivedLogs = request.get(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, List.class);
946948
assertThat(receivedLogs).isNotNull().isEmpty();
947949
}
948950

949951
@Test
950952
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
951953
void testBuildLogsWithSubmissionBuildSuccessful() throws Exception {
952954
programmingExerciseUtilService.createProgrammingSubmission(participation, false);
953-
request.get(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.FORBIDDEN, List.class);
955+
var buildLogs = request.get(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, List.class);
956+
assertThat(buildLogs).isEmpty();
954957
}
955958

956959
@Test
@@ -960,7 +963,7 @@ void testBuildLogsWithManualResult() throws Exception {
960963
var buildLogEntries = buildLogEntryService.saveBuildLogs(logs, submission);
961964
submission.setBuildLogEntries(buildLogEntries);
962965
participationUtilService.addResultToSubmission(submission, AssessmentType.SEMI_AUTOMATIC);
963-
var receivedLogs = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
966+
var receivedLogs = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
964967
assertThat(receivedLogs).hasSize(2);
965968
assertLogsContent(receivedLogs);
966969
}
@@ -972,7 +975,7 @@ void testBuildLogs() throws Exception {
972975
var buildLogEntries = buildLogEntryService.saveBuildLogs(logs, submission);
973976
submission.setBuildLogEntries(buildLogEntries);
974977
participationUtilService.addResultToSubmission(submission, AssessmentType.AUTOMATIC);
975-
var receivedLogs = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
978+
var receivedLogs = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
976979
assertThat(receivedLogs).hasSize(2);
977980
assertLogsContent(receivedLogs);
978981
}
@@ -981,7 +984,7 @@ void testBuildLogs() throws Exception {
981984
@WithMockUser(username = TEST_PREFIX + "student2", roles = "USER")
982985
void testGetBuildLogs_cannotAccessParticipation() throws Exception {
983986
// student2 should not have access to student1's participation.
984-
request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.FORBIDDEN, BuildLogEntry.class);
987+
request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.FORBIDDEN, BuildLogEntry.class);
985988
}
986989

987990
private void assertLogsContent(List<BuildLogEntry> receivedLogs) {
@@ -1012,7 +1015,7 @@ void testBuildLogsFromDatabase() throws Exception {
10121015
submission.setBuildLogEntries(buildLogEntries);
10131016
programmingExerciseUtilService.addProgrammingSubmission(programmingExercise, submission, TEST_PREFIX + "student1");
10141017

1015-
var receivedLogs = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
1018+
var receivedLogs = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
10161019
assertThat(receivedLogs).hasSize(3).isEqualTo(buildLogEntries);
10171020
}
10181021

@@ -1052,17 +1055,17 @@ void testBuildLogsFromDatabaseForSpecificResults() throws Exception {
10521055
var result2 = participationUtilService.addResultToSubmission(submission2, AssessmentType.AUTOMATIC).getFirstResult();
10531056

10541057
// Specify to use result1
1055-
var receivedLogs1 = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class,
1058+
var receivedLogs1 = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class,
10561059
parameters(Map.of("resultId", result1.getId())));
10571060
assertThat(receivedLogs1).isEqualTo(submission1Logs);
10581061

10591062
// Specify to use result2
1060-
var receivedLogs2 = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class,
1063+
var receivedLogs2 = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class,
10611064
parameters(Map.of("resultId", result2.getId())));
10621065
assertThat(receivedLogs2).isEqualTo(submission2Logs);
10631066

10641067
// Without parameters, the latest submission must be used
1065-
var receivedLogsLatest = request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
1068+
var receivedLogsLatest = request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.OK, BuildLogEntry.class);
10661069
assertThat(receivedLogsLatest).isEqualTo(submission2Logs);
10671070
}
10681071

@@ -1072,7 +1075,7 @@ void testBuildLogsFromDatabaseForSpecificResults_otherParticipation() throws Exc
10721075
var result = participationUtilService.addProgrammingParticipationWithResultForExercise(programmingExercise, TEST_PREFIX + "tutor1");
10731076
programmingExerciseUtilService.addProgrammingSubmissionToResultAndParticipation(result, (StudentParticipation) result.getSubmission().getParticipation(), "xyz");
10741077

1075-
request.getList(studentRepoBaseUrl + participation.getId() + "/buildlogs", HttpStatus.FORBIDDEN, BuildLogEntry.class, parameters(Map.of("resultId", result.getId())));
1078+
request.getList(participationsBaseUrl + participation.getId() + "/buildlogs", HttpStatus.FORBIDDEN, BuildLogEntry.class, parameters(Map.of("resultId", result.getId())));
10761079
}
10771080

10781081
@Disabled

src/test/java/de/tum/cit/aet/artemis/programming/RepositoryProgrammingExerciseParticipationJenkinsIntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ void testGetLatestBuildLogsFails() throws Exception {
6363
jenkinsRequestMockProvider.mockGetJob(programmingExercise.getProjectKey(), buildPlanId, jobWithDetails, false);
6464
jenkinsRequestMockProvider.mockGetBuildStatus(programmingExercise.getProjectKey(), buildPlanId, true, true, false, true);
6565

66-
var url = "/api/programming/repository/" + programmingExerciseParticipation.getId() + "/buildlogs";
66+
var url = "/api/programming/participations/" + programmingExerciseParticipation.getId() + "/buildlogs";
6767
var buildLogs = request.getList(url, HttpStatus.OK, BuildLogEntry.class);
6868
assertThat(buildLogs).hasSize(3);
6969
}

0 commit comments

Comments
 (0)