Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions bases/rsptx/admin_server_api/routers/instructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -1206,15 +1206,18 @@ async def copy_assignment(
target_course=course,
importing_user=user,
)
message = f'Copied as "{result.name}". It is hidden until you make it visible.'
if result.duedate_warning:
message += f" {result.duedate_warning}"
return JSONResponse(
content={
"success": True,
"message": f'Copied as "{result.name}". It is hidden until you '
f"make it visible.",
"message": message,
"imported": [result.name],
"skipped_existing": [],
"skipped_readings": result.skipped_readings,
"failed": [],
"duedate_warning": result.duedate_warning,
}
)

Expand All @@ -1237,16 +1240,25 @@ async def copy_assignment(
parts.append(f"{len(bulk.failed)} could not be copied")
if bulk.skipped_readings:
parts.append(f"{bulk.skipped_readings} readings left behind")
if bulk.duedate_not_shifted:
parts.append(f"{bulk.duedate_not_shifted} due dates not adjusted")

message = ", ".join(parts) + ". Copies are hidden until you make them visible."
if bulk.duedate_not_shifted:
message += (
" Some due dates could not be adjusted because a course timezone "
"setting is invalid -- update it in Course Settings."
)

return JSONResponse(
content={
"success": True,
"message": ", ".join(parts)
+ ". Copies are hidden until you make them visible.",
"message": message,
"imported": bulk.imported,
"skipped_existing": bulk.skipped_existing,
"skipped_readings": bulk.skipped_readings,
"failed": bulk.failed,
"duedate_not_shifted": bulk.duedate_not_shifted,
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ const {
already_imported: false,
imported_as: null,
skipped_readings: 0,
duedate_warning: null,
questions: [
{
id: 11,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ export const ImportPreviewPanel = ({ assignmentId, onClose }: ImportPreviewPanel
</Alert>
) : null}

{preview.duedate_warning ? (
<Alert variant="light" color="yellow" title="Due date not adjusted">
{preview.duedate_warning}
</Alert>
) : null}
Comment on lines +71 to +75

{preview.skipped_readings > 0 ? (
<Alert
variant="light"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ export const ASSIGNMENT_TOAST_COPY = {
} that belong to the other book. It is hidden until you make it visible.`
: `Imported as "${name}". It is hidden until you make it visible.`,
importError: "Couldn't import assignment. It may no longer be shared.",
// Shown separately from the success toast so it doesn't get lost inside a
// longer message -- the instructor needs to notice and go fix the timezone.
duedateWarning: (duedateWarning: string) => duedateWarning,
loadTreeError: "Couldn't load shared assignments. Try again.",
// Partial success is a normal outcome for a multi-import, so the toast counts
// rather than claiming everything worked.
Expand All @@ -47,6 +50,7 @@ export const ASSIGNMENT_TOAST_COPY = {
skipped_existing: string[];
skipped_readings: number;
failed: string[];
duedate_not_shifted: number;
}) => {
const parts = [`Imported ${result.imported.length}`];

Expand All @@ -59,6 +63,9 @@ export const ASSIGNMENT_TOAST_COPY = {
if (result.skipped_readings) {
parts.push(`${result.skipped_readings} readings left behind`);
}
if (result.duedate_not_shifted) {
parts.push(`${result.duedate_not_shifted} due dates not adjusted`);
}
return `${parts.join(", ")}. Imports are hidden until you make them visible.`;
}
} as const;
Expand Down Expand Up @@ -260,6 +267,15 @@ export const assignmentApi = createApi({
queryFulfilled
.then(({ data }) => {
notify.success(ASSIGNMENT_TOAST_COPY.importedMany(data.detail));
if (data.detail.duedate_not_shifted) {
notify.info(
ASSIGNMENT_TOAST_COPY.duedateWarning(
"Some due dates could not be adjusted because a course timezone " +
"setting is invalid. Update it in Course Settings, then check " +
"the affected assignments."
)
);
}
})
.catch(() => {
notify.error(ASSIGNMENT_TOAST_COPY.importError);
Expand All @@ -279,6 +295,9 @@ export const assignmentApi = createApi({
notify.success(
ASSIGNMENT_TOAST_COPY.imported(data.detail.name, data.detail.skipped_readings)
);
if (data.detail.duedate_warning) {
notify.info(ASSIGNMENT_TOAST_COPY.duedateWarning(data.detail.duedate_warning));
}
})
.catch(() => {
notify.error(ASSIGNMENT_TOAST_COPY.importError);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export type SharedAssignmentPreview = {
imported_as: string | null;
/** How many readings will be left behind, which only happens cross-book. */
skipped_readings: number;
/** Set when the due date could not be shifted because a course's timezone is invalid. */
duedate_warning: string | null;
questions: SharedAssignmentQuestion[];
};

Expand All @@ -44,6 +46,8 @@ export type ImportAssignmentResult = {
id: number;
name: string;
skipped_readings: number;
/** Set when the due date could not be shifted because a course's timezone is invalid. */
duedate_warning: string | null;
};

/** One assignment under a course in the import tree. */
Expand Down Expand Up @@ -103,4 +107,6 @@ export type ImportCourseResult = {
skipped_existing: string[];
skipped_readings: number;
failed: string[];
/** How many imports kept their original due date because a course timezone is invalid. */
duedate_not_shifted: number;
};
2 changes: 2 additions & 0 deletions bases/rsptx/assignment_server_api/routers/instructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2235,6 +2235,7 @@ async def import_assignment_endpoint(
"id": result.assignment.id,
"name": result.name,
"skipped_readings": result.skipped_readings,
"duedate_warning": result.duedate_warning,
},
)

Expand Down Expand Up @@ -2374,6 +2375,7 @@ async def import_course_assignments_endpoint(
"skipped_existing": result.skipped_existing,
"skipped_readings": result.skipped_readings,
"failed": result.failed,
"duedate_not_shifted": result.duedate_not_shifted,
},
)

Expand Down
70 changes: 54 additions & 16 deletions components/rsptx/db/crud/assignment.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import datetime
from typing import NamedTuple, Optional, List
from zoneinfo import ZoneInfo
from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
from fastapi import HTTPException, status
from asyncpg.exceptions import UniqueViolationError
from rsptx.validation import schemas
Expand Down Expand Up @@ -1504,24 +1504,44 @@ def term_start_utc(

def shift_duedate_between_courses(
duedate: datetime.datetime, source_course, target_course
) -> datetime.datetime:
) -> tuple[datetime.datetime, Optional[str]]:
"""Re-date ``duedate`` so it keeps its offset from the start of term.

Both term starts are anchored in their own course timezone, so the offset
is preserved as local wall clock time even when the two terms fall on
opposite sides of a DST change. If either course is missing a term start
there is nothing to shift against, so the original date is kept.

A course timezone that is no longer a recognized zoneinfo key (a user can
pick one that later gets removed from the tz database) would otherwise
take the whole import down with it. That is worse than an unshifted due
date, so the shift is skipped and a warning string is returned instead of
raising -- the caller still gets an assignment, just not a re-dated one.
"""
if not (source_course.term_start_date and target_course.term_start_date):
return duedate
return duedate, None

due_delta = duedate - term_start_utc(
source_course.term_start_date, source_course.timezone
)
return (
term_start_utc(target_course.term_start_date, target_course.timezone)
+ due_delta
)
try:
due_delta = duedate - term_start_utc(
source_course.term_start_date, source_course.timezone
)
shifted = (
term_start_utc(target_course.term_start_date, target_course.timezone)
+ due_delta
)
except (ZoneInfoNotFoundError, ValueError) as e:
rslogger.warning(
f"Could not shift due date from {source_course.course_name!r} to "
f"{target_course.course_name!r}, one of their timezones is invalid "
f"({source_course.timezone!r} -> {target_course.timezone!r}): {e}"
)
return duedate, (
"The due date could not be adjusted because a course timezone "
"setting is invalid. Update the timezone in Course Settings, then "
"check this assignment's due date."
)

return shifted, None


def _normalize_assignment_kind(kind: str, is_timed: bool, is_peer: bool):
Expand Down Expand Up @@ -1579,12 +1599,16 @@ class ImportedAssignment(NamedTuple):

``skipped_readings`` is how many rows were left behind by the cross-book
rule above, so the caller can say so rather than letting the instructor
discover the gap themselves.
discover the gap themselves. ``duedate_warning`` is set when the due date
could not be shifted to the target course's term because a course
timezone is invalid -- the assignment still imports, with its original
due date.
"""

assignment: AssignmentValidator
name: str
skipped_readings: int
duedate_warning: Optional[str] = None


def _searchable_column(field: str, joined_columns: dict):
Expand Down Expand Up @@ -2029,8 +2053,11 @@ async def fetch_assignment_for_preview(
)

duedate = assignment.duedate
duedate_warning = None
if target_course is not None:
duedate = shift_duedate_between_courses(duedate, source_course, target_course)
duedate, duedate_warning = shift_duedate_between_courses(
duedate, source_course, target_course
)

cross_book = (
target_course is not None
Expand Down Expand Up @@ -2066,6 +2093,7 @@ async def fetch_assignment_for_preview(
"already_imported": source_assignment_id in existing_imports,
"imported_as": existing_imports.get(source_assignment_id),
"skipped_readings": will_import.count(False),
"duedate_warning": duedate_warning,
"questions": [
{
"id": row.Question.id,
Expand Down Expand Up @@ -2109,7 +2137,7 @@ async def import_assignment(
source_assignment_id, importing_user.id
)

duedate = shift_duedate_between_courses(
duedate, duedate_warning = shift_duedate_between_courses(
assignment.duedate, source_course, target_course
)
is_timed, is_peer = _normalize_assignment_kind(
Expand Down Expand Up @@ -2209,7 +2237,7 @@ async def import_assignment(
f"into {target_course.course_name} as {result.id} ({new_name}) with "
f"{len(rows_to_import)} questions, {skipped_readings} readings skipped"
)
return ImportedAssignment(result, new_name, skipped_readings)
return ImportedAssignment(result, new_name, skipped_readings, duedate_warning)


class BulkImportResult(NamedTuple):
Expand All @@ -2218,12 +2246,16 @@ class BulkImportResult(NamedTuple):
Names rather than counts so a caller can show either. ``skipped_existing``
holds the source names that were passed over, which is the difference
between "that course only had two new ones" and "nothing happened".
``duedate_not_shifted`` counts imports whose due date could not be moved
to the target term because a course timezone is invalid -- the assignment
still imports, just with its original due date.
"""

imported: List[str]
skipped_existing: List[str]
skipped_readings: int
failed: List[str]
duedate_not_shifted: int = 0


async def import_course_assignments(
Expand Down Expand Up @@ -2324,6 +2356,7 @@ async def import_course_assignments(
skipped_existing: List[str] = []
failed: List[str] = []
skipped_readings = 0
duedate_not_shifted = 0

for source in rows:
if skip_existing and source.id in already_imported:
Expand All @@ -2343,13 +2376,18 @@ async def import_course_assignments(
continue
imported.append(result.name)
skipped_readings += result.skipped_readings
if result.duedate_warning:
duedate_not_shifted += 1

rslogger.info(
f"Bulk imported {len(imported)} assignments from {source_course.course_name} "
f"into {target_course.course_name}; {len(skipped_existing)} already present, "
f"{len(failed)} failed, {skipped_readings} readings skipped"
f"{len(failed)} failed, {skipped_readings} readings skipped, "
f"{duedate_not_shifted} due dates not shifted"
)
return BulkImportResult(
imported, skipped_existing, skipped_readings, failed, duedate_not_shifted
)
return BulkImportResult(imported, skipped_existing, skipped_readings, failed)


async def search_shareable_courses(
Expand Down
36 changes: 36 additions & 0 deletions test/components/rsptx/db/test_assignment_sharing.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,42 @@ async def test_import_shifts_the_due_date_by_the_offset_from_term_start(sharing_
)


async def test_import_keeps_the_original_duedate_when_a_course_timezone_is_invalid(
sharing_world,
):
"""A timezone the tz database no longer recognizes should not sink the import.

An instructor can end up with a course timezone that zoneinfo later stops
recognizing (a deprecated alias dropped from the tz database). The due
date shift depends on being able to resolve that zone, so it is skipped
rather than raising -- the assignment still imports, just without a
re-dated due date, and the caller gets a warning to relay.
"""
bad_tz_course = await _make_course(
"sharing_bad_tz_course",
SRC_BOOK,
datetime.date(2026, 1, 12),
"Not/A_Real_Zone",
)
await create_instructor_course_entry(sharing_world["owner"].id, bad_tz_course.id)
await _share_assignments(bad_tz_course.id)

stored = datetime.datetime(2026, 1, 20, 23, 59)
source_assignment = await _make_assignment(
bad_tz_course.id, "Bad Timezone Homework", is_private=False, duedate=stored
)

result = await import_assignment(
source_assignment_id=source_assignment.id,
target_course=sharing_world["dst"],
importing_user=sharing_world["importer"],
)

assert result.assignment.duedate == stored
assert result.duedate_warning is not None
assert "timezone" in result.duedate_warning.lower()


# Already imported
# ----------------

Expand Down
Loading