Skip to content

Commit 8a423e5

Browse files
committed
fix #2929 -- Due Date update for courses
now when you change the due date and don't change it again for 3s, it gets updated in the course project *and* in all student projects that have already got the assignment
1 parent af00c5d commit 8a423e5

File tree

3 files changed

+155
-47
lines changed

3 files changed

+155
-47
lines changed

src/packages/frontend/course/assignments/actions.ts

Lines changed: 149 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
uuid,
3838
} from "@cocalc/util/misc";
3939
import { delay, map } from "awaiting";
40+
import { debounce } from "lodash";
4041
import { Map } from "immutable";
4142
import { CourseActions } from "../actions";
4243
import { export_assignment } from "../export/export-assignment";
@@ -69,8 +70,11 @@ import {
6970
STUDENT_SUBDIR,
7071
PEER_GRADING_GUIDELINES_GRADE_MARKER,
7172
PEER_GRADING_GUIDELINES_COMMENT_MARKER,
73+
DUE_DATE_FILENAME,
7274
} from "./consts";
7375

76+
const UPDATE_DUE_DATE_FILENAME_DEBOUNCE_MS = 3000;
77+
7478
export class AssignmentsActions {
7579
private course_actions: CourseActions;
7680

@@ -282,18 +286,89 @@ export class AssignmentsActions {
282286
});
283287
};
284288

285-
set_due_date = (
289+
set_due_date = async (
286290
assignment_id: string,
287291
due_date: Date | string | undefined | null,
288-
): void => {
289-
if (due_date == null) {
290-
this.set_assignment_field(assignment_id, "due_date", null);
292+
): Promise<void> => {
293+
const { assignment } = this.course_actions.resolve({
294+
assignment_id,
295+
});
296+
if (assignment == null) {
297+
return;
298+
}
299+
const prev_due_date = assignment.get("due_date");
300+
301+
if (!due_date) {
302+
// deleting it
303+
if (prev_due_date) {
304+
// not deleted so delete it
305+
this.set_assignment_field(assignment_id, "due_date", null);
306+
this.updateDueDateFile(assignment_id);
307+
}
291308
return;
292309
}
310+
293311
if (typeof due_date !== "string") {
294312
due_date = due_date.toISOString(); // using strings instead of ms for backward compatibility.
295313
}
314+
315+
if (prev_due_date == due_date) {
316+
// nothing to do.
317+
return;
318+
}
319+
296320
this.set_assignment_field(assignment_id, "due_date", due_date);
321+
// it changed, so update the file in all student projects that have already been assigned
322+
// https://github.com/sagemathinc/cocalc/issues/2929
323+
// NOTE: updateDueDate is debounced, so if set_due_date is called a lot, then the
324+
// actual update only happens after it stabilizes for a while. Also, we can be
325+
// sure the store has updated the assignment.
326+
this.updateDueDateFile(assignment_id);
327+
};
328+
329+
private updateDueDateFile = debounce(async (assignment_id: string) => {
330+
// important to check actions due to debounce.
331+
if (this.course_actions.is_closed()) return;
332+
await this.copy_assignment_create_due_date_file(assignment_id);
333+
if (this.course_actions.is_closed()) return;
334+
335+
const desc = `Copying modified ${DUE_DATE_FILENAME} to all students who have already received it`;
336+
const short_desc = `copy ${DUE_DATE_FILENAME}`;
337+
338+
// by default, doesn't create the due file
339+
await this.assignment_action_all_students({
340+
assignment_id,
341+
old_only: true,
342+
action: this.writeDueDateFile,
343+
step: "assignment",
344+
desc,
345+
short_desc,
346+
});
347+
}, UPDATE_DUE_DATE_FILENAME_DEBOUNCE_MS);
348+
349+
private writeDueDateFile = async (
350+
assignment_id: string,
351+
student_id: string,
352+
) => {
353+
const { student, assignment } = this.course_actions.resolve({
354+
assignment_id,
355+
student_id,
356+
});
357+
if (!student || !assignment) return;
358+
const content = this.dueDateFileContent(assignment_id);
359+
const project_id = student.get("project_id");
360+
if (!project_id) return;
361+
const path = join(assignment.get("target_path"), DUE_DATE_FILENAME);
362+
console.log({
363+
project_id,
364+
path,
365+
content,
366+
});
367+
await webapp_client.project_client.write_text_file({
368+
project_id,
369+
path,
370+
content,
371+
});
297372
};
298373

299374
set_assignment_note = (assignment_id: string, note: string): void => {
@@ -800,22 +875,17 @@ ${details}
800875
private copy_assignment_create_due_date_file = async (
801876
assignment_id: string,
802877
): Promise<void> => {
803-
const { assignment, store } = this.course_actions.resolve({
878+
const { assignment } = this.course_actions.resolve({
804879
assignment_id,
805880
});
806881
if (!assignment) return;
807882
// write the due date to a file
808-
const due_date = store.get_due_date(assignment_id);
809883
const src_path = this.assignment_src_path(assignment);
810-
const due_date_fn = "DUE_DATE.txt";
811-
if (due_date == null) {
812-
return;
813-
}
814884
const due_id = this.course_actions.set_activity({
815-
desc: `Creating ${due_date_fn} file...`,
885+
desc: `Creating ${DUE_DATE_FILENAME} file...`,
816886
});
817-
const content = `This assignment is due\n\n ${due_date.toLocaleString()}`;
818-
const path = join(src_path, due_date_fn);
887+
const content = this.dueDateFileContent(assignment_id);
888+
const path = join(src_path, DUE_DATE_FILENAME);
819889

820890
try {
821891
await this.write_text_file_to_course_project({
@@ -824,13 +894,22 @@ ${details}
824894
});
825895
} catch (err) {
826896
throw Error(
827-
`Problem writing ${due_date_fn} file ('${err}'). Try again...`,
897+
`Problem writing ${DUE_DATE_FILENAME} file ('${err}'). Try again...`,
828898
);
829899
} finally {
830900
this.course_actions.clear_activity(due_id);
831901
}
832902
};
833903

904+
private dueDateFileContent = (assignment_id) => {
905+
const due_date = this.get_store()?.get_due_date(assignment_id);
906+
if (due_date) {
907+
return `This assignment is due\n\n ${due_date.toLocaleString()}`;
908+
} else {
909+
return "No due date has been set.";
910+
}
911+
};
912+
834913
copy_assignment = async (
835914
type: AssignmentCopyType,
836915
assignment_id: string,
@@ -881,15 +960,15 @@ ${details}
881960
await this.copy_assignment_create_due_date_file(assignment_id);
882961
if (this.course_actions.is_closed()) return;
883962
// by default, doesn't create the due file
884-
await this.assignment_action_all_students(
963+
await this.assignment_action_all_students({
885964
assignment_id,
886965
new_only,
887-
this.copy_assignment_to_student,
888-
"assignment",
966+
action: this.copy_assignment_to_student,
967+
step: "assignment",
889968
desc,
890969
short_desc,
891970
overwrite,
892-
);
971+
});
893972
};
894973

895974
// Copy the given assignment to from all non-deleted students, doing several copies in parallel at once.
@@ -902,14 +981,14 @@ ${details}
902981
desc += " from whom we have not already copied it";
903982
}
904983
const short_desc = "copy from student";
905-
await this.assignment_action_all_students(
984+
await this.assignment_action_all_students({
906985
assignment_id,
907986
new_only,
908-
this.copy_assignment_from_student,
909-
"collect",
987+
action: this.copy_assignment_from_student,
988+
step: "collect",
910989
desc,
911990
short_desc,
912-
);
991+
});
913992
};
914993

915994
private start_all_for_peer_grading = async (): Promise<void> => {
@@ -956,14 +1035,14 @@ ${details}
9561035
}
9571036
await this.start_all_for_peer_grading();
9581037
// OK, now do the assignment... in parallel.
959-
await this.assignment_action_all_students(
1038+
await this.assignment_action_all_students({
9601039
assignment_id,
9611040
new_only,
962-
this.peer_copy_to_student,
963-
"peer_assignment",
1041+
action: this.peer_copy_to_student,
1042+
step: "peer_assignment",
9641043
desc,
9651044
short_desc,
966-
);
1045+
});
9671046
}
9681047

9691048
async peer_collect_from_all_students(
@@ -976,14 +1055,14 @@ ${details}
9761055
}
9771056
const short_desc = "copy peer grading from students";
9781057
await this.start_all_for_peer_grading();
979-
await this.assignment_action_all_students(
1058+
await this.assignment_action_all_students({
9801059
assignment_id,
9811060
new_only,
982-
this.peer_collect_from_student,
983-
"peer_collect",
1061+
action: this.peer_collect_from_student,
1062+
step: "peer_collect",
9841063
desc,
9851064
short_desc,
986-
);
1065+
});
9871066
await this.peerParseStudentGrading(assignment_id);
9881067
}
9891068

@@ -1098,19 +1177,36 @@ ${details}
10981177
this.course_actions.clear_activity(id);
10991178
};
11001179

1101-
private assignment_action_all_students = async (
1102-
assignment_id: string,
1103-
new_only: boolean,
1180+
private assignment_action_all_students = async ({
1181+
assignment_id,
1182+
new_only,
1183+
old_only,
1184+
action,
1185+
step,
1186+
desc,
1187+
short_desc,
1188+
overwrite,
1189+
}: {
1190+
assignment_id: string;
1191+
// only do the action when it hasn't been done already
1192+
new_only?: boolean;
1193+
// only do the action when it HAS been done already
1194+
old_only?: boolean;
11041195
action: (
11051196
assignment_id: string,
11061197
student_id: string,
11071198
opts: any,
1108-
) => Promise<void>,
1109-
step,
1110-
desc,
1111-
short_desc: string,
1112-
overwrite?: boolean,
1113-
): Promise<void> => {
1199+
) => Promise<void>;
1200+
step;
1201+
desc;
1202+
short_desc: string;
1203+
overwrite?: boolean;
1204+
}): Promise<void> => {
1205+
if (new_only && old_only) {
1206+
// no matter what, this means the empty set, so nothing to do.
1207+
// Of course no code shouild actually call this.
1208+
return;
1209+
}
11141210
const id = this.course_actions.set_activity({ desc });
11151211
const finish = (err) => {
11161212
this.course_actions.clear_activity(id);
@@ -1135,14 +1231,22 @@ ${details}
11351231
) {
11361232
return;
11371233
}
1138-
if (
1139-
new_only &&
1140-
store.last_copied(step, assignment_id, student_id, true)
1141-
) {
1234+
const alreadyCopied = !!store.last_copied(
1235+
step,
1236+
assignment_id,
1237+
student_id,
1238+
true,
1239+
);
1240+
if (new_only && alreadyCopied) {
1241+
// only for the ones that haven't already been copied
1242+
return;
1243+
}
1244+
if (old_only && !alreadyCopied) {
1245+
// only for the ones that *HAVE* already been copied.
11421246
return;
11431247
}
11441248
try {
1145-
await action.bind(this)(assignment_id, student_id, { overwrite });
1249+
await action(assignment_id, student_id, { overwrite });
11461250
} catch (err) {
11471251
errors += `\n ${err}`;
11481252
}
@@ -1257,7 +1361,7 @@ ${details}
12571361
target_path,
12581362
overwrite_newer: false,
12591363
delete_missing: false,
1260-
exclude: ["*STUDENT*.txt", "*DUE_DATE.txt*"],
1364+
exclude: ["*STUDENT*.txt", "*" + DUE_DATE_FILENAME + "*"],
12611365
});
12621366
};
12631367

src/packages/frontend/course/assignments/consts.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const PEER_GRADING_GUIDE_FILENAME = "GRADING-GUIDE.md";
2020

2121
// Everything from GRADING_GUIDELINES_GRADE_MARKER to GRADING_GUIDELINES_COMMENT_MARKER
2222
// is parsed as a numerical grade, if possible. i18n's don't mess this up! Also,
23-
// changing this would break outstanding assignments, so change with caution.
23+
// changing this would break outstanding assignments, so change with caution.
2424
// A fix
2525
// would be to store these strings somewhere when pushing the assignment out, so that
2626
// the same ones are used when collecting and parsing. But that will take a few hours
@@ -43,3 +43,7 @@ ${PEER_GRADING_GUIDELINES_GRADE_MARKER}
4343
${PEER_GRADING_GUIDELINES_COMMENT_MARKER}
4444
4545
`;
46+
47+
// this could get translated somehow...
48+
export const DUE_DATE_FILENAME = "DUE_DATE.txt";
49+

src/packages/frontend/course/store.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export type LastCopyInfo = {
8181
export type AssignmentRecord = TypedMap<{
8282
assignment_id: string;
8383
deleted: boolean;
84-
due_date: Date;
84+
due_date: string; // iso string
8585
path: string;
8686
peer_grade?: {
8787
enabled: boolean;

0 commit comments

Comments
 (0)