diff --git a/CLAUDE.md b/CLAUDE.md index 219fa2c..291273a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,7 @@ FRONTEND_URL=http://localhost:8080 | Task | Location | |------|----------| | Add API endpoint | `main.py` | +| Change grading logic | `grading/bulk.py` (`evaluate_student`, shared by single and bulk grading) | | Add React component | `frontend/courses-front/src/components/` | | Add/edit course | `courses/` directory + `index.yaml` | | Add translation | `frontend/courses-front/src/locales/{en,ru,zh}/` | @@ -118,6 +119,25 @@ Orchestration lives in `grading/propagate.py` (in-memory job state, single-worke `docs/PROJECT_DESCRIPTION.md`). All `/admin/...` and course-management routes require the `require_admin` FastAPI dependency in `main.py`, not just the frontend's `ProtectedRoute`. +## Bulk Grading (admin) + +Grades a whole group for one lab in a single run, started from the admin lab list page +(`/admin/courses/{course_id}/labs`) - see `docs/PROJECT_DESCRIPTION.md` for the full behaviour. + +- `grading/bulk.py:evaluate_student` holds the grading decision and is shared by `grade_lab` and the + bulk run, so the two cannot drift apart. It never touches Sheets: the spreadsheet context arrives + through a lazily-invoked provider, which is what lets `grade_lab` still answer repository and CI + errors without opening a Sheets connection. +- Endpoints: `POST /admin/courses/{id}/groups/{g}/labs/{l}/bulk-grade` (202 + `job_id`), + `GET /admin/bulk-grade-jobs/{job_id}`, `POST /admin/bulk-grade-jobs/{job_id}/cancel`. +- Job state mirrors `grading/propagate.py` (in-memory, single-worker backend required, 409 on a + second run for the same course/group/lab). +- With `name_file` set, repos are discovered by the lab's prefix and matched to sheet rows by the + first line of that file; without it, only students who already have a username in the sheet are + graded. Name matching is exact after normalization - no fuzzy matching, by design. +- Reads the worksheet once via `get_all_values()` and writes grades in batches of 10: the per-cell + helpers spend ~6 Sheets requests per student, over the 60 reads/minute quota for a group of 30. + ## CI/CD - **Tests**: Run on every push via `.github/workflows/tests.yml` diff --git a/docs/BULK_GRADING_PLAN.md b/docs/BULK_GRADING_PLAN.md new file mode 100644 index 0000000..a3ece92 --- /dev/null +++ b/docs/BULK_GRADING_PLAN.md @@ -0,0 +1,57 @@ +# Массовая проверка лабораторных: разбор решений + +Запись принятых решений по фиче. Поведение и API описаны в `docs/PROJECT_DESCRIPTION.md` (раздел «Массовая проверка лабораторной»); здесь - только то, почему сделано именно так. + +## 1. Постановка задачи + +Преподаватель выбирает курс, группу и лабораторную и нажимает кнопку. Анализируются репозитории лабы в организации курса, результат проверки проставляется в Google Таблицу. Необязательное поле «файл с ФИО» переключает режим: с ним обходятся все репозитории лабы и GitHub-логины проставляются в таблицу по ФИО из файла, без него проверяются только студенты с уже указанным логином. + +## 2. Одно ядро для одиночной и массовой проверки + +`grade_lab` (main.py) - функция на 240 строк, в которой перемешаны HTTP-слой, поячеечное чтение Google Таблицы и логика выставления оценки. Массовая проверка не может переиспользовать её как есть: ей нужен тот же алгоритм без `HTTPException` и без обращений к Sheets на каждого студента. + +Дублировать алгоритм нельзя - разойдутся. Поэтому решение об оценке вынесено в `grading/bulk.py:evaluate_student`, а `grade_lab` переписан в тонкую обёртку над ним. + +### Почему контекст таблицы передаётся через провайдер + +`grade_lab` намеренно устроен так, что ошибки репозитория и CI возвращаются **до** подключения к Google Sheets: при отсутствии коммитов или незавершённом CI соединение с таблицей не открывается вовсе. Если бы `evaluate_student` принимала готовый `SheetContext`, вызывающему пришлось бы читать таблицу заранее - и это свойство потерялось бы. + +Поэтому `evaluate_student` принимает не значение, а функцию, возвращающую `SheetContext`, и вызывает её не раньше, чем CI даст результат, который есть смысл записывать. `grade_lab` передаёт замыкание, открывающее лист; массовая проверка - функцию, отдающую уже прочитанную строку сетки. + +## 3. Чтение таблицы одним запросом + +Квота Google Sheets API - 60 запросов чтения в минуту. Поячеечные хелперы (`sheet.cell`, `row_values`, `col_values`, `worksheet.find`) тратят ~6 запросов на студента: на группе из 30 человек это ~180 запросов и гарантированное превышение квоты. + +Массовый режим читает лист один раз через `get_all_values()` и ищет по сетке в памяти. В `grading/sheets_client.py` добавлены grid-варианты существующих хелперов; поячеечные не тронуты, ими продолжает пользоваться `grade_lab`. Парсинг дедлайна вынесен в общую `parse_deadline`, чтобы обе ветки разбирали даты одинаково. + +Запись - пакетами по 10 ячеек **по ходу работы**, а не одним блоком в конце: падение, перезапуск или отмена сохраняют уже проставленные оценки. + +## 4. Сопоставление ФИО - только точное + +ФИО берётся из файла, который студент заполнял руками, поэтому сравнение нормализует пробелы (включая неразрывные), регистр и `ё`/`е`. Дальше - только точное совпадение. + +Нечёткий подбор (по фамилии, по расстоянию Левенштейна) сознательно не реализован: цена ошибки - оценка, записанная не тому студенту, и обнаружится она нескоро. Несопоставленные репозитории попадают в отчёт с прочитанным ФИО, чтобы преподаватель исправил вручную - это дешевле, чем разбирать последствия неверного совпадения. + +По той же причине совпадение ФИО у двух студентов - не повод выбрать первого: строка пропускается со статусом `ambiguous`. + +## 5. Конфликт GitHub-логинов + +Если в строке студента уже указан **другой** логин, репозиторий не проверяется ни под одним из них и попадает в отчёт со статусом `conflict`. Молча перезаписать логин нельзя (потеряется факт, что студент регистрировался сам), а проверить репозиторий под логином из строки - значит проверить чужую работу. + +## 6. Защита оценок + +Правило `can_overwrite_cell` (перезаписываются только пустые ячейки, `x` и значения с `?` в начале) действует и здесь. Флага принудительной перезаписи нет: одна случайная галочка затирала бы оценки всей группы, а пересдача - штучная операция, для которой достаточно очистить ячейку в таблице. + +## 7. Последовательная обработка + +Параллелизм не вводится. GitHub применяет secondary rate limits к всплескам параллельных запросов от одного токена, а работа и так фоновая с прогресс-баром: выигрыш во времени не стоит риска упереться в лимит на середине группы. + +## 8. Границы отказа + +Ошибка на одном студенте (нет коммитов, неверный вариант, недоступный репозиторий, неожиданное исключение) записывается в его строку отчёта и не останавливает работу. Останавливают только сбои, общие для всего запуска: нет столбца `GitHub` или столбца лабы, недоступен список репозиториев организации - продолжать после них бессмысленно. + +## 9. Где живёт интерфейс + +Отдельный экран не заводился: преподаватель уже выбирает курс и лабораторную на странице лабораторных в админке (`/admin/courses/{course_id}/labs`), откуда запускается рассылка обновлений шаблона. Массовая проверка добавлена туда же вторым действием в строке. Группа спрашивается в диалоге, потому что список лаб общий для курса, а лист - у каждой группы свой. + +Устройство фоновой работы (хранилище в памяти под блокировкой, `202` с `job_id`, опрос статуса, `409` на повторный запуск) повторяет `grading/propagate.py` - тот же сценарий и те же ограничения. diff --git a/docs/COURSE_CONFIG.md b/docs/COURSE_CONFIG.md index 00d6e4d..db65227 100644 --- a/docs/COURSE_CONFIG.md +++ b/docs/COURSE_CONFIG.md @@ -369,6 +369,21 @@ files: - README.md ``` +### `student-name-file` +**Тип:** `string` +**Описание:** Файл в репозитории студента, первая строка которого содержит его ФИО. Используется только массовой проверкой из админки: по этому ФИО находится строка студента в Google Таблице и в неё записывается GitHub-логин из имени репозитория (см. «Массовая проверка лабораторной» в `docs/PROJECT_DESCRIPTION.md`). Если ключ задан, поле «Файл с ФИО студента» в диалоге проверки предзаполняется этим значением; преподаватель может его изменить или очистить. +**По умолчанию:** не задан - массовая проверка тогда работает только по логинам, уже указанным в таблице +**Пример:** +```yaml +labs: + "1": + github-prefix: os-task1 + short-name: ЛР1 + files: + - info.md + student-name-file: info.md +``` + ### `forbidden-modifications` **Тип:** `list[string]` **Описание:** Список файлов/директорий, которые студент не может изменять diff --git a/docs/PROJECT_DESCRIPTION.md b/docs/PROJECT_DESCRIPTION.md index 3feb4d8..aa9e9dd 100644 --- a/docs/PROJECT_DESCRIPTION.md +++ b/docs/PROJECT_DESCRIPTION.md @@ -89,6 +89,7 @@ 2. **Редактирование конфигураций** — YAML редактор с подсветкой синтаксиса 3. **Загрузка новых курсов** — импорт конфигураций через веб-интерфейс 4. **Аутентификация** — защищенный доступ к административным функциям +5. **Массовая проверка лабораторных** — проверка всей группы одной кнопкой (см. «Массовая проверка лабораторной») ### Процесс проверки работы @@ -191,9 +192,12 @@ lab_grader_web/ | GET | `/courses/{course_id}/edit` | Получение YAML конфигурации | | PUT | `/courses/{course_id}/edit` | Сохранение изменений курса | | POST | `/courses/upload` | Загрузка нового курса | -| GET | `/admin/courses/{course_id}/labs` | Список лаб курса для админки (номер, github-prefix, template-repo, repo-provisioning) | +| GET | `/admin/courses/{course_id}/labs` | Список лаб курса для админки (номер, github-prefix, template-repo, repo-provisioning, files, student-name-file) | | POST | `/admin/courses/{course_id}/labs/{lab_id}/propagate-template-update` | Рассылка обновлений шаблона в репозитории студентов через fork PR (только `repo-provisioning: fork`, см. ниже) | | GET | `/admin/propagate-jobs/{job_id}` | Статус фоновой рассылки обновлений | +| POST | `/admin/courses/{course_id}/groups/{group_id}/labs/{lab_id}/bulk-grade` | Массовая проверка лабораторной для группы (см. ниже) | +| GET | `/admin/bulk-grade-jobs/{job_id}` | Прогресс и отчёт массовой проверки | +| POST | `/admin/bulk-grade-jobs/{job_id}/cancel` | Остановка массовой проверки | Все административные маршруты (включая перечисленные выше) защищены зависимостью `require_admin` - её отсутствие раньше означало, что `POST /courses/upload`, `DELETE /courses/{course_id}` и `GET/PUT /courses/{course_id}/edit` были доступны без аутентификации на уровне API (защита была только на уровне фронтенда). @@ -209,6 +213,38 @@ lab_grader_web/ - Технически PR открывается не «из шаблона в форк»: GitHub не даёт создать кросс-репо pull request, когда шаблон и репозитории студентов принадлежат одной организации (`head` вида `owner:branch` в этом случае разрешается в сам целевой репозиторий). Вместо этого сервис пользуется тем, что форк и шаблон делят хранилище объектов: в репозиторий студента создаётся служебная ветка `template-update`, указывающая на текущий коммит шаблона, и PR открывается из неё в ветку по умолчанию. При каждой новой рассылке ветка перемещается на актуальный коммит - уже открытый PR при этом обновляется, а не дублируется. Студенту работать с этой веткой не нужно, но она видна в списке веток его репозитория. - Прогон CI после мержа PR зависит от `paths`-фильтров workflow конкретной лабы в репозитории-шаблоне - обновление файлов вне этих фильтров не запустит проверку автоматически. +### Массовая проверка лабораторной (`bulk-grade`) + +Проверка всей группы одной кнопкой, вместо ожидания, пока каждый студент сам отправит работу. Запускается со страницы лабораторных курса в админке (`/admin/courses/{course_id}/labs`): у каждой лабы есть действие «Заполнить таблицу», в диалоге выбирается группа и, при необходимости, файл с ФИО. + +Каждый студент проходит ровно те же проверки, что и при самостоятельной отправке: обязательные файлы, `.github/workflows`, коммиты, запрещённые модификации, результаты CI, номер варианта, баллы из логов, штраф за просрочку. Решение об оценке принимает `grading/bulk.py:evaluate_student` - общая функция для одиночной проверки (`grade_lab`) и массовой, поэтому их поведение не может разойтись. + +**Два режима, выбираются полем «Файл с ФИО студента»:** + +| Поле | Режим | Что происходит | +|------|-------|----------------| +| заполнено | `by_file` | Перебираются все репозитории организации с префиксом лабы. Из указанного файла берётся первая строка - ФИО студента. По ФИО находится строка в таблице, GitHub-логин из имени репозитория записывается в столбец `GitHub`, затем работа проверяется | +| пусто | `by_sheet` | Проверяются только студенты, у которых GitHub-логин уже указан в таблице | + +Подсказки в поле берутся из списка `files` лабы; ключ `student-name-file` (см. `docs/COURSE_CONFIG.md`) предзаполняет его. + +**Сопоставление ФИО** - точное после нормализации: схлопывание пробелов (включая неразрывные), регистр, `ё` → `е`. Нечёткий подбор не выполняется намеренно: цена ошибки - оценка, записанная не тому студенту. Репозитории, для которых ФИО не нашлось или нашлось дважды, попадают в отчёт со статусами `unmatched` / `ambiguous` и прочитанным ФИО, чтобы преподаватель исправил вручную. + +**Запись GitHub-логина:** пустая ячейка - логин записывается; тот же логин (без учёта регистра) - не перезаписывается; другой логин - `conflict`, строка пропускается целиком и не проверяется ни под одним из логинов. + +**Защита оценок:** действует то же правило `can_overwrite_cell`, что и при самостоятельной отправке - перезаписываются только пустые ячейки, `x` и значения, начинающиеся с `?`. Остальные попадают в отчёт со статусом `rejected` и текущим значением. Флага принудительной перезаписи нет. + +**Пробный запуск** (`dry_run`) выполняет все проверки и строит полный отчёт, не записывая в таблицу ничего - ни оценок, ни логинов. В отличие от `dry_run` у рассылки шаблона, здесь он тоже фоновая работа: проверки настоящие и занимают столько же времени. + +**Технические детали:** + +- Запуск возвращает `202` и `job_id`; работа идёт в фоне (`BackgroundTasks`), прогресс опрашивается через `GET /admin/bulk-grade-jobs/{job_id}` раз в 2 секунды. Хранилище работ - то же по устройству, что у рассылки шаблона (модульный словарь под блокировкой, не переживает перезапуск). Повторный запуск для той же тройки курс/группа/лаба, пока работа выполняется, отклоняется с `409`. +- Лист читается одним вызовом `get_all_values()`, все поиски (строка студента, столбец лабы, дедлайн, номер варианта) выполняются по сетке в памяти. Поячеечные хелперы тратят ~6 обращений к Sheets API на студента, что для группы из 30 человек выходит за квоту в 60 запросов чтения в минуту. +- Запись - пакетами по 10 ячеек по ходу работы, а не одним блоком в конце: падение, перезапуск или отмена сохраняют уже проставленные оценки. +- Студенты обрабатываются последовательно: GitHub применяет secondary rate limits к всплескам параллельных запросов от одного токена. +- Ошибка на одном студенте (нет коммитов, неверный вариант, недоступный репозиторий) не останавливает работу - она попадает в его строку отчёта. Останавливают работу только общие сбои: нет столбца `GitHub` или столбца лабы, недоступен список репозиториев организации. +- Отмена проверяется между студентами: накопленная порция дописывается в таблицу, работа завершается со статусом `cancelled`. + ## Конфигурация курса ### Индексный файл курсов (`courses/index.yaml`) diff --git a/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx b/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx new file mode 100644 index 0000000..c0cb424 --- /dev/null +++ b/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx @@ -0,0 +1,278 @@ +import { useState, useEffect, useRef, useCallback } from "react"; +import { useTranslation } from "react-i18next"; +import { + Autocomplete, + Checkbox, + Chip, + Dialog, + DialogTitle, + DialogContent, + DialogActions, + FormControlLabel, + LinearProgress, + MenuItem, + Table, + TableHead, + TableBody, + TableRow, + TableCell, + TextField, + Button as MuiButton, +} from "@mui/material"; +import { fetchGroups } from "../../../api"; +import { TableWrapper, HintText, StatusChipRow } from "./styled"; + +const JOB_POLL_INTERVAL_MS = 2000; + +const RESULT_STATUS_COLOR = { + updated: "success", + rejected: "warning", + pending: "info", + error: "error", + conflict: "error", + unmatched: "warning", + ambiguous: "warning", +}; + +async function fetchJson(url, options) { + const response = await fetch(url, { credentials: "include", ...options }); + let data = null; + try { + data = await response.json(); + } catch { + // no body + } + if (!response.ok) { + const error = new Error((data && data.detail) || `HTTP ${response.status}`); + error.status = response.status; + throw error; + } + return data; +} + +/** + * Запуск массовой проверки одной лабораторной для одной группы и показ отчёта. + * + * Группа спрашивается здесь, а не на странице: список лабораторных общий для + * курса, а проверка идёт по листу конкретной группы. + */ +export const BulkGradeDialog = ({ courseId, lab, onClose, onError }) => { + const { t } = useTranslation(); + + const [groups, setGroups] = useState([]); + const [groupId, setGroupId] = useState(""); + const [nameFile, setNameFile] = useState(lab.name_file || ""); + const [dryRun, setDryRun] = useState(false); + const [starting, setStarting] = useState(false); + + const [job, setJob] = useState(null); + const pollRef = useRef(null); + + useEffect(() => { + fetchGroups(courseId) + .then(setGroups) + .catch((err) => onError(err.message || t("adminLabs.bulk.errors.groupsFailed"))); + }, [courseId, onError, t]); + + const stopPolling = () => { + if (pollRef.current) { + clearInterval(pollRef.current); + pollRef.current = null; + } + }; + + useEffect(() => stopPolling, []); + + const pollJob = useCallback((jobId) => { + stopPolling(); + const poll = () => { + fetchJson(`/api/v1/admin/bulk-grade-jobs/${jobId}`) + .then((data) => { + setJob(data); + if (data.status !== "running") { + stopPolling(); + } + }) + .catch(() => { + stopPolling(); + }); + }; + poll(); + pollRef.current = setInterval(poll, JOB_POLL_INTERVAL_MS); + }, []); + + const handleStart = () => { + setStarting(true); + fetchJson( + `/api/v1/admin/courses/${courseId}/groups/${encodeURIComponent(groupId)}/labs/${encodeURIComponent(lab.id)}/bulk-grade`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name_file: nameFile || null, dry_run: dryRun }), + } + ) + .then((data) => { + setStarting(false); + setJob({ job_id: data.job_id, status: "running", total: 0, processed: 0, results: [] }); + pollJob(data.job_id); + }) + .catch((err) => { + setStarting(false); + if (err.status === 409) { + onError(t("adminLabs.bulk.errors.alreadyRunning")); + } else { + onError(err.message || t("adminLabs.bulk.errors.startFailed")); + } + }); + }; + + const handleCancelJob = () => { + if (!job) return; + fetchJson(`/api/v1/admin/bulk-grade-jobs/${job.job_id}/cancel`, { method: "POST" }) + .then(setJob) + .catch((err) => onError(err.message || t("adminLabs.bulk.errors.cancelFailed"))); + }; + + const handleClose = () => { + stopPolling(); + onClose(); + }; + + const running = job && job.status === "running"; + const results = (job && job.results) || []; + + return ( + + {t("adminLabs.bulk.title", { lab: lab.short_name })} + + {!job && ( + <> + setGroupId(e.target.value)} + disabled={groups.length === 0} + > + {groups.map((group) => ( + + {group} + + ))} + + + setNameFile(value || "")} + onInputChange={(_, value) => setNameFile(value || "")} + renderInput={(params) => ( + + )} + /> + + + {nameFile ? t("adminLabs.bulk.modeByFile") : t("adminLabs.bulk.modeBySheet")} + + + setDryRun(e.target.checked)} /> + } + label={t("adminLabs.bulk.dryRun")} + /> + + )} + + {job && ( + <> +

+ {running && t("adminLabs.bulk.inProgress", { processed: job.processed, total: job.total })} + {job.status === "done" && t("adminLabs.bulk.done")} + {job.status === "cancelled" && t("adminLabs.bulk.cancelled")} + {job.status === "failed" && + `${t("adminLabs.bulk.failed")}${job.error ? `: ${job.error}` : ""}`} +

+ + {job.dry_run && } + + {running && ( + + )} + + {job.counts && Object.keys(job.counts).length > 0 && ( + + {Object.entries(job.counts).map(([status, count]) => ( + + ))} + + )} + + {results.length > 0 && ( + + + + + {t("adminLabs.bulk.columns.student")} + {t("adminLabs.bulk.columns.github")} + {t("adminLabs.bulk.columns.status")} + {t("adminLabs.bulk.columns.grade")} + {t("adminLabs.bulk.columns.message")} + + + + {results.map((r, index) => ( + + {r.student_name || "—"} + + {r.github || "—"} + {r.registered && ` (${t("adminLabs.bulk.registered")})`} + + + + + {r.grade || "—"} + {r.message} + + ))} + +
+ + )} + + )} + + + {running ? ( + + {t("adminLabs.bulk.cancelRun")} + + ) : ( + {t("adminLabs.bulk.close")} + )} + {!job && ( + + {t("adminLabs.bulk.start")} + + )} + + + ); +}; diff --git a/frontend/courses-front/src/components/admin/LabList/index.jsx b/frontend/courses-front/src/components/admin/LabList/index.jsx index 5c55d53..f7101aa 100644 --- a/frontend/courses-front/src/components/admin/LabList/index.jsx +++ b/frontend/courses-front/src/components/admin/LabList/index.jsx @@ -18,6 +18,7 @@ import { Button as MuiButton, Checkbox, } from "@mui/material"; +import { BulkGradeDialog } from "./BulkGradeDialog"; import { Container, Panel, @@ -76,6 +77,9 @@ export const LabList = ({ courseId, onBack }) => { const [job, setJob] = useState(null); const pollRef = useRef(null); + // Лаба, для которой открыт диалог массовой проверки (независим от рассылки) + const [bulkLab, setBulkLab] = useState(null); + const showSnackbar = (message, severity = "info") => setSnackbar({ open: true, message, severity }); const loadLabs = useCallback(() => { @@ -252,6 +256,14 @@ export const LabList = ({ courseId, onBack }) => { )} + setBulkLab(lab)} + > + {t("adminLabs.bulk.button")} + ))} @@ -401,6 +413,15 @@ export const LabList = ({ courseId, onBack }) => { + {bulkLab && ( + setBulkLab(null)} + onError={(message) => showSnackbar(message, "error")} + /> + )} + int | None: + """ + Resolve the 1-based column holding student order numbers for TASKID checks. + + Args: + course_info: Course configuration dict from YAML + lab_config: Lab configuration dict from YAML + + Returns: + 1-based column number, or None when the TASKID check does not apply to + this lab (no `task-id-column`, no `taskid-max`, or `ignore-task-id`) + """ + column = course_info.get("google", {}).get("task-id-column") + if column is None: + return None + + if lab_config.get("taskid-max") is None: + return None + + if lab_config.get("ignore-task-id", False): + return None + + # 0-based in config, 1-based for gspread + return column + 1 + + +def repo_name_for(lab_config: dict[str, Any], username: str) -> str: + """ + Build a student's repository name by the project's naming convention. + + Args: + lab_config: Lab configuration dict from YAML + username: Student's GitHub username + + Returns: + Repository name, e.g. "os-task2-student1" + """ + return f"{lab_config.get('github-prefix', '')}-{username}" + + +def evaluate_student( + grader: LabGrader, + org: str, + username: str, + lab_config: dict[str, Any], + course_info: dict[str, Any], + sheet_context: Callable[[], SheetContext], +) -> StudentOutcome: + """ + Grade one student's repository. + + Steps: + 1. Repository checks (required files, workflows, commits) + 2. Forbidden file modifications + 3. CI evaluation and score extraction + 4. TASKID validation (if configured) + 5. Penalty calculation (if a deadline is set) + 6. Grade formatting and cell protection check + + Args: + grader: Configured LabGrader + org: GitHub organization + username: Student's GitHub username + lab_config: Lab configuration dict from YAML + course_info: Course configuration dict from YAML + sheet_context: Callable returning the spreadsheet context. Invoked at + most once, and only after CI evaluation succeeds, so callers may + defer opening a Sheets connection until then. + + Returns: + StudentOutcome. For status "updated", `cell_value` is what should be + written to the grade cell; the caller performs the write. + """ + repo_name = repo_name_for(lab_config, username) + logger.info(f"Evaluating repository: {org}/{repo_name}") + + # Step 1: Repository checks (required files, workflows, commits) + repo_error = grader.check_repository(org, repo_name, lab_config) + if repo_error: + logger.warning(f"Repository check failed: {repo_error.message}") + return StudentOutcome( + status="error", + message=repo_error.message, + error_code=repo_error.error_code, + ) + + # Step 2: Forbidden file modifications + forbidden_error = grader.check_forbidden_files(org, repo_name, lab_config) + if forbidden_error: + logger.warning(f"Forbidden modification: {forbidden_error.message}") + return StudentOutcome( + status="error", + message=forbidden_error.message, + error_code=forbidden_error.error_code, + ) + + # Step 3: CI evaluation + ci_evaluation = grader._evaluate_ci_internal(org, repo_name, lab_config) + + if ci_evaluation.grade_result.status == GradeStatus.ERROR: + logger.warning(f"CI error: {ci_evaluation.grade_result.message}") + return StudentOutcome( + status="error", + message=ci_evaluation.grade_result.message, + error_code=ci_evaluation.grade_result.error_code, + ) + + if ci_evaluation.grade_result.status == GradeStatus.PENDING: + logger.info(f"CI pending: {ci_evaluation.grade_result.message}") + return StudentOutcome( + status="pending", + message=ci_evaluation.grade_result.message, + passed=ci_evaluation.grade_result.passed, + checks=ci_evaluation.grade_result.checks, + ) + + # CI evaluation is complete - the spreadsheet is needed from here on + context = sheet_context() + + final_result = ci_evaluation.grade_result.result # "v" or "x" + final_message = ci_evaluation.grade_result.message + score_value = ci_evaluation.score + decimal_separator = context.decimal_separator + + # Steps 4-5: additional checks only make sense when CI passed + if ci_evaluation.ci_passed: + if taskid_column(course_info, lab_config) is not None and context.student_order is not None: + taskid_shift = lab_config.get("taskid-shift", 0) + taskid_max = lab_config.get("taskid-max") + expected_taskid = calculate_expected_taskid( + context.student_order, taskid_shift, taskid_max + ) + logger.info( + f"Expected TASKID: {expected_taskid} " + f"(order={context.student_order}, shift={taskid_shift}, max={taskid_max})" + ) + + taskid_error = grader.check_taskid( + org, repo_name, + ci_evaluation.successful_runs, + expected_taskid, + ) + if taskid_error: + logger.warning(f"TASKID error: {taskid_error.message}") + return StudentOutcome( + status="error", + message=taskid_error.message, + error_code=taskid_error.error_code, + ) + + penalty = 0 + if context.deadline and ci_evaluation.latest_success_time: + penalty_max = lab_config.get("penalty-max", 0) + strategy_name = lab_config.get("penalty-strategy", "weekly") + try: + strategy = PenaltyStrategy(strategy_name) + except ValueError: + strategy = PenaltyStrategy.WEEKLY + + penalty = calculate_penalty( + completed_at=ci_evaluation.latest_success_time, + deadline=context.deadline, + penalty_max=penalty_max, + strategy=strategy, + ) + + if penalty > 0: + logger.info(f"Calculated penalty: {penalty}") + + # Step 6: format the grade with score and penalty + if score_value is not None: + final_result = format_grade_with_score( + "v", score_value, penalty, decimal_separator + ) + logger.info(f"Formatted grade with score: {final_result}") + + formatted_score = format_score(score_value, decimal_separator) + if penalty > 0: + final_message = ( + f"Результат CI: ✅ Все проверки пройдены " + f"(Баллы: {formatted_score}, штраф: -{penalty})" + ) + else: + final_message = ( + f"Результат CI: ✅ Все проверки пройдены (Баллы: {formatted_score})" + ) + elif penalty > 0: + final_result = format_grade_with_penalty("v", penalty) + final_message = f"Результат CI: ✅ Все проверки пройдены (штраф: -{penalty})" + logger.info(f"Applied penalty {penalty} for late submission: {final_result}") + + formatted_score = ( + format_score(score_value, decimal_separator) if score_value is not None else None + ) + + # Cell protection + if not can_overwrite_cell(context.current_cell_value): + logger.warning( + f"Update rejected: cell already contains '{context.current_cell_value}'" + ) + return StudentOutcome( + status="rejected", + cell_value=context.current_cell_value, + message=CELL_PROTECTED_MESSAGE, + passed=ci_evaluation.grade_result.passed, + checks=ci_evaluation.grade_result.checks, + score=formatted_score, + current_grade=context.current_cell_value, + ) + + return StudentOutcome( + status="updated", + cell_value=final_result, + message=final_message, + passed=ci_evaluation.grade_result.passed, + checks=ci_evaluation.grade_result.checks, + score=formatted_score, + ) + + +# --------------------------------------------------------------------------- +# Repository discovery and student matching ("by file" mode) +# --------------------------------------------------------------------------- + + +def filter_lab_repos(repo_names: list[str], prefix: str) -> dict[str, str]: + """ + Select the repositories belonging to one lab and extract usernames. + + The dash after the prefix is required, so the prefix "os-task1" does not + swallow "os-task10-student1". + + Args: + repo_names: All repository names in the organization + prefix: Lab's github-prefix + + Returns: + Mapping of GitHub username -> repository name + + Examples: + >>> filter_lab_repos(["os-task1-alice", "os-task10-bob"], "os-task1") + {'alice': 'os-task1-alice'} + >>> filter_lab_repos(["os-task1-jane-doe"], "os-task1") + {'jane-doe': 'os-task1-jane-doe'} + """ + if not prefix: + return {} + + matched: dict[str, str] = {} + marker = f"{prefix}-" + + for name in repo_names: + if not name.startswith(marker): + continue + + username = name[len(marker):] + if not username: + continue + + matched[username] = name + + return matched + + +def extract_full_name(file_content: str | None) -> str | None: + """ + Extract a student's full name from the contents of their name file. + + The name is the first non-empty line of the file. + + Args: + file_content: Decoded file content, or None if the file is unavailable + + Returns: + The name with surrounding whitespace stripped, or None if the file is + empty or holds only blank lines + + Examples: + >>> extract_full_name("Иванов Иван Иванович\\nЛР1\\n") + 'Иванов Иван Иванович' + >>> extract_full_name("\\r\\n Петров Пётр \\r\\n") + 'Петров Пётр' + >>> extract_full_name(" ") is None + True + """ + if not file_content: + return None + + for line in file_content.splitlines(): + stripped = line.strip() + if stripped: + return stripped + + return None + + +def normalize_full_name(name: str | None) -> str: + """ + Normalize a full name for comparison. + + Students type their name by hand into the name file, so comparison ignores + letter case, collapses whitespace runs (including non-breaking spaces) and + treats "ё" as "е". + + Args: + name: Raw name string + + Returns: + Normalized name, or "" for an empty input + + Examples: + >>> normalize_full_name("Иванов Иван\\tИванович") + 'иванов иван иванович' + >>> normalize_full_name("Алёшин Алексей") == normalize_full_name("Алешин Алексей") + True + """ + if not name: + return "" + + collapsed = " ".join(name.replace(" ", " ").split()) + return collapsed.casefold().replace("ё", "е") + + +class NameMatchError(Exception): + """Raised when a full name cannot be resolved to exactly one sheet row.""" + + def __init__(self, code: str, message: str): + super().__init__(message) + self.code = code + self.message = message + + +def find_row_by_full_name( + student_names: list[str], + full_name: str, + start_row: int = FIRST_DATA_ROW, +) -> int: + """ + Find the spreadsheet row for a student by full name. + + Matching is exact after normalization (see normalize_full_name). Fuzzy + matching is deliberately not attempted: a wrong match writes a grade + against the wrong student. + + Args: + student_names: Values of the student name column, starting at start_row + full_name: Name read from the repository's name file + start_row: 1-based row number of the first entry in student_names + + Returns: + 1-based row number + + Raises: + NameMatchError: code "unmatched" when no row matches, "ambiguous" when + several rows carry the same name + """ + target = normalize_full_name(full_name) + if not target: + raise NameMatchError("unmatched", "ФИО не найдено в файле") + + matches = [ + start_row + idx + for idx, value in enumerate(student_names) + if normalize_full_name(value) == target + ] + + if not matches: + raise NameMatchError("unmatched", f"ФИО «{full_name}» не найдено в таблице") + + if len(matches) > 1: + raise NameMatchError( + "ambiguous", + f"ФИО «{full_name}» встречается в таблице несколько раз " + f"(строки {', '.join(str(row) for row in matches)})", + ) + + return matches[0] + + +def resolve_github_cell(existing: str, username: str) -> tuple[bool, str | None]: + """ + Decide what to do with a student's GitHub cell in "by file" mode. + + Args: + existing: Current cell value + username: Username taken from the repository name + + Returns: + (should_write, conflict_message). `should_write` is True when the cell + is empty and the username has to be recorded. When the cell holds a + different username, `conflict_message` explains the conflict and the + student must not be graded under either name. + + Examples: + >>> resolve_github_cell("", "alice") + (True, None) + >>> resolve_github_cell("Alice", "alice") + (False, None) + """ + current = (existing or "").strip() + + if not current: + return True, None + + if current.casefold() == username.casefold(): + return False, None + + return False, ( + f"В таблице указан другой аккаунт GitHub: «{current}», " + f"репозиторий принадлежит «{username}»" + ) + + +# --------------------------------------------------------------------------- +# Background job: store, planning and execution +# --------------------------------------------------------------------------- + + +@dataclass +class BulkResult: + """Outcome for a single student, as shown in the run report.""" + status: str # updated | rejected | pending | error | conflict | unmatched | ambiguous + student_name: str | None = None + github: str | None = None + repo: str | None = None + grade: str | None = None + message: str = "" + registered: bool = False # GitHub username was written to the sheet + + def to_dict(self) -> dict: + return { + "status": self.status, + "student_name": self.student_name, + "github": self.github, + "repo": self.repo, + "grade": self.grade, + "message": self.message, + "registered": self.registered, + } + + +@dataclass +class BulkJob: + """State of one background bulk grading run.""" + job_id: str + course_id: str + group_id: str + lab_id: str + mode: str # by_sheet | by_file + dry_run: bool = False + name_file: str | None = None + status: str = "running" # running | done | failed | cancelled + started_at: str = "" + finished_at: str | None = None + total: int = 0 + processed: int = 0 + results: list[BulkResult] = field(default_factory=list) + error: str | None = None + cancel_requested: bool = False + + def to_dict(self) -> dict: + counts: dict[str, int] = {} + for result in self.results: + counts[result.status] = counts.get(result.status, 0) + 1 + + return { + "job_id": self.job_id, + "course_id": self.course_id, + "group_id": self.group_id, + "lab_id": self.lab_id, + "mode": self.mode, + "dry_run": self.dry_run, + "name_file": self.name_file, + "status": self.status, + "started_at": self.started_at, + "finished_at": self.finished_at, + "total": self.total, + "processed": self.processed, + "counts": counts, + "results": [r.to_dict() for r in self.results], + "error": self.error, + } + + +# Module-level job store, same shape and rationale as propagate.py: guarded by +# a lock because BackgroundTasks run in FastAPI's threadpool, and safe as a +# plain dict only as long as the backend stays a single uvicorn worker. +_jobs: "OrderedDict[str, BulkJob]" = OrderedDict() +_running_keys: set[tuple[str, str, str]] = set() +_jobs_lock = threading.Lock() + + +def _now() -> str: + return datetime.now(timezone.utc).isoformat() + + +def get_bulk_job(job_id: str) -> BulkJob | None: + """Look up a job by id (used by GET /admin/bulk-grade-jobs/{job_id}).""" + with _jobs_lock: + return _jobs.get(job_id) + + +def _evict_old_jobs_locked() -> None: + """Keep at most MAX_JOBS_KEPT jobs, oldest first. Must hold _jobs_lock.""" + if len(_jobs) <= MAX_JOBS_KEPT: + return + for job_id in list(_jobs.keys()): + if len(_jobs) <= MAX_JOBS_KEPT: + break + # Never evict a running job - it would leave _running_keys pointing at + # a job GET can no longer find. + if _jobs[job_id].status == "running": + continue + del _jobs[job_id] + + +def try_start_bulk_job( + course_id: str, + group_id: str, + lab_id: str, + mode: str, + dry_run: bool, + name_file: str | None, +) -> BulkJob | None: + """ + Atomically create and register a running job for (course, group, lab), + unless one is already running for that same triple. + + Returns: + The new BulkJob, or None if a job for this group and lab is already + running (caller should respond HTTP 409). + """ + with _jobs_lock: + if (course_id, group_id, lab_id) in _running_keys: + return None + job = BulkJob( + job_id=uuid.uuid4().hex, + course_id=course_id, + group_id=group_id, + lab_id=lab_id, + mode=mode, + dry_run=dry_run, + name_file=name_file, + started_at=_now(), + ) + _jobs[job.job_id] = job + _running_keys.add((course_id, group_id, lab_id)) + _evict_old_jobs_locked() + return job + + +def request_bulk_job_cancel(job_id: str) -> BulkJob | None: + """ + Ask a running job to stop after the student it is currently on. + + Returns: + The job, or None if there is no job with this id. + """ + with _jobs_lock: + job = _jobs.get(job_id) + if job and job.status == "running": + job.cancel_requested = True + return job + + +def _finish_job_locked(job: BulkJob, status: str, error: str | None = None) -> None: + job.status = status + job.error = error + job.finished_at = _now() + _running_keys.discard((job.course_id, job.group_id, job.lab_id)) + + +@dataclass +class _Target: + """A student queued for grading.""" + row: int + username: str + student_name: str | None + repo: str + registered: bool = False # Username was queued for writing into the sheet + + +def _plan_by_file( + job: BulkJob, + github_client: GitHubClient, + org: str, + lab_config: dict[str, Any], + name_file: str, + values: list[list[str]], + student_col: int, + github_col: int, +) -> tuple[list[_Target], list[tuple[int, int, str]]]: + """ + Discover repositories and map them to spreadsheet rows via the name file. + + Rows that cannot be resolved are appended to `job.results` here - they are + finished work, not something to retry per student. + + Returns: + (targets, github_writes), where github_writes are (row, col, value) + triples recording newly resolved GitHub usernames + + Raises: + BulkGradingError: the organization's repositories are unavailable + """ + prefix = lab_config.get("github-prefix", "") + + org_repos = github_client.list_org_repos(org) + if org_repos is None: + raise BulkGradingError("Не удалось получить список репозиториев организации") + + repos = filter_lab_repos([repo.get("name", "") for repo in org_repos], prefix) + logger.info(f"Found {len(repos)} repositories with prefix '{prefix}' in {org}") + + student_names = column_values_from_grid(values, student_col, start_row=FIRST_DATA_ROW) + + targets: list[_Target] = [] + github_writes: list[tuple[int, int, str]] = [] + + for username, repo in sorted(repos.items(), key=lambda pair: pair[0].casefold()): + full_name = extract_full_name( + github_client.get_file_content(org, repo, name_file) + ) + if full_name is None: + job.results.append(BulkResult( + status="unmatched", + github=username, + repo=repo, + message=f"Файл {name_file} не найден, пуст или не читается как текст", + )) + continue + + try: + row = find_row_by_full_name(student_names, full_name, FIRST_DATA_ROW) + except NameMatchError as e: + job.results.append(BulkResult( + status=e.code, + student_name=full_name, + github=username, + repo=repo, + message=e.message, + )) + continue + + should_write, conflict = resolve_github_cell( + cell_from_grid(values, row, github_col), username + ) + if conflict: + job.results.append(BulkResult( + status="conflict", + student_name=full_name, + github=username, + repo=repo, + message=conflict, + )) + continue + + if should_write: + github_writes.append((row, github_col, username)) + + targets.append(_Target( + row=row, + username=username, + student_name=cell_from_grid(values, row, student_col) or full_name, + repo=repo, + registered=should_write, + )) + + return targets, github_writes + + +def _plan_by_sheet( + values: list[list[str]], + student_col: int, + github_col: int, + lab_config: dict[str, Any], +) -> list[_Target]: + """Queue every student who already has a GitHub username in the sheet.""" + targets: list[_Target] = [] + + github_values = column_values_from_grid(values, github_col, start_row=FIRST_DATA_ROW) + for idx, value in enumerate(github_values): + username = (value or "").strip() + if not username: + continue + + row = FIRST_DATA_ROW + idx + targets.append(_Target( + row=row, + username=username, + student_name=cell_from_grid(values, row, student_col) or None, + repo=repo_name_for(lab_config, username), + )) + + return targets + + +def run_bulk_grading( + job: BulkJob, + grader: LabGrader, + github_client: GitHubClient, + worksheet, + spreadsheet, + course_info: dict[str, Any], + lab_config: dict[str, Any], + lab_number: int, +) -> None: + """ + Execute a bulk grading job, updating `job` in place as it goes. + + Reads the whole worksheet once and resolves rows, columns, deadline and + task IDs in memory. The per-cell helpers spend ~6 Sheets API requests per + student, which for a group of 30 exceeds the 60 reads/minute quota. + + Students are processed sequentially: GitHub applies secondary rate limits + to bursts of parallel requests from one token, and the run is backgrounded + anyway. + + Args: + job: Job to run and report progress into + grader: Configured LabGrader + github_client: Configured GitHubClient + worksheet: gspread Worksheet for the group + spreadsheet: gspread Spreadsheet (used for the locale) + course_info: Course configuration dict from YAML + lab_config: Lab configuration dict from YAML + lab_number: Lab number from its config key, for the lab-column fallback + """ + from gspread.utils import rowcol_to_a1 + + org = course_info.get("github", {}).get("organization") + pending: list[tuple[int, int, str]] = [] + + def flush() -> None: + """Write buffered cells to the spreadsheet in one API call.""" + if job.dry_run or not pending: + pending.clear() + return + + worksheet.batch_update([ + {"range": rowcol_to_a1(row, col), "values": [[value]]} + for row, col, value in pending + ]) + logger.info(f"Bulk job {job.job_id}: flushed {len(pending)} cell(s)") + pending.clear() + + try: + values = worksheet.get_all_values() + decimal_separator = get_decimal_separator(spreadsheet) + + header_row = values[0] if values else [] + if "GitHub" not in header_row: + raise BulkGradingError("Столбец 'GitHub' не найден в таблице") + github_col = header_row.index("GitHub") + 1 + + student_col = course_info.get("google", {}).get("student-name-column", 1) + 1 + + lab_short_name = lab_config.get("short-name") + if lab_short_name: + lab_col = find_lab_column_in_grid(values, lab_short_name) + if not lab_col: + raise BulkGradingError(f"Столбец '{lab_short_name}' не найден в таблице") + else: + # Same fallback as grade_lab when a lab has no short-name. + lab_offset = course_info.get("google", {}).get("lab-column-offset", 1) + lab_col = calculate_lab_column(lab_number, lab_offset) + + deadline = get_deadline_from_grid( + values, lab_col, deadline_row=1, timezone_str=course_info.get("timezone") + ) + task_id_col = taskid_column(course_info, lab_config) + + if job.mode == "by_file": + targets, github_writes = _plan_by_file( + job, github_client, org, lab_config, job.name_file, + values, student_col, github_col, + ) + pending.extend(github_writes) + else: + targets = _plan_by_sheet(values, student_col, github_col, lab_config) + + # Rows rejected while planning are already done; count them as processed + job.total = len(targets) + len(job.results) + job.processed = len(job.results) + logger.info( + f"Bulk job {job.job_id}: {len(targets)} student(s) to grade, " + f"{len(job.results)} rejected while planning" + ) + + cancelled = False + for target in targets: + if job.cancel_requested: + logger.info(f"Bulk job {job.job_id}: cancellation requested") + cancelled = True + break + + def context_for(target=target) -> SheetContext: + return SheetContext( + current_cell_value=cell_from_grid(values, target.row, lab_col), + student_order=( + get_student_order_from_grid(values, target.row, task_id_col) + if task_id_col is not None else None + ), + deadline=deadline, + decimal_separator=decimal_separator, + ) + + try: + outcome = evaluate_student( + grader, org, target.username, lab_config, course_info, context_for, + ) + result = BulkResult( + status=outcome.status, + student_name=target.student_name, + github=target.username, + repo=target.repo, + grade=outcome.cell_value if outcome.status in ("updated", "rejected") else None, + message=outcome.message, + registered=target.registered, + ) + if outcome.status == "updated": + pending.append((target.row, lab_col, outcome.cell_value)) + except Exception as e: + logger.exception(f"Bulk job {job.job_id}: error grading {target.username}") + result = BulkResult( + status="error", + student_name=target.student_name, + github=target.username, + repo=target.repo, + message=f"Внутренняя ошибка при проверке: {e}", + registered=target.registered, + ) + + job.results.append(result) + job.processed += 1 + + if len(pending) >= WRITE_BATCH_SIZE: + flush() + + flush() + + with _jobs_lock: + _finish_job_locked(job, "cancelled" if cancelled else "done") + + except Exception as e: + logger.exception(f"Bulk job {job.job_id} failed: {e}") + try: + flush() + except Exception: + logger.exception(f"Bulk job {job.job_id}: could not flush pending writes") + with _jobs_lock: + _finish_job_locked(job, "failed", str(e)) + finally: + logger.info( + f"Bulk job {job.job_id} finished with status '{job.status}': " + f"{job.processed}/{job.total} processed" + ) diff --git a/grading/github_client.py b/grading/github_client.py index 4a1e945..56bfd1c 100644 --- a/grading/github_client.py +++ b/grading/github_client.py @@ -4,10 +4,15 @@ This module provides a client for interacting with GitHub API to check repositories, commits, and CI status. """ +import base64 +import binascii import requests from dataclasses import dataclass from typing import Any +# Text files above this size are not fetched for content extraction +MAX_TEXT_FILE_SIZE = 1024 * 1024 + @dataclass class CommitInfo: @@ -102,6 +107,59 @@ def file_exists(self, org: str, repo: str, path: str) -> bool: resp = requests.get(url, headers=self.headers) return resp.status_code == 200 + def get_file_content( + self, + org: str, + repo: str, + path: str, + max_size: int = MAX_TEXT_FILE_SIZE, + ) -> str | None: + """ + Read a repository file as text. + + Used by bulk grading to pull the student's full name out of the file + named by the lab's `student-name-file`. + + Args: + org: Organization or user name + repo: Repository name + path: File path within repository + max_size: Skip files larger than this many bytes + + Returns: + Decoded text (BOM stripped), or None if the file is missing, too + large, a directory, or not valid UTF-8 + """ + url = f"{self.BASE_URL}/repos/{org}/{repo}/contents/{path}" + resp = requests.get(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT) + + if resp.status_code != 200: + return None + + data = resp.json() + + # A directory path comes back as a list of entries, not file content + if not isinstance(data, dict) or data.get("type") != "file": + return None + + # GitHub omits the body of large files, answering with encoding "none" + if data.get("size", 0) > max_size or data.get("encoding") != "base64": + return None + + content = data.get("content") + if content is None: + return None + + try: + raw = base64.b64decode(content) + except (binascii.Error, ValueError): + return None + + try: + return raw.decode("utf-8-sig") + except UnicodeDecodeError: + return None + def check_required_files( self, org: str, diff --git a/grading/sheets_client.py b/grading/sheets_client.py index f8eef9a..b526164 100644 --- a/grading/sheets_client.py +++ b/grading/sheets_client.py @@ -50,8 +50,8 @@ def find_student_row( Examples: >>> find_student_row(["user1", "user2", "user3"], "user2") 4 - >>> find_student_row(["user1", "user2"], "unknown") - None + >>> find_student_row(["user1", "user2"], "unknown") is None + True """ # Handle case-insensitive comparison github_lower = github_username.lower() @@ -241,6 +241,35 @@ def get_deadline_from_sheet( Returns: datetime object with timezone or None if not found/parseable + Supported formats: + - "DD.MM.YYYY" (e.g., "15.03.2025") + - "DD.MM.YYYY HH:MM" (e.g., "15.03.2025 23:59") + - "YYYY-MM-DD" (e.g., "2025-03-15") + - "YYYY-MM-DDTHH:MM:SS" (ISO format) + - Dates with timezone info (e.g., "2025-03-15T23:59:59+03:00") + """ + try: + cell_value = worksheet.cell(deadline_row, lab_col).value + return parse_deadline(cell_value, timezone_str) + except Exception as e: + logger.error(f"Error reading deadline: {e}") + return None + + +def parse_deadline( + cell_value: str | None, + timezone_str: str | None = None, +) -> datetime | None: + """ + Parse a deadline cell value into a timezone-aware datetime. + + Args: + cell_value: Raw cell content (may be None or empty) + timezone_str: Timezone string (e.g., "UTC+3", "UTC-5") to apply if date is naive + + Returns: + datetime object with timezone or None if empty/unparseable + Supported formats: - "DD.MM.YYYY" (e.g., "15.03.2025") - "DD.MM.YYYY HH:MM" (e.g., "15.03.2025 23:59") @@ -251,62 +280,57 @@ def get_deadline_from_sheet( from datetime import timezone, timedelta import re - try: - cell_value = worksheet.cell(deadline_row, lab_col).value - if not cell_value: - return None + if not cell_value: + return None - cell_value = cell_value.strip() - - # Try different date formats - formats = [ - "%d.%m.%Y %H:%M", # 15.03.2025 23:59 - "%d.%m.%Y", # 15.03.2025 - "%Y-%m-%d %H:%M:%S", # 2025-03-15 23:59:59 - "%Y-%m-%d %H:%M", # 2025-03-15 23:59 - "%Y-%m-%d", # 2025-03-15 - "%Y-%m-%dT%H:%M:%S", # ISO format - ] - - parsed_dt = None - for fmt in formats: - try: - parsed_dt = datetime.strptime(cell_value, fmt) - break - except ValueError: - continue - - if parsed_dt is None: - logger.warning(f"Could not parse deadline '{cell_value}' at row {deadline_row}, col {lab_col}") - return None + cell_value = cell_value.strip() + + # Try different date formats + formats = [ + "%d.%m.%Y %H:%M", # 15.03.2025 23:59 + "%d.%m.%Y", # 15.03.2025 + "%Y-%m-%d %H:%M:%S", # 2025-03-15 23:59:59 + "%Y-%m-%d %H:%M", # 2025-03-15 23:59 + "%Y-%m-%d", # 2025-03-15 + "%Y-%m-%dT%H:%M:%S", # ISO format + ] + + parsed_dt = None + for fmt in formats: + try: + parsed_dt = datetime.strptime(cell_value, fmt) + break + except ValueError: + continue + + if parsed_dt is None: + logger.warning(f"Could not parse deadline '{cell_value}'") + return None - # If date was parsed without time (midnight), set to end of day (23:59:59) - # This ensures deadlines like "19.11.2025" mean "until the end of that day" - if parsed_dt.hour == 0 and parsed_dt.minute == 0 and parsed_dt.second == 0: - parsed_dt = parsed_dt.replace(hour=23, minute=59, second=59) - logger.debug(f"Deadline parsed without time, set to end of day: {parsed_dt}") - - # If datetime already has timezone info, return as-is - if parsed_dt.tzinfo is not None: - logger.debug(f"Deadline already has timezone: {parsed_dt}") - return parsed_dt - - # If no timezone and timezone_str provided, apply it - if timezone_str: - # Parse timezone string like "UTC+3" or "UTC-5" - match = re.match(r'UTC([+-]\d+)', timezone_str) - if match: - offset_hours = int(match.group(1)) - tz = timezone(timedelta(hours=offset_hours)) - parsed_dt = parsed_dt.replace(tzinfo=tz) - logger.debug(f"Applied timezone {timezone_str} to deadline: {parsed_dt}") - else: - logger.warning(f"Could not parse timezone string '{timezone_str}', using naive datetime") + # If date was parsed without time (midnight), set to end of day (23:59:59) + # This ensures deadlines like "19.11.2025" mean "until the end of that day" + if parsed_dt.hour == 0 and parsed_dt.minute == 0 and parsed_dt.second == 0: + parsed_dt = parsed_dt.replace(hour=23, minute=59, second=59) + logger.debug(f"Deadline parsed without time, set to end of day: {parsed_dt}") + # If datetime already has timezone info, return as-is + if parsed_dt.tzinfo is not None: + logger.debug(f"Deadline already has timezone: {parsed_dt}") return parsed_dt - except Exception as e: - logger.error(f"Error reading deadline: {e}") - return None + + # If no timezone and timezone_str provided, apply it + if timezone_str: + # Parse timezone string like "UTC+3" or "UTC-5" + match = re.match(r'UTC([+-]\d+)', timezone_str) + if match: + offset_hours = int(match.group(1)) + tz = timezone(timedelta(hours=offset_hours)) + parsed_dt = parsed_dt.replace(tzinfo=tz) + logger.debug(f"Applied timezone {timezone_str} to deadline: {parsed_dt}") + else: + logger.warning(f"Could not parse timezone string '{timezone_str}', using naive datetime") + + return parsed_dt def get_student_order( @@ -380,3 +404,159 @@ def get_decimal_separator(spreadsheet) -> str: except Exception as e: logger.warning(f"Could not determine spreadsheet locale: {e}. Using default separator '.'") return '.' + + +# --------------------------------------------------------------------------- +# Grid-based helpers +# +# Bulk grading reads the whole worksheet once via ``get_all_values()`` and then +# resolves everything in memory. The Sheets API allows 60 read requests per +# minute, while the per-cell helpers above spend ~6 requests per student. +# +# ``values`` is the raw grid returned by gspread: a list of rows, each a list +# of cell strings. Rows may be ragged - trailing empty cells are omitted. +# --------------------------------------------------------------------------- + + +def cell_from_grid(values: list[list[str]], row: int, col: int) -> str: + """ + Read a single cell from the grid using 1-based coordinates. + + Args: + values: Grid from worksheet.get_all_values() + row: 1-based row number + col: 1-based column number + + Returns: + Cell value, or "" if the coordinates are outside the grid + + Examples: + >>> cell_from_grid([["a", "b"], ["c"]], 1, 2) + 'b' + >>> cell_from_grid([["a", "b"], ["c"]], 2, 2) + '' + """ + if row < 1 or col < 1 or row > len(values): + return "" + + row_values = values[row - 1] + if col > len(row_values): + return "" + + return row_values[col - 1] or "" + + +def column_values_from_grid( + values: list[list[str]], + col: int, + start_row: int = 1, +) -> list[str]: + """ + Read a column from the grid using 1-based coordinates. + + Mirrors gspread's ``col_values()``: missing cells become empty strings. + + Args: + values: Grid from worksheet.get_all_values() + col: 1-based column number + start_row: 1-based row to start from (default 1 = from the top) + + Returns: + List of cell values from start_row to the last row of the grid + + Examples: + >>> column_values_from_grid([["a", "b"], ["c", "d"], ["e"]], 2) + ['b', 'd', ''] + >>> column_values_from_grid([["a", "b"], ["c", "d"]], 1, start_row=2) + ['c'] + """ + return [ + cell_from_grid(values, row, col) + for row in range(start_row, len(values) + 1) + ] + + +def find_lab_column_in_grid( + values: list[list[str]], + short_name: str, +) -> int | None: + """ + Find lab column by searching for short_name in the grid. + + Reproduces the scan order of gspread's ``worksheet.find()``: rows top to + bottom, cells left to right, first exact match wins. + + Args: + values: Grid from worksheet.get_all_values() + short_name: Lab short name to find (e.g., "ЛР1") + + Returns: + 1-based column number or None if not found + + Examples: + >>> find_lab_column_in_grid([["№", "ФИО", "GitHub", "ЛР1"]], "ЛР1") + 4 + >>> find_lab_column_in_grid([["№", "ФИО"]], "ЛР1") is None + True + """ + if not short_name: + return None + + for row_values in values: + for idx, value in enumerate(row_values): + if value == short_name: + return idx + 1 + + return None + + +def get_deadline_from_grid( + values: list[list[str]], + lab_col: int, + deadline_row: int = 1, + timezone_str: str | None = None, +) -> datetime | None: + """ + Get deadline datetime from the grid. + + Grid-based counterpart of get_deadline_from_sheet(). + + Args: + values: Grid from worksheet.get_all_values() + lab_col: 1-based column number of the lab + deadline_row: Row number containing deadline (default 1) + timezone_str: Timezone string (e.g., "UTC+3") to apply if date is naive + + Returns: + datetime object with timezone or None if not found/parseable + """ + return parse_deadline(cell_from_grid(values, deadline_row, lab_col), timezone_str) + + +def get_student_order_from_grid( + values: list[list[str]], + row: int, + task_id_column: int, +) -> int | None: + """ + Get student's task ID / order number from the grid. + + Grid-based counterpart of get_student_order(). + + Args: + values: Grid from worksheet.get_all_values() + row: Student's 1-based row number + task_id_column: 1-based column number containing task IDs + + Returns: + Integer order number or None if empty/unparseable + """ + cell_value = cell_from_grid(values, row, task_id_column) + if not cell_value: + return None + + try: + return int(cell_value.strip()) + except (ValueError, TypeError) as e: + logger.warning(f"Could not parse task ID at row {row}, col {task_id_column}: {e}") + return None diff --git a/main.py b/main.py index 911d0b1..86ccfd1 100644 --- a/main.py +++ b/main.py @@ -42,6 +42,13 @@ try_start_propagate_job, run_propagation, get_propagate_job, + SheetContext, + evaluate_student, + taskid_column, + try_start_bulk_job, + get_bulk_job, + request_bulk_job_cancel, + run_bulk_grading, ) # Configure logging to both file and console @@ -662,8 +669,9 @@ def grade_lab(request: Request, course_id: str, group_id: str, lab_id: str, grad """ Grade a lab submission by checking GitHub repository and CI status. - Uses the LabGrader orchestrator for GitHub checks and CI evaluation, - then updates the grade in Google Sheets. + The grading decision itself lives in grading.bulk.evaluate_student, shared + with the bulk admin run; this endpoint supplies the spreadsheet context and + translates the outcome into an HTTP response. Flow (preserves original behavior): 1. GitHub checks (files, workflows, commits, forbidden mods) @@ -698,197 +706,136 @@ def grade_lab(request: Request, course_id: str, group_id: str, lab_id: str, grad grader = LabGrader(github_client) username = grade_request.github - repo_name = f"{repo_prefix}-{username}" - logger.info(f"Checking repository: {org}/{repo_name}") - - # Step 1: Check repository (required files, workflows, commits) - repo_error = grader.check_repository(org, repo_name, lab_config_dict) - if repo_error: - logger.warning(f"Repository check failed: {repo_error.message}") - # Use 404 for "no commits" to match original behavior - status_code = 404 if repo_error.error_code == "NO_COMMITS" else 400 - raise HTTPException(status_code=status_code, detail=repo_error.message) - - # Step 2: Check forbidden file modifications - forbidden_error = grader.check_forbidden_files(org, repo_name, lab_config_dict) - if forbidden_error: - logger.warning(f"Forbidden modification: {forbidden_error.message}") - raise HTTPException(status_code=403, detail=forbidden_error.message) - - # Step 3: Evaluate CI results - ci_evaluation = grader._evaluate_ci_internal(org, repo_name, lab_config_dict) - - # Return early for errors (no Sheets needed) - if ci_evaluation.grade_result.status == GradeStatus.ERROR: - logger.warning(f"CI error: {ci_evaluation.grade_result.message}") - raise HTTPException(status_code=400, detail=ci_evaluation.grade_result.message) - - # Return early for pending (no Sheets needed) - if ci_evaluation.grade_result.status == GradeStatus.PENDING: - logger.info(f"CI pending: {ci_evaluation.grade_result.message}") - return { - "status": "pending", - "message": ci_evaluation.grade_result.message, - "passed": ci_evaluation.grade_result.passed, - "checks": ci_evaluation.grade_result.checks - } - - # CI evaluation complete - now connect to Sheets for writing result - logger.info(f"Connecting to Google Sheets for group {group_id}") - scope = ["https://spreadsheets.google.com/feeds", "https://www.googleapis.com/auth/drive"] - creds = ServiceAccountCredentials.from_json_keyfile_name(CREDENTIALS_FILE, scope) - sheets_client = gspread.authorize(creds) - try: - spreadsheet = sheets_client.open_by_key(spreadsheet_id) - sheet = spreadsheet.worksheet(group_id) - logger.info(f"Successfully opened worksheet '{group_id}'") - except Exception as e: - logger.error(f"Failed to open worksheet '{group_id}': {str(e)}") - raise HTTPException(status_code=404, detail="Группа не найдена в Google Таблице") + # Where the grade goes once evaluate_student produces one. Filled in by + # load_sheet_context(), which only runs if we get that far. + target: dict = {} + + def load_sheet_context() -> SheetContext: + """Open the group's sheet and read everything the grading needs from it.""" + logger.info(f"Connecting to Google Sheets for group {group_id}") + scope = ["https://spreadsheets.google.com/feeds", "https://www.googleapis.com/auth/drive"] + creds = ServiceAccountCredentials.from_json_keyfile_name(CREDENTIALS_FILE, scope) + sheets_client = gspread.authorize(creds) + + try: + spreadsheet = sheets_client.open_by_key(spreadsheet_id) + sheet = spreadsheet.worksheet(group_id) + logger.info(f"Successfully opened worksheet '{group_id}'") + except Exception as e: + logger.error(f"Failed to open worksheet '{group_id}': {str(e)}") + raise HTTPException(status_code=404, detail="Группа не найдена в Google Таблице") + + # Get decimal separator from spreadsheet locale + decimal_separator = get_decimal_separator(spreadsheet) + logger.info(f"Using decimal separator: '{decimal_separator}'") + + # Find GitHub column and student row + header_row = sheet.row_values(1) + try: + github_col_idx = header_row.index("GitHub") + 1 + except ValueError: + logger.error(f"'GitHub' column not found in spreadsheet headers") + raise HTTPException(status_code=400, detail="Столбец 'GitHub' не найден") + + github_values = sheet.col_values(github_col_idx)[2:] + row_idx = find_student_row(github_values, username) + + if row_idx is None: + logger.warning(f"GitHub username '{username}' not found in spreadsheet for group {group_id}") + raise HTTPException(status_code=404, detail="GitHub логин не найден в таблице. Зарегистрируйтесь.") + + # Find lab column + lab_short_name = lab_config_dict.get("short-name") + if lab_short_name: + lab_col = find_lab_column_by_name(sheet, lab_short_name) + if lab_col: + logger.info(f"Found lab column '{lab_short_name}' at column {lab_col}") + else: + logger.error(f"Lab column '{lab_short_name}' not found in spreadsheet") + raise HTTPException(status_code=400, detail=f"Столбец '{lab_short_name}' не найден в таблице") + else: + logger.warning(f"Lab config for '{lab_id}' is missing 'short-name', using offset calculation") + lab_offset = course_info.get("google", {}).get("lab-column-offset", 1) + # Номер берём из ключа конфига: строка клиента может быть short-name. + lab_number = parse_lab_id(lab_key or lab_id) + lab_col = calculate_lab_column(lab_number, lab_offset) + logger.info(f"Calculated lab column using offset: {lab_offset} + {lab_number} = {lab_col}") + + # Get current cell value for protection check + current_value = sheet.cell(row_idx, lab_col).value or "" + logger.info(f"Current cell value at row {row_idx}, column {lab_col}: '{current_value}'") + + # Student order is only read when the TASKID check applies to this lab + student_order = None + task_id_column = taskid_column(course_info, lab_config_dict) + if task_id_column is not None: + student_order = get_student_order(sheet, row_idx, task_id_column) - # Get decimal separator from spreadsheet locale - decimal_separator = get_decimal_separator(spreadsheet) - logger.info(f"Using decimal separator: '{decimal_separator}'") + # Deadline for penalty calculation, in the course timezone + timezone_str = course_info.get("timezone") + deadline = get_deadline_from_sheet(sheet, lab_col, deadline_row=1, timezone_str=timezone_str) - # Find GitHub column and student row - header_row = sheet.row_values(1) - try: - github_col_idx = header_row.index("GitHub") + 1 - except ValueError: - logger.error(f"'GitHub' column not found in spreadsheet headers") - raise HTTPException(status_code=400, detail="Столбец 'GitHub' не найден") + target["sheet"] = sheet + target["row"] = row_idx + target["col"] = lab_col - github_values = sheet.col_values(github_col_idx)[2:] - row_idx = find_student_row(github_values, username) + return SheetContext( + current_cell_value=current_value, + student_order=student_order, + deadline=deadline, + decimal_separator=decimal_separator, + ) - if row_idx is None: - logger.warning(f"GitHub username '{username}' not found in spreadsheet for group {group_id}") - raise HTTPException(status_code=404, detail="GitHub логин не найден в таблице. Зарегистрируйтесь.") + outcome = evaluate_student( + grader, org, username, lab_config_dict, course_info, load_sheet_context, + ) - # Find lab column - lab_short_name = lab_config_dict.get("short-name") - if lab_short_name: - lab_col = find_lab_column_by_name(sheet, lab_short_name) - if lab_col: - logger.info(f"Found lab column '{lab_short_name}' at column {lab_col}") + if outcome.status == "error": + # Use 404 for "no commits" and 403 for forbidden edits, as before + if outcome.error_code == "NO_COMMITS": + status_code = 404 + elif outcome.error_code == "FORBIDDEN_MODIFICATION": + status_code = 403 else: - logger.error(f"Lab column '{lab_short_name}' not found in spreadsheet") - raise HTTPException(status_code=400, detail=f"Столбец '{lab_short_name}' не найден в таблице") - else: - logger.warning(f"Lab config for '{lab_id}' is missing 'short-name', using offset calculation") - lab_offset = course_info.get("google", {}).get("lab-column-offset", 1) - # Номер берём из ключа конфига: строка клиента может быть short-name. - lab_number = parse_lab_id(lab_key or lab_id) - lab_col = calculate_lab_column(lab_number, lab_offset) - logger.info(f"Calculated lab column using offset: {lab_offset} + {lab_number} = {lab_col}") - - # Get current cell value for protection check - current_value = sheet.cell(row_idx, lab_col).value or "" - logger.info(f"Current cell value at row {row_idx}, column {lab_col}: '{current_value}'") - - # Determine final grade - final_result = ci_evaluation.grade_result.result # "v" or "x" - final_message = ci_evaluation.grade_result.message - score_value = ci_evaluation.score # Extracted score from logs (if any) - - # Additional checks only if CI passed - if ci_evaluation.ci_passed: - # Check TASKID if configured - task_id_column_config = course_info.get("google", {}).get("task-id-column") - taskid_max = lab_config_dict.get("taskid-max") - ignore_taskid = lab_config_dict.get("ignore-task-id", False) - - if task_id_column_config is not None and taskid_max is not None and not ignore_taskid: - task_id_column = task_id_column_config + 1 - student_order = get_student_order(sheet, row_idx, task_id_column) + status_code = 400 + raise HTTPException(status_code=status_code, detail=outcome.message) - if student_order is not None: - taskid_shift = lab_config_dict.get("taskid-shift", 0) - expected_taskid = calculate_expected_taskid(student_order, taskid_shift, taskid_max) - logger.info(f"Expected TASKID: {expected_taskid} (order={student_order}, shift={taskid_shift}, max={taskid_max})") - - taskid_error = grader.check_taskid( - org, repo_name, - ci_evaluation.successful_runs, - expected_taskid, - ) - if taskid_error: - logger.warning(f"TASKID error: {taskid_error.message}") - raise HTTPException(status_code=400, detail=taskid_error.message) - - # Calculate penalty if deadline configured - # Get timezone from course config to apply to deadline from sheet - timezone_str = course_info.get("timezone") - deadline = get_deadline_from_sheet(sheet, lab_col, deadline_row=1, timezone_str=timezone_str) - penalty = 0 - if deadline and ci_evaluation.latest_success_time: - from grading.penalty import calculate_penalty, format_grade_with_penalty, PenaltyStrategy - penalty_max = lab_config_dict.get("penalty-max", 0) - strategy_name = lab_config_dict.get("penalty-strategy", "weekly") - try: - strategy = PenaltyStrategy(strategy_name) - except ValueError: - strategy = PenaltyStrategy.WEEKLY - - penalty = calculate_penalty( - completed_at=ci_evaluation.latest_success_time, - deadline=deadline, - penalty_max=penalty_max, - strategy=strategy, - ) - - if penalty > 0: - logger.info(f"Calculated penalty: {penalty}") - - # Format final result with score and penalty - if score_value is not None: - # Format grade with score (and penalty if present) - final_result = format_grade_with_score("v", score_value, penalty, decimal_separator) - logger.info(f"Formatted grade with score: {final_result}") - - # Build message - formatted_score = format_score(score_value, decimal_separator) - if penalty > 0: - final_message = f"Результат CI: ✅ Все проверки пройдены (Баллы: {formatted_score}, штраф: -{penalty})" - else: - final_message = f"Результат CI: ✅ Все проверки пройдены (Баллы: {formatted_score})" - elif penalty > 0: - # No score, but penalty exists - from grading.penalty import format_grade_with_penalty - final_result = format_grade_with_penalty("v", penalty) - final_message = f"Результат CI: ✅ Все проверки пройдены (штраф: -{penalty})" - logger.info(f"Applied penalty {penalty} for late submission: {final_result}") - - # Check cell protection - if not can_overwrite_cell(current_value): - logger.warning(f"Update rejected: cell already contains '{current_value}'") + if outcome.status == "pending": + return { + "status": "pending", + "message": outcome.message, + "passed": outcome.passed, + "checks": outcome.checks + } + + if outcome.status == "rejected": response = { "status": "rejected", - "result": current_value, - "message": "⚠️ Работа уже была проверена ранее. Обратитесь к преподавателю для пересдачи.", - "passed": ci_evaluation.grade_result.passed, - "checks": ci_evaluation.grade_result.checks, - "current_grade": current_value + "result": outcome.current_grade, + "message": outcome.message, + "passed": outcome.passed, + "checks": outcome.checks, + "current_grade": outcome.current_grade } - if score_value is not None: - response["score"] = format_score(score_value, decimal_separator) + if outcome.score is not None: + response["score"] = outcome.score return response # Update Google Sheets with new grade - logger.info(f"Updating cell at row {row_idx}, column {lab_col} with result '{final_result}'") - sheet.update_cell(row_idx, lab_col, final_result) + logger.info(f"Updating cell at row {target['row']}, column {target['col']} with result '{outcome.cell_value}'") + target["sheet"].update_cell(target["row"], target["col"], outcome.cell_value) logger.info(f"Successfully updated grade for '{username}' in lab {lab_id}") response = { "status": "updated", - "result": final_result, - "message": final_message, - "passed": ci_evaluation.grade_result.passed, - "checks": ci_evaluation.grade_result.checks + "result": outcome.cell_value, + "message": outcome.message, + "passed": outcome.passed, + "checks": outcome.checks } - if score_value is not None: - response["score"] = format_score(score_value, decimal_separator) + if outcome.score is not None: + response["score"] = outcome.score return response except HTTPException: raise @@ -897,6 +844,7 @@ def grade_lab(request: Request, course_id: str, group_id: str, lab_id: str, grad raise HTTPException(status_code=500, detail=f"Внутренняя ошибка сервера: {str(e)}") + # --------------------------------------------------------------------------- # /join: automatic student repo creation (replaces GitHub Classroom) # See docs/REPO_GENERATION_PLAN.md for the full design. @@ -1192,6 +1140,10 @@ def admin_list_course_labs(request: Request, course_id: str, admin: str = Depend "template_repo": template_repo, "repo_provisioning": repo_provisioning, "can_propagate": bool(template_repo) and repo_provisioning == "fork", + # Bulk grading: candidates for the file holding the student's full + # name, and the one preselected via `student-name-file`. + "files": lab_config.get("files", []), + "name_file": lab_config.get("student-name-file"), }) # Порядок как у преподавателя в таблице: ЛР0, ЛР0.1, ЛР1... Сортировка по @@ -1314,6 +1266,136 @@ def get_propagate_job_status(request: Request, job_id: str, admin: str = Depends return job.to_dict() +# --------------------------------------------------------------------------- +# Admin: bulk grading of a whole group's submissions for one lab. +# Same job machinery as propagate above - a group takes minutes, so the run is +# backgrounded and polled. See docs/PROJECT_DESCRIPTION.md. +# --------------------------------------------------------------------------- + + +class BulkGradeRequest(BaseModel): + # Файл, из первой строки которого берётся ФИО студента. Задан - обходятся + # все репозитории лабы в организации и логины проставляются в таблицу; + # пуст - проверяются только студенты с уже указанным логином. + name_file: str | None = None + # Прогнать все проверки и собрать отчёт, ничего не записывая в таблицу. + dry_run: bool = False + + +def _open_group_worksheet(spreadsheet_id: str, group_id: str): + """ + Open a group's worksheet, returning (spreadsheet, worksheet). + + Opened by the endpoint rather than inside the job, so that a wrong group + fails the request with 404 instead of a job that dies immediately. + """ + scope = ["https://spreadsheets.google.com/feeds", "https://www.googleapis.com/auth/drive"] + creds = ServiceAccountCredentials.from_json_keyfile_name(CREDENTIALS_FILE, scope) + sheets_client = gspread.authorize(creds) + + try: + spreadsheet = sheets_client.open_by_key(spreadsheet_id) + worksheet = spreadsheet.worksheet(group_id) + except Exception as e: + logger.error(f"Failed to open worksheet '{group_id}': {str(e)}") + raise HTTPException(status_code=404, detail="Группа не найдена в Google Таблице") + + return spreadsheet, worksheet + + +@app.post("/admin/courses/{course_id}/groups/{group_id}/labs/{lab_id}/bulk-grade") +@limiter.limit("10/minute") +def start_bulk_grade( + request: Request, + course_id: str, + group_id: str, + lab_id: str, + background_tasks: BackgroundTasks, + body: BulkGradeRequest = BulkGradeRequest(), + admin: str = Depends(require_admin), +): + """ + Start grading a whole group for one lab in the background. + + Every student goes through the same checks as a self-submitted work + (grading.bulk.evaluate_student). Unlike propagate's read-only dry run, + dry_run here still runs the full CI checks, so it is a background job too; + it just writes nothing to the spreadsheet. + + Returns 202 with a job_id to poll via GET /admin/bulk-grade-jobs/{job_id}; + only one run per (course_id, group_id, lab_id) at a time - a second POST + while one is in flight gets HTTP 409. + """ + name_file = (body.name_file or "").strip() or None + mode = "by_file" if name_file else "by_sheet" + + course_info = get_course_by_id(course_id) + org = course_info.get("github", {}).get("organization") + spreadsheet_id = course_info.get("google", {}).get("spreadsheet") + + resolved = find_lab_config(course_info.get("labs", {}), lab_id) + lab_key, lab_config_dict = resolved if resolved else (None, {}) + repo_prefix = lab_config_dict.get("github-prefix") + + if not all([org, spreadsheet_id, repo_prefix]): + logger.error( + f"Missing course configuration for {course_id}: org={org}, " + f"spreadsheet={spreadsheet_id}, repo_prefix={repo_prefix}" + ) + raise HTTPException(status_code=400, detail="Missing course configuration") + + spreadsheet, worksheet = _open_group_worksheet(spreadsheet_id, group_id) + + job = try_start_bulk_job(course_id, group_id, lab_id, mode, body.dry_run, name_file) + if job is None: + raise HTTPException( + status_code=409, + detail="Проверка этой лабораторной для этой группы уже выполняется", + ) + + logger.info( + f"Starting bulk grading job {job.job_id} for {course_id}/{group_id}/{lab_id} " + f"(mode={mode}, dry_run={body.dry_run}, admin={admin})" + ) + github_client = GitHubClient(GITHUB_TOKEN) + background_tasks.add_task( + run_bulk_grading, + job, + LabGrader(github_client), + github_client, + worksheet, + spreadsheet, + course_info, + lab_config_dict, + parse_lab_id(lab_key or lab_id), + ) + return JSONResponse(status_code=202, content={"job_id": job.job_id}) + + +@app.get("/admin/bulk-grade-jobs/{job_id}") +@limiter.limit("120/minute") +def get_bulk_grade_job_status(request: Request, job_id: str, admin: str = Depends(require_admin)): + job = get_bulk_job(job_id) + if job is None: + raise HTTPException(status_code=404, detail="Работа не найдена") + return job.to_dict() + + +@app.post("/admin/bulk-grade-jobs/{job_id}/cancel") +@limiter.limit("30/minute") +def cancel_bulk_grade_job(request: Request, job_id: str, admin: str = Depends(require_admin)): + """ + Ask a running bulk grading job to stop. + + It finishes the student it is on, flushes the grades buffered so far and + ends with status "cancelled". + """ + job = request_bulk_job_cancel(job_id) + if job is None: + raise HTTPException(status_code=404, detail="Работа не найдена") + return job.to_dict() + + @app.post("/courses/upload") @limiter.limit("10/minute") async def upload_course(request: Request, file: UploadFile = File(...), admin: str = Depends(require_admin)): diff --git a/tests/test_admin_endpoints.py b/tests/test_admin_endpoints.py index adee3f9..51239c9 100644 --- a/tests/test_admin_endpoints.py +++ b/tests/test_admin_endpoints.py @@ -12,7 +12,7 @@ import sys import os import yaml -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest import responses @@ -24,7 +24,9 @@ import main as main_module from main import app +from grading.grader import CIEvaluation, GradeResult, GradeStatus from grading.propagate import _jobs, _running_lab_keys +from grading.bulk import _jobs as _bulk_jobs, _running_keys as _bulk_running_keys @pytest.fixture(autouse=True) @@ -41,9 +43,13 @@ def disable_real_rate_limiting(monkeypatch): def clean_job_store(): _jobs.clear() _running_lab_keys.clear() + _bulk_jobs.clear() + _bulk_running_keys.clear() yield _jobs.clear() _running_lab_keys.clear() + _bulk_jobs.clear() + _bulk_running_keys.clear() @pytest.fixture @@ -89,6 +95,13 @@ def admin_course_env(tmp_path, monkeypatch): {"json": {"dry_run": True}}, ), ("GET", "/admin/propagate-jobs/does-not-exist", {}), + ( + "POST", + "/admin/courses/test-course/groups/P3300/labs/1/bulk-grade", + {"json": {"dry_run": True}}, + ), + ("GET", "/admin/bulk-grade-jobs/does-not-exist", {}), + ("POST", "/admin/bulk-grade-jobs/does-not-exist/cancel", {}), ] @@ -156,6 +169,16 @@ def test_admin_labs_returns_200(self, client, admin_course_env): assert response.status_code == 200 assert response.json() == [] + def test_bulk_grade_job_status_returns_404_for_unknown_job_not_401(self, client): + client.cookies.set("admin_session", valid_cookie()) + response = client.get("/admin/bulk-grade-jobs/does-not-exist") + assert response.status_code == 404 + + def test_bulk_grade_job_cancel_returns_404_for_unknown_job_not_401(self, client): + client.cookies.set("admin_session", valid_cookie()) + response = client.post("/admin/bulk-grade-jobs/does-not-exist/cancel") + assert response.status_code == 404 + def test_propagate_job_status_returns_404_for_unknown_job_not_401(self, client): client.cookies.set("admin_session", valid_cookie()) response = client.get("/admin/propagate-jobs/does-not-exist") @@ -340,3 +363,136 @@ def test_unknown_job_status_is_404(self, mock_request): with pytest.raises(HTTPException) as exc_info: main_module.get_propagate_job_status(mock_request, "does-not-exist", admin="admin") assert exc_info.value.status_code == 404 + + +class TestBulkGradeEndpoint: + """The bulk grading endpoint itself: config checks, 202 + job, 409.""" + + @pytest.fixture + def bulk_course_config(self, sample_course_config): + sample_course_config["google"]["spreadsheet"] = "sheet-id" + return sample_course_config + + @pytest.fixture + def mock_worksheet(self): + """Patch the Sheets connection the endpoint opens before starting a job.""" + worksheet = MagicMock() + worksheet.get_all_values.return_value = [ + ["№", "ФИО", "GitHub", ""], + ["", "", "", "ЛР1"], + ["1", "Иванов Иван", "student1", ""], + ] + spreadsheet = MagicMock() + spreadsheet.fetch_sheet_metadata.return_value = {"properties": {"locale": "en_US"}} + with patch.object(main_module, "_open_group_worksheet", return_value=(spreadsheet, worksheet)): + yield worksheet + + def test_missing_spreadsheet_config_is_400(self, mock_request, sample_course_config): + sample_course_config["google"].pop("spreadsheet", None) + with patch("main.get_course_by_id", return_value=sample_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР1", BackgroundTasks(), + body=main_module.BulkGradeRequest(), admin="admin", + ) + assert exc_info.value.status_code == 400 + + def test_unknown_lab_is_400(self, mock_request, bulk_course_config): + with patch("main.get_course_by_id", return_value=bulk_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР42", BackgroundTasks(), + body=main_module.BulkGradeRequest(), admin="admin", + ) + assert exc_info.value.status_code == 400 + + def test_returns_202_and_runs_the_job(self, mock_request, bulk_course_config, mock_worksheet): + import json + + with patch("main.get_course_by_id", return_value=bulk_course_config): + bg = BackgroundTasks() + response = main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР1", bg, + body=main_module.BulkGradeRequest(dry_run=True), admin="admin", + ) + assert response.status_code == 202 + job_id = json.loads(response.body)["job_id"] + + job = main_module.get_bulk_job(job_id) + assert job.mode == "by_sheet" + assert job.dry_run is True + + with patch.object(main_module.LabGrader, "check_repository", return_value=None), \ + patch.object(main_module.LabGrader, "check_forbidden_files", return_value=None), \ + patch.object(main_module.LabGrader, "_evaluate_ci_internal") as evaluate: + evaluate.return_value = CIEvaluation( + grade_result=GradeResult( + status=GradeStatus.UPDATED, result="v", + message="Результат CI: ✅ Все проверки пройдены", passed="1/1", + ), + ci_passed=True, + ) + run_background_tasks(bg) + + assert job.status == "done" + assert [r.github for r in job.results] == ["student1"] + # dry_run: the report is built, the spreadsheet is left alone + mock_worksheet.batch_update.assert_not_called() + + def test_name_file_selects_by_file_mode(self, mock_request, bulk_course_config, mock_worksheet): + import json + + with patch("main.get_course_by_id", return_value=bulk_course_config): + response = main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР1", BackgroundTasks(), + body=main_module.BulkGradeRequest(name_file="info.md"), admin="admin", + ) + + job = main_module.get_bulk_job(json.loads(response.body)["job_id"]) + assert job.mode == "by_file" + assert job.name_file == "info.md" + + def test_blank_name_file_falls_back_to_by_sheet_mode(self, mock_request, bulk_course_config, mock_worksheet): + import json + + with patch("main.get_course_by_id", return_value=bulk_course_config): + response = main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР1", BackgroundTasks(), + body=main_module.BulkGradeRequest(name_file=" "), admin="admin", + ) + + job = main_module.get_bulk_job(json.loads(response.body)["job_id"]) + assert job.mode == "by_sheet" + assert job.name_file is None + + def test_second_run_for_same_group_and_lab_is_409(self, mock_request, bulk_course_config, mock_worksheet): + from grading.bulk import try_start_bulk_job + try_start_bulk_job("test-course", "P3300", "ЛР1", "by_sheet", False, None) + + with patch("main.get_course_by_id", return_value=bulk_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.start_bulk_grade( + mock_request, "test-course", "P3300", "ЛР1", BackgroundTasks(), + body=main_module.BulkGradeRequest(), admin="admin", + ) + assert exc_info.value.status_code == 409 + + def test_other_group_may_start_while_one_runs(self, mock_request, bulk_course_config, mock_worksheet): + from grading.bulk import try_start_bulk_job + try_start_bulk_job("test-course", "P3300", "ЛР1", "by_sheet", False, None) + + with patch("main.get_course_by_id", return_value=bulk_course_config): + response = main_module.start_bulk_grade( + mock_request, "test-course", "P3301", "ЛР1", BackgroundTasks(), + body=main_module.BulkGradeRequest(), admin="admin", + ) + assert response.status_code == 202 + + def test_cancel_marks_a_running_job(self, mock_request): + from grading.bulk import try_start_bulk_job + job = try_start_bulk_job("test-course", "P3300", "ЛР1", "by_sheet", False, None) + + result = main_module.cancel_bulk_grade_job(mock_request, job.job_id, admin="admin") + + assert result["status"] == "running" + assert job.cancel_requested is True diff --git a/tests/test_bulk_grading.py b/tests/test_bulk_grading.py new file mode 100644 index 0000000..dc5edf6 --- /dev/null +++ b/tests/test_bulk_grading.py @@ -0,0 +1,812 @@ +""" +Tests for bulk grading (grading/bulk.py). + +Covers repository discovery, student matching by full name, the shared +grading decision, the background job store and the orchestrator. +""" +import pytest +from datetime import datetime, timedelta +from unittest.mock import MagicMock, patch +import sys +import os + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from grading.bulk import ( + BulkGradingError, + BulkJob, + BulkResult, + NameMatchError, + SheetContext, + StudentOutcome, + evaluate_student, + extract_full_name, + filter_lab_repos, + find_row_by_full_name, + normalize_full_name, + repo_name_for, + resolve_github_cell, + run_bulk_grading, + taskid_column, + try_start_bulk_job, + get_bulk_job, + request_bulk_job_cancel, + _jobs, + _running_keys, +) +from grading.grader import GradeResult, GradeStatus, CIEvaluation +from grading.sheets_client import ( + cell_from_grid, + column_values_from_grid, + find_lab_column_in_grid, + get_deadline_from_grid, + get_student_order_from_grid, + parse_deadline, +) + + +class TestFilterLabRepos: + """Selecting a lab's repositories out of an organization.""" + + def test_extracts_username_after_prefix(self): + repos = ["os-task1-alice", "os-task1-bob", "other-repo"] + assert filter_lab_repos(repos, "os-task1") == { + "alice": "os-task1-alice", + "bob": "os-task1-bob", + } + + def test_prefix_collision_with_longer_lab_number(self): + """os-task1 must not swallow os-task10 repositories.""" + repos = ["os-task1-alice", "os-task10-bob", "os-task11-carol"] + + assert filter_lab_repos(repos, "os-task1") == {"alice": "os-task1-alice"} + assert filter_lab_repos(repos, "os-task10") == {"bob": "os-task10-bob"} + + def test_username_with_dashes(self): + assert filter_lab_repos(["os-task1-jane-doe"], "os-task1") == { + "jane-doe": "os-task1-jane-doe" + } + + def test_template_repo_without_username_is_skipped(self): + assert filter_lab_repos(["os-task1", "os-task1-"], "os-task1") == {} + + def test_empty_prefix_matches_nothing(self): + assert filter_lab_repos(["os-task1-alice"], "") == {} + + def test_no_repos(self): + assert filter_lab_repos([], "os-task1") == {} + + +class TestExtractFullName: + """Reading the student's name out of the name file.""" + + def test_first_line(self): + assert extract_full_name("Иванов Иван Иванович\nЛР1\n") == "Иванов Иван Иванович" + + def test_skips_leading_blank_lines(self): + assert extract_full_name("\n\n Петров Пётр \n") == "Петров Пётр" + + def test_crlf_line_endings(self): + assert extract_full_name("Сидоров Сидор\r\nтекст") == "Сидоров Сидор" + + def test_empty_file(self): + assert extract_full_name("") is None + + def test_whitespace_only_file(self): + assert extract_full_name(" \n\t\n") is None + + def test_missing_file(self): + assert extract_full_name(None) is None + + +class TestNormalizeFullName: + """Name normalization used for matching.""" + + def test_collapses_whitespace(self): + assert normalize_full_name("Иванов Иван\tИванович") == "иванов иван иванович" + + def test_case_insensitive(self): + assert normalize_full_name("ИВАНОВ Иван") == normalize_full_name("иванов иван") + + def test_yo_equals_ye(self): + assert normalize_full_name("Алёшин Пётр") == normalize_full_name("Алешин Петр") + + def test_non_breaking_space(self): + assert normalize_full_name("Иванов Иван") == "иванов иван" + + def test_empty(self): + assert normalize_full_name("") == "" + assert normalize_full_name(None) == "" + + +class TestFindRowByFullName: + """Resolving a name to a spreadsheet row.""" + + NAMES = ["Иванов Иван Иванович", "Петров Пётр Петрович", "Сидоров Сидор"] + + def test_exact_match(self): + assert find_row_by_full_name(self.NAMES, "Петров Пётр Петрович") == 4 + + def test_first_row_is_three(self): + """Two header rows precede student data.""" + assert find_row_by_full_name(self.NAMES, "Иванов Иван Иванович") == 3 + + def test_match_after_normalization(self): + assert find_row_by_full_name(self.NAMES, "петров петр петрович") == 4 + + def test_unmatched_raises(self): + with pytest.raises(NameMatchError) as exc: + find_row_by_full_name(self.NAMES, "Неизвестный Студент") + assert exc.value.code == "unmatched" + + def test_empty_name_raises_unmatched(self): + with pytest.raises(NameMatchError) as exc: + find_row_by_full_name(self.NAMES, " ") + assert exc.value.code == "unmatched" + + def test_ambiguous_raises(self): + names = ["Иванов Иван", "Петров Пётр", "Иванов Иван"] + with pytest.raises(NameMatchError) as exc: + find_row_by_full_name(names, "Иванов Иван") + assert exc.value.code == "ambiguous" + assert "3" in exc.value.message and "5" in exc.value.message + + def test_surname_alone_does_not_match(self): + """Fuzzy matching is deliberately not attempted.""" + with pytest.raises(NameMatchError): + find_row_by_full_name(self.NAMES, "Иванов") + + +class TestResolveGithubCell: + """Policy for writing the GitHub username into the sheet.""" + + def test_empty_cell_is_written(self): + assert resolve_github_cell("", "alice") == (True, None) + + def test_whitespace_cell_is_written(self): + assert resolve_github_cell(" ", "alice") == (True, None) + + def test_same_username_is_not_rewritten(self): + assert resolve_github_cell("alice", "alice") == (False, None) + + def test_same_username_different_case(self): + should_write, conflict = resolve_github_cell("Alice", "alice") + assert should_write is False + assert conflict is None + + def test_different_username_is_a_conflict(self): + should_write, conflict = resolve_github_cell("bob", "alice") + assert should_write is False + assert "bob" in conflict and "alice" in conflict + + +class TestGridHelpers: + """In-memory equivalents of the per-cell Sheets helpers.""" + + GRID = [ + ["", "", "", "19.11.2025"], + ["№", "ФИО", "GitHub", "ЛР1"], + ["1", "Иванов Иван", "alice", "v"], + ["2", "Петров Пётр", "bob"], + ] + + def test_cell_from_grid(self): + assert cell_from_grid(self.GRID, 3, 3) == "alice" + + def test_cell_beyond_row_length(self): + """Trailing empty cells are omitted by gspread.""" + assert cell_from_grid(self.GRID, 4, 4) == "" + + def test_cell_beyond_grid(self): + assert cell_from_grid(self.GRID, 99, 1) == "" + assert cell_from_grid(self.GRID, 0, 1) == "" + + def test_column_values_pads_missing_cells(self): + assert column_values_from_grid(self.GRID, 4, start_row=3) == ["v", ""] + + def test_column_values_from_top(self): + assert column_values_from_grid(self.GRID, 3) == ["", "GitHub", "alice", "bob"] + + def test_find_lab_column(self): + assert find_lab_column_in_grid(self.GRID, "ЛР1") == 4 + + def test_find_lab_column_missing(self): + assert find_lab_column_in_grid(self.GRID, "ЛР9") is None + + def test_find_lab_column_empty_name(self): + assert find_lab_column_in_grid(self.GRID, "") is None + + def test_deadline_from_grid(self): + deadline = get_deadline_from_grid(self.GRID, 4, deadline_row=1, timezone_str="UTC+3") + assert deadline.year == 2025 and deadline.month == 11 and deadline.day == 19 + # A date without a time means the end of that day + assert (deadline.hour, deadline.minute) == (23, 59) + assert deadline.tzinfo is not None + + def test_deadline_missing(self): + assert get_deadline_from_grid(self.GRID, 2, deadline_row=1) is None + + def test_student_order_from_grid(self): + assert get_student_order_from_grid(self.GRID, 3, 1) == 1 + assert get_student_order_from_grid(self.GRID, 4, 1) == 2 + + def test_student_order_unparseable(self): + assert get_student_order_from_grid(self.GRID, 2, 1) is None + + def test_parse_deadline_matches_sheet_version(self): + assert parse_deadline("15.03.2025 23:59").hour == 23 + assert parse_deadline("") is None + assert parse_deadline("не дата") is None + + +class TestTaskidColumn: + """When the TASKID check applies.""" + + COURSE = {"google": {"task-id-column": 0}} + + def test_configured(self): + assert taskid_column(self.COURSE, {"taskid-max": 20}) == 1 + + def test_no_column_in_course(self): + assert taskid_column({"google": {}}, {"taskid-max": 20}) is None + + def test_no_taskid_max_in_lab(self): + assert taskid_column(self.COURSE, {}) is None + + def test_ignore_task_id(self): + assert taskid_column(self.COURSE, {"taskid-max": 20, "ignore-task-id": True}) is None + + +class TestRepoNameFor: + def test_builds_conventional_name(self): + assert repo_name_for({"github-prefix": "os-task2"}, "alice") == "os-task2-alice" + + +def _grade_result(status, result=None, message="", error_code=None, passed=None): + return GradeResult( + status=status, + result=result, + message=message, + passed=passed, + error_code=error_code, + ) + + +def _grader_mock(ci_evaluation, repo_error=None, forbidden_error=None, taskid_error=None): + grader = MagicMock() + grader.check_repository.return_value = repo_error + grader.check_forbidden_files.return_value = forbidden_error + grader._evaluate_ci_internal.return_value = ci_evaluation + grader.check_taskid.return_value = taskid_error + return grader + + +class TestEvaluateStudent: + """The grading decision shared by the endpoint and the bulk run.""" + + LAB = {"github-prefix": "os-task1", "short-name": "ЛР1"} + COURSE = {"github": {"organization": "test-org"}, "google": {}} + + def test_passing_ci_gives_v(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "Результат CI: ✅ Все проверки пройдены"), + ci_passed=True, + ) + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, + lambda: SheetContext(), + ) + assert outcome.status == "updated" + assert outcome.cell_value == "v" + + def test_failing_ci_gives_x(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "x", "Результат CI: ❌ Обнаружены ошибки"), + ci_passed=False, + ) + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, + lambda: SheetContext(), + ) + assert outcome.status == "updated" + assert outcome.cell_value == "x" + + def test_repository_error_short_circuits(self): + repo_error = _grade_result( + GradeStatus.ERROR, message="Нет коммитов в репозитории", error_code="NO_COMMITS" + ) + grader = _grader_mock(None, repo_error=repo_error) + + outcome = evaluate_student( + grader, "test-org", "alice", self.LAB, self.COURSE, lambda: SheetContext(), + ) + assert outcome.status == "error" + assert outcome.error_code == "NO_COMMITS" + grader._evaluate_ci_internal.assert_not_called() + + def test_forbidden_modification_short_circuits(self): + forbidden = _grade_result( + GradeStatus.ERROR, message="🚨 Нельзя изменять test_main.py", + error_code="FORBIDDEN_MODIFICATION", + ) + grader = _grader_mock(None, forbidden_error=forbidden) + + outcome = evaluate_student( + grader, "test-org", "alice", self.LAB, self.COURSE, lambda: SheetContext(), + ) + assert outcome.status == "error" + assert outcome.error_code == "FORBIDDEN_MODIFICATION" + + def test_sheet_context_not_requested_on_error(self): + """Callers may defer opening a Sheets connection until a grade exists.""" + repo_error = _grade_result(GradeStatus.ERROR, message="нет файла", error_code="MISSING_FILES") + provider = MagicMock() + + evaluate_student( + _grader_mock(None, repo_error=repo_error), + "test-org", "alice", self.LAB, self.COURSE, provider, + ) + provider.assert_not_called() + + def test_sheet_context_not_requested_when_pending(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.PENDING, message="CI-проверки ещё выполняются ⏳"), + ci_passed=False, + ) + provider = MagicMock() + + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, provider, + ) + assert outcome.status == "pending" + provider.assert_not_called() + + def test_protected_cell_is_rejected(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + ) + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, + lambda: SheetContext(current_cell_value="v"), + ) + assert outcome.status == "rejected" + assert outcome.current_grade == "v" + + def test_x_in_cell_can_be_overwritten(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + ) + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, + lambda: SheetContext(current_cell_value="x"), + ) + assert outcome.status == "updated" + + def test_penalty_applied_for_late_submission(self): + deadline = datetime(2025, 3, 15, 23, 59) + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + latest_success_time=deadline + timedelta(days=8), + ) + lab = dict(self.LAB, **{"penalty-max": 9}) + + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", lab, self.COURSE, + lambda: SheetContext(deadline=deadline), + ) + assert outcome.cell_value == "v-2" + assert "штраф" in outcome.message + + def test_score_formatted_with_sheet_separator(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + score="10.5", + ) + outcome = evaluate_student( + _grader_mock(ci), "test-org", "alice", self.LAB, self.COURSE, + lambda: SheetContext(decimal_separator=","), + ) + assert outcome.cell_value == "v@10,5" + assert outcome.score == "10,5" + + def test_taskid_checked_when_configured(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + ) + taskid_error = _grade_result( + GradeStatus.ERROR, message="Неверный вариант", error_code="WRONG_TASKID" + ) + grader = _grader_mock(ci, taskid_error=taskid_error) + course = {"github": {"organization": "test-org"}, "google": {"task-id-column": 0}} + lab = dict(self.LAB, **{"taskid-max": 20, "taskid-shift": 4}) + + outcome = evaluate_student( + grader, "test-org", "alice", lab, course, + lambda: SheetContext(student_order=3), + ) + assert outcome.status == "error" + assert outcome.error_code == "WRONG_TASKID" + grader.check_taskid.assert_called_once() + + def test_taskid_not_checked_when_ignored(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + ) + grader = _grader_mock(ci) + course = {"github": {"organization": "test-org"}, "google": {"task-id-column": 0}} + lab = dict(self.LAB, **{"taskid-max": 20, "ignore-task-id": True}) + + evaluate_student( + grader, "test-org", "alice", lab, course, + lambda: SheetContext(student_order=3), + ) + grader.check_taskid.assert_not_called() + + def test_no_taskid_check_when_ci_failed(self): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "x", "ошибки"), + ci_passed=False, + ) + grader = _grader_mock(ci) + course = {"github": {"organization": "test-org"}, "google": {"task-id-column": 0}} + lab = dict(self.LAB, **{"taskid-max": 20}) + + outcome = evaluate_student( + grader, "test-org", "alice", lab, course, + lambda: SheetContext(student_order=3), + ) + assert outcome.cell_value == "x" + grader.check_taskid.assert_not_called() + + +@pytest.fixture(autouse=True) +def clean_job_store(): + """Same isolation as tests/test_admin_endpoints.py does for propagate jobs.""" + _jobs.clear() + _running_keys.clear() + yield + _jobs.clear() + _running_keys.clear() + + +class TestBulkJobStore: + """Lifecycle of background jobs, mirroring propagate's job store.""" + + def test_start_and_get(self): + job = try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) + + assert get_bulk_job(job.job_id) is job + assert job.status == "running" + assert job.started_at + + def test_get_unknown_job(self): + assert get_bulk_job("nope") is None + + def test_second_job_for_same_group_and_lab_is_refused(self): + first = try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) + + assert try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) is None + assert first.status == "running" + + def test_other_group_or_lab_may_run_concurrently(self): + try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) + + assert try_start_bulk_job("c", "g2", "ЛР1", "by_sheet", False, None) is not None + assert try_start_bulk_job("c", "g", "ЛР2", "by_sheet", False, None) is not None + + def test_finished_job_releases_the_slot(self, bulk_setup): + job = try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) + _run(job, bulk_setup) + + assert job.status == "done" + assert try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) is not None + + def test_request_cancel(self): + job = try_start_bulk_job("c", "g", "ЛР1", "by_sheet", False, None) + + assert request_bulk_job_cancel(job.job_id) is job + assert job.cancel_requested is True + + def test_request_cancel_unknown_job(self): + assert request_bulk_job_cancel("nope") is None + + def test_to_dict_counts_statuses(self): + job = BulkJob(job_id="j", course_id="c", group_id="g", lab_id="ЛР1", mode="by_sheet") + job.results = [ + BulkResult(status="updated"), BulkResult(status="updated"), + BulkResult(status="error"), + ] + + payload = job.to_dict() + assert payload["counts"] == {"updated": 2, "error": 1} + assert len(payload["results"]) == 3 + + +@pytest.fixture +def bulk_setup(): + """Worksheet, spreadsheet and configs for orchestrator tests.""" + # Row 1 carries the left-hand headers and the lab deadline, row 2 the lab + # short name; student data starts at row 3 - as grade_lab reads them. + grid = [ + ["№", "ФИО", "GitHub", ""], + ["", "", "", "ЛР1"], + ["1", "Иванов Иван", "alice", ""], + ["2", "Петров Пётр", "bob", ""], + ["3", "Сидоров Сидор", "", ""], + ] + + worksheet = MagicMock() + worksheet.get_all_values.return_value = grid + + spreadsheet = MagicMock() + spreadsheet.fetch_sheet_metadata.return_value = {"properties": {"locale": "en_US"}} + + return { + "grid": grid, + "worksheet": worksheet, + "spreadsheet": spreadsheet, + "course_info": { + "github": {"organization": "test-org"}, + "google": {"spreadsheet": "sid", "student-name-column": 1}, + }, + "lab_config": {"github-prefix": "os-task1", "short-name": "ЛР1"}, + } + + +def _passing_grader(): + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "Результат CI: ✅ Все проверки пройдены"), + ci_passed=True, + ) + return _grader_mock(ci) + + +def _job(mode="by_sheet", dry_run=False, name_file=None): + return BulkJob( + job_id="j", course_id="c", group_id="g", lab_id="ЛР1", + mode=mode, dry_run=dry_run, name_file=name_file, + ) + + +def _run(job, setup, grader=None, github_client=None): + run_bulk_grading( + job, + grader or _passing_grader(), + github_client or MagicMock(), + setup["worksheet"], + setup["spreadsheet"], + setup["course_info"], + setup["lab_config"], + 1, + ) + + +def _written_cells(worksheet): + """Collect (range, value) pairs from every batch_update call.""" + written = [] + for call in worksheet.batch_update.call_args_list: + for entry in call.args[0]: + written.append((entry["range"], entry["values"][0][0])) + return written + + +class TestRunBulkGradingBySheet: + """Mode with an empty name file: grade whoever has a username.""" + + def test_grades_students_with_usernames_only(self, bulk_setup): + job = _job() + _run(job, bulk_setup) + + assert job.status == "done" + assert job.total == 2 # Сидоров has no GitHub username + assert job.processed == 2 + assert [r.github for r in job.results] == ["alice", "bob"] + + def test_writes_grades_to_the_lab_column(self, bulk_setup): + job = _job() + _run(job, bulk_setup) + + assert _written_cells(bulk_setup["worksheet"]) == [("D3", "v"), ("D4", "v")] + + def test_dry_run_writes_nothing(self, bulk_setup): + job = _job(dry_run=True) + _run(job, bulk_setup) + + assert job.status == "done" + assert [r.status for r in job.results] == ["updated", "updated"] + bulk_setup["worksheet"].batch_update.assert_not_called() + + def test_protected_cell_is_reported_not_overwritten(self, bulk_setup): + bulk_setup["grid"][2][3] = "v@8" + job = _job() + _run(job, bulk_setup) + + assert job.results[0].status == "rejected" + assert job.results[0].grade == "v@8" + assert _written_cells(bulk_setup["worksheet"]) == [("D4", "v")] + + def test_missing_github_column_fails_the_job(self, bulk_setup): + bulk_setup["grid"][0][2] = "Гитхаб" + job = _job() + _run(job, bulk_setup) + + assert job.status == "failed" + assert "GitHub" in job.error + assert job.finished_at + + def test_missing_lab_column_fails_the_job(self, bulk_setup): + bulk_setup["lab_config"]["short-name"] = "ЛР9" + job = _job() + _run(job, bulk_setup) + + assert job.status == "failed" + assert "ЛР9" in job.error + + def test_lab_without_short_name_falls_back_to_column_offset(self, bulk_setup): + del bulk_setup["lab_config"]["short-name"] + bulk_setup["course_info"]["google"]["lab-column-offset"] = 3 + job = _job() + _run(job, bulk_setup) + + # offset 3 + lab number 1 = column 4 = D + assert _written_cells(bulk_setup["worksheet"]) == [("D3", "v"), ("D4", "v")] + + def test_error_on_one_student_does_not_stop_the_run(self, bulk_setup): + grader = _passing_grader() + grader.check_repository.side_effect = [ + _grade_result(GradeStatus.ERROR, message="Нет коммитов", error_code="NO_COMMITS"), + None, + ] + job = _job() + _run(job, bulk_setup, grader=grader) + + assert job.status == "done" + assert [r.status for r in job.results] == ["error", "updated"] + assert _written_cells(bulk_setup["worksheet"]) == [("D4", "v")] + + def test_unexpected_exception_is_reported_per_student(self, bulk_setup): + grader = _passing_grader() + grader.check_repository.side_effect = [RuntimeError("boom"), None] + job = _job() + _run(job, bulk_setup, grader=grader) + + assert job.status == "done" + assert job.results[0].status == "error" + assert "boom" in job.results[0].message + + def test_cancellation_stops_and_flushes(self, bulk_setup): + job = _job() + job.cancel_requested = True + _run(job, bulk_setup) + + assert job.status == "cancelled" + assert job.processed == 0 + bulk_setup["worksheet"].batch_update.assert_not_called() + + def test_deadline_and_penalty_come_from_the_grid(self, bulk_setup): + bulk_setup["grid"][0][3] = "15.03.2025" # deadline sits above the lab header + bulk_setup["lab_config"]["penalty-max"] = 9 + bulk_setup["course_info"]["timezone"] = "UTC+3" + + from datetime import timezone as tz + late = datetime(2025, 3, 30, 12, 0, tzinfo=tz(timedelta(hours=3))) + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.UPDATED, "v", "ok"), + ci_passed=True, + latest_success_time=late, + ) + job = _job() + _run(job, bulk_setup, grader=_grader_mock(ci)) + + assert job.results[0].grade == "v-3" + + +class TestRunBulkGradingByFile: + """Mode with a name file: discover repos and match students by name.""" + + def _github_client(self, repo_names, files): + client = MagicMock() + client.list_org_repos.return_value = [{"name": name} for name in repo_names] + client.get_file_content.side_effect = lambda org, repo, path: files.get(repo) + return client + + def test_registers_username_and_grades(self, bulk_setup): + client = self._github_client( + ["os-task1-carol"], {"os-task1-carol": "Сидоров Сидор\nЛР1"} + ) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.status == "done" + assert job.results[0].status == "updated" + assert job.results[0].registered is True + written = _written_cells(bulk_setup["worksheet"]) + assert ("C5", "carol") in written + assert ("D5", "v") in written + + def test_existing_matching_username_is_not_rewritten(self, bulk_setup): + client = self._github_client(["os-task1-alice"], {"os-task1-alice": "Иванов Иван"}) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].registered is False + assert _written_cells(bulk_setup["worksheet"]) == [("D3", "v")] + + def test_conflicting_username_is_reported_and_not_graded(self, bulk_setup): + """The row already names a different account, so nothing is touched.""" + client = self._github_client(["os-task1-mallory"], {"os-task1-mallory": "Иванов Иван"}) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].status == "conflict" + assert job.total == 1 and job.processed == 1 + bulk_setup["worksheet"].batch_update.assert_not_called() + + def test_unknown_name_is_reported(self, bulk_setup): + client = self._github_client(["os-task1-dave"], {"os-task1-dave": "Неизвестный Студент"}) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].status == "unmatched" + assert "Неизвестный Студент" in job.results[0].message + + def test_missing_name_file_is_reported(self, bulk_setup): + client = self._github_client(["os-task1-dave"], {}) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].status == "unmatched" + assert "info.md" in job.results[0].message + + def test_repos_of_other_labs_are_ignored(self, bulk_setup): + client = self._github_client( + ["os-task1-alice", "os-task10-bob", "unrelated"], + {"os-task1-alice": "Иванов Иван"}, + ) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.total == 1 + assert [r.github for r in job.results] == ["alice"] + + def test_unavailable_org_repos_fail_the_job(self, bulk_setup): + client = MagicMock() + client.list_org_repos.return_value = None + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.status == "failed" + assert "репозиториев организации" in job.error + + def test_dry_run_does_not_register_usernames(self, bulk_setup): + client = self._github_client(["os-task1-carol"], {"os-task1-carol": "Сидоров Сидор"}) + job = _job(mode="by_file", dry_run=True, name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].registered is True + bulk_setup["worksheet"].batch_update.assert_not_called() + + def test_ambiguous_name_is_reported(self, bulk_setup): + bulk_setup["grid"][4][1] = "Иванов Иван" # duplicate name + client = self._github_client(["os-task1-dave"], {"os-task1-dave": "Иванов Иван"}) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.results[0].status == "ambiguous" + + def test_planning_failures_count_toward_progress(self, bulk_setup): + client = self._github_client( + ["os-task1-alice", "os-task1-dave"], + {"os-task1-alice": "Иванов Иван", "os-task1-dave": "Кто-то Другой"}, + ) + job = _job(mode="by_file", name_file="info.md") + _run(job, bulk_setup, github_client=client) + + assert job.total == 2 + assert job.processed == 2 diff --git a/tests/test_github_client.py b/tests/test_github_client.py index db0f5d3..72b73f2 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -425,6 +425,98 @@ def test_single_short_page_stops_after_one_request(self): assert call.call_count == 1 +class TestGitHubClientGetFileContent: + """Tests for get_file_content method (used by bulk grading).""" + + URL = "https://api.github.com/repos/test-org/test-repo/contents/info.md" + + def _file_payload(self, text, **overrides): + import base64 + payload = { + "type": "file", + "encoding": "base64", + "size": len(text.encode("utf-8")), + "content": base64.b64encode(text.encode("utf-8")).decode(), + } + payload.update(overrides) + return payload + + @responses.activate + def test_decodes_utf8(self): + """Cyrillic content comes back intact.""" + responses.add( + responses.GET, self.URL, + json=self._file_payload("Иванов Иван\nЛР1"), + status=200 + ) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") == "Иванов Иван\nЛР1" + + @responses.activate + def test_strips_bom(self): + """A UTF-8 BOM must not end up glued to the student's surname.""" + responses.add( + responses.GET, self.URL, + json=self._file_payload("\ufeffИванов Иван"), + status=200 + ) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") == "Иванов Иван" + + @responses.activate + def test_missing_file(self): + """A missing file returns None.""" + responses.add(responses.GET, self.URL, json={"message": "Not Found"}, status=404) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") is None + + @responses.activate + def test_directory_returns_none(self): + """A directory path answers with a list of entries, not content.""" + responses.add(responses.GET, self.URL, json=[{"name": "a.md"}], status=200) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") is None + + @responses.activate + def test_oversized_file_is_skipped(self): + """Files above the size limit are not decoded.""" + responses.add( + responses.GET, self.URL, + json=self._file_payload("Иванов Иван", size=10 * 1024 * 1024), + status=200 + ) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") is None + + @responses.activate + def test_unsupported_encoding_returns_none(self): + """GitHub omits the body of large files, answering encoding "none".""" + responses.add( + responses.GET, self.URL, + json={"type": "file", "encoding": "none", "size": 4, "content": ""}, + status=200 + ) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") is None + + @responses.activate + def test_non_utf8_content_returns_none(self): + """Binary content that is not valid UTF-8 returns None, never raises.""" + import base64 + responses.add( + responses.GET, self.URL, + json={ + "type": "file", + "encoding": "base64", + "size": 4, + "content": base64.b64encode(b"\xff\xfe\x00\x01").decode(), + }, + status=200 + ) + client = GitHubClient("test_token") + assert client.get_file_content("test-org", "test-repo", "info.md") is None + + class TestGitHubClientCreatePullRequest: """Tests for create_pull_request method."""