Skip to content

Commit ccac54e

Browse files
committed
course -- fix #3286 -- "course assignment UI indicator could be stuck in assigning"
- also use wrap on Space'd buttons so they don't get cut off ever.
1 parent a4037f7 commit ccac54e

File tree

10 files changed

+45
-26
lines changed

10 files changed

+45
-26
lines changed

src/packages/frontend/client/project.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export class ProjectClient {
9595
overwrite_newer?: boolean; // overwrite newer versions of file at destination (destructive)
9696
delete_missing?: boolean; // delete files in dest that are missing from source (destructive)
9797
backup?: boolean; // make ~ backup files instead of overwriting changed files
98-
timeout?: number; // how long to wait for the copy to complete before reporting "error" (though it could still succeed)
98+
timeout?: number; // **timeout in seconds** -- how long to wait for the copy to complete before reporting "error" (though it could still succeed)
9999
exclude?: string[]; // list of patterns to exclude; this uses exactly the (confusing) rsync patterns
100100
}): Promise<void> {
101101
const is_public = opts.public;

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import {
2929
defaults,
3030
endswith,
3131
len,
32-
mswalltime,
3332
path_split,
3433
peer_grading,
3534
split,
@@ -72,6 +71,7 @@ import {
7271
PEER_GRADING_GUIDELINES_COMMENT_MARKER,
7372
DUE_DATE_FILENAME,
7473
} from "./consts";
74+
import { COPY_TIMEOUT_MS } from "../consts";
7575

7676
const UPDATE_DUE_DATE_FILENAME_DEBOUNCE_MS = 3000;
7777

@@ -464,6 +464,7 @@ export class AssignmentsActions {
464464
overwrite_newer: true,
465465
backup: true,
466466
delete_missing: false,
467+
timeout: COPY_TIMEOUT_MS / 1000,
467468
});
468469
// write their name to a file
469470
const name = store.get_student_name_extra(student_id);
@@ -623,6 +624,7 @@ ${details}
623624
backup: true,
624625
delete_missing: false,
625626
exclude: peer_graded ? ["*GRADER*.txt"] : undefined,
627+
timeout: COPY_TIMEOUT_MS / 1000,
626628
});
627629
finish("");
628630
} catch (err) {
@@ -712,7 +714,7 @@ ${details}
712714
const a = this.course_actions.get_one(obj);
713715
if (a == null) return;
714716
const x = a[type] ? a[type] : {};
715-
x[student_id] = { time: mswalltime() };
717+
x[student_id] = { time: webapp_client.server_time() };
716718
if (err) {
717719
x[student_id].error = err;
718720
}
@@ -741,7 +743,7 @@ ${details}
741743
if (y.start != null && webapp_client.server_time() - y.start <= 15000) {
742744
return true; // never retry a copy until at least 15 seconds later.
743745
}
744-
y.start = mswalltime();
746+
y.start = webapp_client.server_time();
745747
x[student_id] = y;
746748
obj[type] = x;
747749
this.course_actions.set(obj);
@@ -853,6 +855,7 @@ ${details}
853855
overwrite_newer: !!overwrite, // default is "false"
854856
delete_missing: !!overwrite, // default is "false"
855857
backup: !!!overwrite, // default is "true"
858+
timeout: COPY_TIMEOUT_MS / 1000,
856859
});
857860

858861
// successful finish
@@ -1362,6 +1365,7 @@ ${details}
13621365
overwrite_newer: false,
13631366
delete_missing: false,
13641367
exclude: ["*STUDENT*.txt", "*" + DUE_DATE_FILENAME + "*"],
1368+
timeout: COPY_TIMEOUT_MS / 1000,
13651369
});
13661370
};
13671371

@@ -1444,6 +1448,7 @@ ${details}
14441448
target_path,
14451449
overwrite_newer: false,
14461450
delete_missing: false,
1451+
timeout: COPY_TIMEOUT_MS / 1000,
14471452
});
14481453

14491454
// write local file identifying the grader
@@ -1998,6 +2003,7 @@ ${details}
19982003
overwrite_newer: true,
19992004
delete_missing: true,
20002005
backup: false,
2006+
timeout: COPY_TIMEOUT_MS / 1000,
20012007
});
20022008
} else {
20032009
ephemeralGradePath = false;
@@ -2193,7 +2199,7 @@ ${details}
21932199
Map(),
21942200
);
21952201
const key = student_id ? `${assignment_id}-${student_id}` : assignment_id;
2196-
nbgrader_run_info = nbgrader_run_info.set(key, Date.now());
2202+
nbgrader_run_info = nbgrader_run_info.set(key, webapp_client.server_time());
21972203
this.course_actions.setState({ nbgrader_run_info });
21982204
};
21992205

src/packages/frontend/course/assignments/assignment.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ export function Assignment({
659659
</div>
660660
{render_has_student_subdir(step)}
661661
{render_skip(step)}
662-
<Space>
662+
<Space wrap>
663663
{render_copy_cancel(step)}
664664
<Button
665665
key="yes"
@@ -719,7 +719,7 @@ export function Assignment({
719719
<div style={{ marginBottom: "15px" }}>
720720
{copy_confirm_all_caution(step)}
721721
</div>
722-
<Space>
722+
<Space wrap>
723723
{render_copy_cancel(step)}
724724
<Button
725725
key={"all"}
@@ -756,7 +756,7 @@ export function Assignment({
756756
</div>
757757
{render_has_student_subdir(step)}
758758
{render_skip(step)}
759-
<Space>
759+
<Space wrap>
760760
{render_copy_cancel(step)}
761761
<Button
762762
key="all"

src/packages/frontend/course/common/student-assignment-info.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* License: MS-RSL – see LICENSE.md for details
44
*/
55

6-
import { Button, Col, Row, Space } from "antd";
6+
import { Button, Col, Row, Space, Spin } from "antd";
77
import { ReactNode, useRef, useState } from "react";
88
import { useActions } from "@cocalc/frontend/app-framework";
99
import {
@@ -27,6 +27,8 @@ import {
2727
} from "../store";
2828
import { AssignmentCopyType } from "../types";
2929
import { useButtonSize } from "../util";
30+
import { webapp_client } from "@cocalc/frontend/webapp-client";
31+
import { COPY_TIMEOUT_MS } from "@cocalc/frontend/course/consts";
3032

3133
interface StudentAssignmentInfoProps {
3234
name: string;
@@ -252,7 +254,7 @@ export function StudentAssignmentInfo({
252254
const t = nbgrader_run_info.get(
253255
assignment.get("assignment_id") + "-" + student.get("student_id"),
254256
);
255-
if (t && Date.now() - t <= 1000 * 60 * 10) {
257+
if (t && webapp_client.server_time() - t <= 1000 * 60 * 10) {
256258
// Time starting is set and it's also within the last few minutes.
257259
// This "few minutes" is just in case -- we probably shouldn't need
258260
// that at all ever, but it could make cocalc state usable in case of
@@ -264,7 +266,7 @@ export function StudentAssignmentInfo({
264266
label = running ? (
265267
<span>
266268
{" "}
267-
<Icon name="cocalc-ring" spin /> Running nbgrader
269+
<Spin /> Running nbgrader
268270
</span>
269271
) : (
270272
<span>{label}</span>
@@ -279,7 +281,7 @@ export function StudentAssignmentInfo({
279281
onClick={() => {
280282
if (
281283
clicked_nbgrader.current != null &&
282-
Date.now() - clicked_nbgrader.current.valueOf() <= 3000
284+
webapp_client.server_time() - clicked_nbgrader.current.valueOf() <= 3000
283285
) {
284286
// User *just* clicked, and we want to avoid double click
285287
// running nbgrader twice.
@@ -375,7 +377,7 @@ export function StudentAssignmentInfo({
375377
</div>,
376378
);
377379
}
378-
return <Space>{v}</Space>;
380+
return <Space wrap>{v}</Space>;
379381
} else {
380382
return (
381383
<Button
@@ -419,9 +421,9 @@ export function StudentAssignmentInfo({
419421

420422
function render_open_copying(name: Steps, open, stop) {
421423
return (
422-
<Space key="open_copying">
424+
<Space key="open_copying" wrap>
423425
<Button key="copy" disabled={true} size={size}>
424-
<Icon name="cocalc-ring" spin /> {name}ing
426+
<Spin /> {name}ing
425427
</Button>
426428
<Button key="stop" danger onClick={stop} size={size}>
427429
Cancel <Icon name="times" />
@@ -488,7 +490,7 @@ export function StudentAssignmentInfo({
488490
const do_stop = () => stop(type, info.assignment_id, info.student_id);
489491
const v: JSX.Element[] = [];
490492
if (enable_copy) {
491-
if (data.start) {
493+
if (webapp_client.server_time() - (data.start ?? 0) < COPY_TIMEOUT_MS) {
492494
v.push(render_open_copying(step, do_open, do_stop));
493495
} else if (data.time) {
494496
v.push(
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// All copy operations (e.g., assigning, collecting, etc.) is set to timeout after this long.
2+
// Also in the UI displaying that a copy is ongoing also times out after this long, e.g, if
3+
// the user refreshes their browser and nothing is going to update things again.
4+
// TODO: make this a configurable parameter, e.g., maybe users have very large assignments
5+
// or things are very slow.
6+
export const COPY_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes, for now -- starting project can take time.

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ import type { CourseActions } from "../actions";
1111
import type { CourseStore, HandoutRecord } from "../store";
1212
import { webapp_client } from "../../webapp-client";
1313
import { redux } from "../../app-framework";
14-
import { uuid, mswalltime } from "@cocalc/util/misc";
14+
import { uuid } from "@cocalc/util/misc";
1515
import { map } from "awaiting";
1616
import type { SyncDBRecordHandout } from "../types";
1717
import { exec } from "../../frame-editors/generic/client";
1818
import { export_student_file_use_times } from "../export/file-use-times";
19+
import { COPY_TIMEOUT_MS } from "../consts";
1920

2021
export class HandoutsActions {
2122
private course_actions: CourseActions;
@@ -131,7 +132,7 @@ export class HandoutsActions {
131132
const status_map: {
132133
[student_id: string]: { time?: number; error?: string };
133134
} = h.status ? h.status : {};
134-
status_map[student_id] = { time: mswalltime() };
135+
status_map[student_id] = { time: webapp_client.server_time() };
135136
if (err) {
136137
status_map[student_id].error = err;
137138
}
@@ -163,7 +164,7 @@ export class HandoutsActions {
163164
) {
164165
return true; // never retry a copy until at least 15 seconds later.
165166
}
166-
student_status.start = mswalltime();
167+
student_status.start = webapp_client.server_time();
167168
status_map[student_id] = student_status;
168169
obj.status = status_map;
169170
this.course_actions.set(obj);
@@ -259,6 +260,7 @@ export class HandoutsActions {
259260
overwrite_newer: !!overwrite, // default is "false"
260261
delete_missing: !!overwrite, // default is "false"
261262
backup: !!!overwrite, // default is "true"
263+
timeout: COPY_TIMEOUT_MS / 1000,
262264
});
263265
finish();
264266
} catch (err) {

src/packages/frontend/course/handouts/handout.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ export function Handout({
314314
<div style={{ marginBottom: "15px" }}>
315315
{copy_confirm_all_caution(step)}
316316
</div>
317-
<Space>
317+
<Space wrap>
318318
{render_copy_cancel()}
319319
<Button key="all" onClick={() => copy_handout(step, false)}>
320320
Yes, do it
@@ -346,7 +346,7 @@ export function Handout({
346346
{capitalize(step_verb(step))} this handout {step_direction(step)}
347347
...
348348
</div>
349-
<Space>
349+
<Space wrap>
350350
{render_copy_cancel()}
351351
<Button
352352
key="all"

src/packages/frontend/course/handouts/handouts-info-panel.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { ErrorDisplay, Icon, Tip } from "../../components";
1212
import { CourseActions } from "../actions";
1313
import { BigTime } from "../common";
1414
import { LastCopyInfo } from "../store";
15+
import { webapp_client } from "@cocalc/frontend/webapp-client";
16+
import { COPY_TIMEOUT_MS } from "@cocalc/frontend/course/consts";
1517

1618
interface StudentHandoutInfoProps {
1719
actions: CourseActions;
@@ -66,7 +68,7 @@ export function StudentHandoutInfo({
6668
<Icon name="share-square" /> Yes, {name.toLowerCase()} again
6769
</Button>,
6870
);
69-
return <Space>{v}</Space>;
71+
return <Space wrap>{v}</Space>;
7072
} else {
7173
return (
7274
<Button type="dashed" key="copy" onClick={() => setRecopy(true)}>
@@ -144,7 +146,7 @@ export function StudentHandoutInfo({
144146
}
145147
const v: any[] = [];
146148
if (enable_copy) {
147-
if (obj.start) {
149+
if (webapp_client.server_time() - (obj.start ?? 0) < COPY_TIMEOUT_MS) {
148150
v.push(render_open_copying(do_open, do_stop));
149151
} else if (obj.time) {
150152
v.push(render_open_recopy(name, do_open, do_copy, copy_tip, open_tip));

src/packages/frontend/course/nbgrader/nbgrader-button.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { CourseStore, NBgraderRunInfo, PARALLEL_DEFAULT } from "../store";
1515
import { CourseActions } from "../actions";
1616
import { nbgrader_status } from "./util";
1717
import { plural } from "@cocalc/util/misc";
18+
import { webapp_client } from "@cocalc/frontend/webapp-client";
1819

1920
interface Props {
2021
name: string;
@@ -40,7 +41,7 @@ export function NbgraderButton({ name, assignment_id }: Props) {
4041
const running = useMemo(() => {
4142
if (nbgrader_run_info == null) return false;
4243
const t = nbgrader_run_info.get(assignment_id);
43-
if (t && Date.now() - t <= 1000 * 60 * 10) {
44+
if (webapp_client.server_time() - (t ?? 0) <= 1000 * 60 * 10) {
4445
// Time starting is set and it's also within the last few minutes.
4546
// This "few minutes" is just in case -- we probably shouldn't need
4647
// that at all ever, but it could make cocalc state usable in case of

src/packages/frontend/course/student-projects/actions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export class StudentProjectsActions {
135135
this.course_actions.set({
136136
table: "students",
137137
student_id,
138-
last_email_invite: Date.now(),
138+
last_email_invite: webapp_client.server_time(),
139139
});
140140
} else {
141141
await redux
@@ -182,7 +182,7 @@ export class StudentProjectsActions {
182182
this.course_actions.set({
183183
table: "students",
184184
student_id,
185-
last_email_invite: Date.now(),
185+
last_email_invite: webapp_client.server_time(),
186186
});
187187
}
188188
}

0 commit comments

Comments
 (0)