Skip to content

Commit dc08d50

Browse files
Bug fix in exercise addition
A bug has been detected in which the templates and solutions of the new exercises are not added correctly, since all the promises for the upload were introduced "all at once" as simultaneous requests to the server, which was saturated. As a consequence, this is solved by sending the requests sequentially (one at a time).
1 parent b743f1b commit dc08d50

File tree

3 files changed

+38
-26
lines changed

3 files changed

+38
-26
lines changed

vscode4teaching-extension/resources/dashboard/dashboard.css

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,15 @@ section.generalStatistics > .row > .col#col-2 > .rowTotals {
8484
margin-bottom: 0.5rem;
8585
}
8686

87+
section.generalStatistics > .row > .col#col-2 .label {
88+
text-align: center;
89+
padding-right: 0.25rem;
90+
}
91+
92+
8793
section.generalStatistics > .row > .col#col-2 .value {
8894
font-weight: 700;
89-
font-size: 1.75rem;
95+
font-size: 1.5rem;
9096
}
9197

9298
section.generalStatistics > .row > .col#col-2 > .rowStatus {
@@ -148,7 +154,7 @@ section.generalStatistics > .row > .col#col-3 > .rowTime:last-child {
148154

149155
section.generalStatistics > .row > .col#col-3 .value {
150156
font-weight: 700;
151-
font-size: 1.75rem;
157+
font-size: 1.5rem;
152158
}
153159

154160
/**

vscode4teaching-extension/src/components/courses/CoursesTreeProvider.ts

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -323,26 +323,26 @@ export class CoursesProvider implements vscode.TreeDataProvider<V4TItem> {
323323
allowEditionAfterSolutionDownloaded: false
324324
})));
325325

326-
// 3.2: an array containing promises for sending ZIP compressed files including all the templates and solutions extracted during the previous phases is generated.
327-
const uploadFilesPromises = await Promise.all(exerciseData.data.map(async (ex, index) =>
328-
[
329-
APIClient.uploadExerciseTemplate(ex.id, await FileZipUtil.getZipFromUris([exercisesDirectories[index].paths.template]), false),
330-
(exercisesDirectories[index].paths.solution) ? APIClient.uploadExerciseSolution(ex.id, await FileZipUtil.getZipFromUris([exercisesDirectories[index].paths.solution!]), false) : undefined
331-
]));
332-
333-
// 3.3: all registered requests (as promises in previous step) are sent and, once completed, the received results are processed.
334-
Promise.all(uploadFilesPromises.flat())
335-
.catch(async (uploadError) => {
336-
// If any upload process fails, all exercises are deleted from database and error is handled (via errorCaught)
337-
errorCaught = true;
338-
v4tLogger.error(uploadError);
339-
try {
340-
exerciseData.data.forEach(async ex => await APIClient.deleteExercise(ex.id));
341-
APIClient.handleAxiosError(uploadError);
342-
} catch (deleteError) {
343-
APIClient.handleAxiosError(deleteError);
344-
}
345-
}).finally(() => {
326+
// 3.2: promises are sent to server in order and one at a time due to performance reasons.
327+
try {
328+
for (const [index, exercise] of exerciseData.data.entries()) {
329+
await APIClient.uploadExerciseTemplate(exercise.id, await FileZipUtil.getZipFromUris([exercisesDirectories[index].paths.template]), false);
330+
331+
if (exercise.includesTeacherSolution) {
332+
await APIClient.uploadExerciseSolution(exercise.id, await FileZipUtil.getZipFromUris([exercisesDirectories[index].paths.solution!]), false);
333+
}
334+
}
335+
} catch (uploadError) {
336+
// If any upload process fails, all exercises are deleted from database and error is handled (via errorCaught)
337+
errorCaught = true;
338+
v4tLogger.error(uploadError);
339+
try {
340+
exerciseData.data.forEach(async ex => await APIClient.deleteExercise(ex.id));
341+
APIClient.handleAxiosError(uploadError);
342+
} catch (deleteError) {
343+
APIClient.handleAxiosError(deleteError);
344+
}
345+
} finally {
346346
// The user is informed of the result of this process, whether it was successful or not.
347347
this.loading = false;
348348
if (errorCaught) {
@@ -354,7 +354,7 @@ export class CoursesProvider implements vscode.TreeDataProvider<V4TItem> {
354354
);
355355
}
356356
CoursesProvider.triggerTreeReload();
357-
});
357+
}
358358
} catch (_) {
359359
errorCaught = true;
360360
}

vscode4teaching-extension/test/unitSuite/TreeView.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,8 @@ describe("Tree View", () => {
502502
expect(mockedClient.uploadExerciseTemplate).toHaveBeenCalledTimes(1);
503503
expect(mockedClient.uploadExerciseTemplate).toHaveBeenNthCalledWith(1, addExerciseResponseBody.id, mockBufferDoc, false);
504504
expect(mockedClient.uploadExerciseSolution).toHaveBeenCalledTimes(0);
505+
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(1);
506+
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(1, "The new exercise was added successfully.");
505507
});
506508

507509
it("should add some exercises without solution", async () => {
@@ -585,7 +587,7 @@ describe("Tree View", () => {
585587
await coursesProvider.addExercises(courseModel, true);
586588

587589

588-
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(1);
590+
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(2);
589591
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(1, "To upload multiple exercises, prepare a directory with a folder for each exercise, each folder including the exercise's corresponding template and solution if wanted. When ready, click 'Accept'.", { title: "Accept" });
590592
expect(mockedVscode.window.showOpenDialog).toHaveBeenCalledTimes(1);
591593
expect(mockedVscode.window.showOpenDialog).toHaveBeenNthCalledWith(1, openDialogOptions);
@@ -611,6 +613,7 @@ describe("Tree View", () => {
611613
expect(mockedClient.uploadExerciseTemplate).toHaveBeenNthCalledWith(4, addExerciseResponseBody[3].id, mockBufferDoc, false);
612614
expect(mockedClient.uploadExerciseTemplate).toHaveBeenNthCalledWith(5, addExerciseResponseBody[4].id, mockBufferDoc, false);
613615
expect(mockedClient.uploadExerciseSolution).toHaveBeenCalledTimes(0);
616+
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(2, "5 exercises were added successfully.");
614617
});
615618

616619
it("should add an exercise with solution", async () => {
@@ -707,9 +710,11 @@ describe("Tree View", () => {
707710
expect(mockedClient.uploadExerciseTemplate).toHaveBeenNthCalledWith(1, addExerciseResponseBody.id, mockBufferDoc, false);
708711
expect(mockedClient.uploadExerciseSolution).toHaveBeenCalledTimes(1);
709712
expect(mockedClient.uploadExerciseSolution).toHaveBeenNthCalledWith(1, addExerciseResponseBody.id, mockBufferDoc, false);
713+
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(1);
714+
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(1, "The new exercise was added successfully.");
710715
});
711716

712-
it("should add some exercises without solution", async () => {
717+
it("should add some exercises with solution", async () => {
713718
const courseModel = new V4TItem(
714719
teacherCourses[0].name,
715720
V4TItemType.CourseTeacher,
@@ -810,7 +815,7 @@ describe("Tree View", () => {
810815
await coursesProvider.addExercises(courseModel, true);
811816

812817

813-
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(1);
818+
expect(mockedVscode.window.showInformationMessage).toHaveBeenCalledTimes(2);
814819
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(1, "To upload multiple exercises, prepare a directory with a folder for each exercise, each folder including the exercise's corresponding template and solution if wanted. When ready, click 'Accept'.", { title: "Accept" });
815820
expect(mockedVscode.window.showOpenDialog).toHaveBeenCalledTimes(1);
816821
expect(mockedVscode.window.showOpenDialog).toHaveBeenNthCalledWith(1, openDialogOptions);
@@ -851,6 +856,7 @@ describe("Tree View", () => {
851856
expect(mockedClient.uploadExerciseSolution).toHaveBeenNthCalledWith(3, addExerciseResponseBody[2].id, mockBufferDoc, false);
852857
expect(mockedClient.uploadExerciseSolution).toHaveBeenNthCalledWith(4, addExerciseResponseBody[3].id, mockBufferDoc, false);
853858
expect(mockedClient.uploadExerciseSolution).toHaveBeenNthCalledWith(5, addExerciseResponseBody[4].id, mockBufferDoc, false);
859+
expect(mockedVscode.window.showInformationMessage).toHaveBeenNthCalledWith(2, "5 exercises were added successfully.");
854860
});
855861

856862
it("should edit exercise", async () => {

0 commit comments

Comments
 (0)