From 09abc996da36ffe61c5b2e0a682626412b7dd01d Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 19:43:40 +0300 Subject: [PATCH 01/10] Document team (group) lab assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit План реализации групповых лабораторных работ (issue #54), продолжение #46. Co-Authored-By: Claude Opus 5 --- docs/TEAM_ASSIGNMENTS_PLAN.md | 636 ++++++++++++++++++++++++++++++++++ 1 file changed, 636 insertions(+) create mode 100644 docs/TEAM_ASSIGNMENTS_PLAN.md diff --git a/docs/TEAM_ASSIGNMENTS_PLAN.md b/docs/TEAM_ASSIGNMENTS_PLAN.md new file mode 100644 index 0000000..16a861a --- /dev/null +++ b/docs/TEAM_ASSIGNMENTS_PLAN.md @@ -0,0 +1,636 @@ +# План разработки: групповые (командные) лабораторные работы + +Продолжение задачи #46 (`docs/REPO_GENERATION_PLAN.md`). Требования и план реализации фичи. + +## 1. Постановка задачи + +Сейчас `/join/{course_id}/{lab_id}` создаёт один приватный репозиторий на одного студента: +`{github-prefix}-{username}`, доступ выдаётся только этому студенту. Нужно поддержать лабы, +которые выполняются командами: один репозиторий на команду, доступ у всех её участников. + +Пользовательский сценарий: + +1. Преподаватель рассылает ту же ссылку `/join/{course_id}/{lab_id}` (формат ссылки не меняется). +2. Студент переходит по ней и авторизуется через GitHub OAuth (механика §3 плана #46 не меняется). +3. Вместо немедленного создания репозитория студент видит список уже созданных команд этой лабы: + человекочитаемое название, описание, состав участников, заполненность; и кнопку «Создать команду». +4. Студент присоединяется к команде или создаёт новую, задав её название и описание. +5. Студент попадает на страницу успеха со ссылкой на репозиторий команды. + +Преподаватель задаёт в конфиге лабы максимальное число участников в команде и/или максимальное +число команд. Преподавателю нужна инструкция, как удалить студента из команды, чтобы тот смог +выбрать другую (типовая ситуация: студент ошибся при выборе). + +Индивидуальные лабы, `/register`, проверка работ для индивидуальных лаб и рассылка обновлений +шаблона (`grading/propagate.py`) этой фичей не затрагиваются. + +## 2. Как это устроено в GitHub Classroom и что переиспользуется + +GHC для group assignment: + +- ведёт «set of teams» — набор команд, переиспользуемый между заданиями; +- на каждую команду создаёт **GitHub team внутри организации** с видимостью `secret`; +- называет репозиторий как `{repository prefix}-{team name}`; +- выдаёт доступ к репозиторию не отдельным студентам, а команде целиком; +- позволяет преподавателю задать максимум участников в команде и максимум команд. + +Переиспользуется **схема именования** (`{github-prefix}-{суффикс команды}`) и **сама модель** +«команда = один общий репозиторий, лимиты задаёт преподаватель, состав формируют студенты сами». + +Не переиспользуется механизм GitHub teams — см. §3.2. Отдельно учтено известное поведение GHC, +которое здесь воспроизводить не нужно: студент, вышедший из команды, в GHC не может вступить в +другую (issues education/classroom#559, #1139) — в этом плане переход между командами штатно +поддержан через удаление студента из коллабораторов репозитория. + +## 3. Архитектурное решение + +### 3.1. Команда — это репозиторий + +Источник истины о составе команды — сам репозиторий команды на GitHub: + +| Сущность | Где хранится | +|---|---| +| Команда | Репозиторий `{org}/{github-prefix}-team-{N}` | +| Состав команды | Прямые коллабораторы репозитория + непринятые приглашения | +| Человекочитаемое название и описание | Поле `description` репозитория | +| Число команд | Число репозиториев лабы с подходящим именем | + +Следствия: + +- Нового хранилища не появляется: у проекта нет БД, состояние задач намеренно держится в памяти + (`grading/propagate.py`, `grading/bulk.py`), а конфигурация — в YAML и Google Таблицах. +- Логика выдачи и починки доступа переиспользуется целиком: `RepoProvisioner._ensure_access` + вместе с исправлением протухших приглашений (замена github-reinvite) работает для командного + репозитория без изменений, отдельно для каждого участника. +- Студенты остаются outside-коллабораторами и не становятся членами организации. +- Удаление студента из команды — штатная операция GitHub: Settings → Collaborators репозитория. +- Всё, что видит студент, и всё, что правит преподаватель, — это одна сущность, репозиторий. + +### 3.2. Отклонённые варианты + +**GitHub teams в организации (подход GHC).** Членство в team возможно только для членов +организации: `PUT /orgs/{org}/teams/{slug}/memberships/{username}` для постороннего аккаунта +создаёт приглашение в организацию, а не в команду. Это меняет модель доступа всего проекта +(сегодня студент — outside collaborator), добавляет второй жизненный цикл приглашений поверх уже +решённого в #46, требует настройки базовых прав организации (`base permission: none`), чтобы +студенты не получили доступ к чужим репозиториям, и влияет на состав/тарификацию организации. + +**Собственное хранилище (JSON-файл или SQLite).** Вводит в проект персистентность, резервное +копирование и миграции, которых сейчас нет; состав команды перестаёт быть виден и правим из +интерфейса GitHub. + +**Отдельный лист в Google Таблице курса.** Второй источник истины (состав — на GitHub, названия — +в таблице), расход квоты Sheets (60 чтений в минуту) на страницу, которую одновременно открывает +вся группа, и новое обязательное требование к структуре таблицы курса. + +### 3.3. Имя репозитория и slug команды + +`repo_name = f"{github-prefix}-team-{N}"`, где `N` — наименьшее свободное натуральное число среди +уже существующих репозиториев лабы. + +Slug не выводится из введённого студентом названия: названия задаются по-русски, транслитерация +даёт нестабильные и конфликтующие имена, а переименование команды не должно требовать +переименования репозитория. Номер вычисляется из списка существующих репозиториев, поэтому +отдельного счётчика хранить не нужно. + +Разбор имени: `^{re.escape(prefix)}-team-(\d+)$`. Обязательный дефис после префикса сохраняет +существующую защиту от пересечения префиксов (`os-task1` против `os-task10`, см. +`grading/bulk.py:filter_lab_repos`). + +### 3.4. Название и описание команды + +Оба значения задаёт создатель команды; хранятся они в поле `description` репозитория: + +- при непустом описании — `f"{title} — {description}"` (разделитель: пробел, длинное тире U+2014, пробел); +- при пустом — `title`. + +Разбор — split по первому вхождению разделителя. Если разделителя нет (преподаватель отредактировал +описание вручную), вся строка показывается как название — деградация без ошибки. + +Валидация ввода: + +| Поле | Правило | +|---|---| +| `title` | обязательно, 3–60 символов после `strip()`; переводы строк и управляющие символы запрещены; последовательности пробелов схлопываются; не должно содержать разделитель `" — "` | +| `description` | необязательно, до 200 символов, те же правила очистки | +| Уникальность | `title` уникален внутри лабы без учёта регистра и краевых пробелов; иначе `409 TITLE_TAKEN` | + +Итоговая строка не превышает лимит GitHub на `description` (350 символов). + +## 4. Конфигурация курса + +Новая секция в `labs.*`. Наличие секции `team` делает лабу командной. + +```yaml +labs: + "5": + github-prefix: os-task5 + short-name: ЛР5 + template-repo: suai-os-2026/os-task5-template + repo-provisioning: fork # работает и с template, и с fork + team: + size-max: 4 # максимум участников в команде + count-max: 8 # максимум команд на лабу +``` + +### `team` +**Тип:** `dict` +**Описание:** Признак командной лабы. Присутствие секции (даже пустой, `team: {}`) переключает +`/join/{course_id}/{lab_id}` на командный сценарий. + +### `team.size-max` +**Тип:** `int`, **по умолчанию:** без ограничения +**Описание:** Максимум участников в одной команде. Учитываются и принятые приглашения, и +ожидающие: студент, получивший приглашение, но не открывший его, уже занимает место. + +### `team.count-max` +**Тип:** `int`, **по умолчанию:** без ограничения +**Описание:** Максимум команд у лабы. При достижении лимита кнопка создания команды недоступна, +присоединение к существующим командам продолжает работать. + +Требования и проверки конфигурации: + +- `template-repo` обязателен, как и для индивидуальных лаб; +- `repo-provisioning` работает в обоих режимах (`template`, `fork`); +- `size-max` и `count-max`, если заданы, должны быть целыми ≥ 1, иначе `/join` для этой лабы + отдаёт ошибку конфигурации (как при неизвестном `repo-provisioning`); +- `taskid-max` вместе с `team` игнорируется — проверка TASKID для командных лаб не выполняется + (§10.3); при загрузке конфига это пишется в лог как предупреждение. + +Документируется в `docs/COURSE_CONFIG.md` по образцу существующих полей секции `labs.*`. + +## 5. Пользовательский сценарий и состояния экрана + +| Состояние | Когда | Что показывается | +|---|---|---| +| `landing` | студент не авторизован | название курса и лабы, пометка «командная работа», лимиты, кнопка «Войти через GitHub» | +| `picker` | авторизован, в команде не состоит | список команд (название, описание, участники, «3 из 4»), кнопка «Присоединиться» у каждой, форма создания команды | +| `member` | авторизован, состоит в команде | карточка своей команды, ссылка на репозиторий, кнопка «Восстановить доступ» | +| `error` | ошибка любого шага | сообщение по коду ошибки (§8.3) | + +Ссылка не одноразовая: повторный переход по ней участником команды приводит в состояние `member`, +а кнопка «Восстановить доступ» повторно проходит §4 плана #46 (пересоздание протухшего +приглашения) — для командных репозиториев это единственная замена github-reinvite. + +## 6. Сессия студента после OAuth + +Для индивидуальной лабы весь сценарий укладывается в один колбэк. Командный требует диалога +(показать список → дождаться выбора), поэтому после подтверждения username заводится короткая +сессия студента. + +- Cookie `join_session`: `HttpOnly`, `SameSite=Lax`, `path=/join`, `max_age=1800`. +- Значение — подписанный тем же `TimestampSigner` (`main.py:143`) base64-JSON + `{"username": ..., "course_id": ..., "lab_id": ...}`, по образцу `_build_join_state` / + `_parse_join_state`. +- Хелперы: `_build_join_session(username, course_id, lab_id)` и `_parse_join_session(cookie)`. +- Зависимость `require_join_session(request, course_id, lab_id)` для командных эндпоинтов: + проверяет подпись и срок, сверяет `course_id`/`lab_id` из cookie с путём запроса, возвращает + username. Несовпадение — `401`, чтобы сессия, полученная для одной лабы, не работала для другой. + +Username берётся **только** из cookie. Из тела запроса и query-параметров он не принимается ни при +каких условиях — это то же требование, что и в §3.2 плана #46: подтверждённая личность приходит +исключительно из серверного обмена `code → access_token → GET /user`. + +В продакшене фронтенд и backend работают за одним прокси (`SameSite=Lax` достаточно). В dev-режиме +фронтенд на `localhost:8080` обращается к backend на `localhost:8000`: порт не входит в понятие +«site», поэтому `Lax` работает и там, но запросы кросс-оригинные и требуют `credentials: "include"` +(так уже сделано в админке, `frontend/courses-front/src/components/admin/LabList/index.jsx:45`). + +## 7. Backend: новый модуль `grading/teams.py` + +Оркестратор командных операций, по образцу `grading/repo_provisioning.py`: получает настроенный +`GitHubClient` на серверном `GITHUB_TOKEN`, не знает про FastAPI и не ходит в Google Sheets. + +```python +TEAM_SLUG_RE = re.compile(r"^team-\d+$") +TEAMS_CACHE_TTL_SECONDS = 30 + +@dataclass +class TeamInfo: + slug: str # "team-3" + number: int # 3 + repo_name: str # "os-task5-team-3" + repo_url: str + title: str + description: str + members: list[str] # принятые коллабораторы + pending: list[str] # приглашённые, но не принявшие + members_unknown: bool = False # состав не удалось прочитать, см. §7.1 + @property + def size(self) -> int: return len(self.members) + len(self.pending) + +class TeamRegistry: + def __init__(self, github: GitHubClient): ... + def list_teams(self, org, github_prefix, teachers=(), fresh=False) -> list[TeamInfo] | None + def find_member_team(self, teams, username) -> TeamInfo | None + def member_index(self, teams) -> dict[str, TeamInfo] # username.casefold() -> команда + def next_team_number(self, teams) -> int + def create_team(self, ...) -> TeamActionResult + def join_team(self, ...) -> TeamActionResult +``` + +### 7.1. Сбор списка команд + +1. `list_org_repos(org)` — 1–3 запроса на страницу пагинации. +2. Отбор по `^{prefix}-team-(\d+)$`; `description` берётся из этого же ответа, без доп. запросов. +3. Для каждой команды: `list_collaborators(org, repo, affiliation="direct")` и + `list_invitations(org, repo)` — 2 запроса на команду. +4. Участником считается коллаборатор с `permissions.push == True` и `permissions.admin == False`. + Этот фильтр отсекает владельцев организации, которые попадают в список коллабораторов по + организационной роли. Дополнительно исключаются логины из `course.github.teachers` + (сравнение без учёта регистра; список смешанный — ФИО и логины, поэтому он вспомогательный, + а основной критерий — права). + +Любая ошибка GitHub API на шаге 1 — `None` (эндпоинт отвечает понятной ошибкой); ошибка на шаге 3 +для отдельной команды — команда возвращается с флагом `members_unknown`, чтобы одна недоступная +команда не ломала весь экран. + +### 7.2. Кэш и блокировки + +- `_teams_cache: dict[(org, prefix), tuple[float, list[TeamInfo]]]`, TTL 30 с. Используется + операциями чтения: списком команд (`GET .../teams`) и определением команды студента при проверке + работ (§10). Группа в 30 человек, одновременно открывшая страницу, тратит один набор запросов + вместо тридцати. +- `_lab_locks: dict[(course_id, lab_id), threading.Lock]`, создаётся под общим мьютексом. + Любая изменяющая операция (создание, присоединение) выполняется под блокировкой своей лабы и + **внутри неё** перечитывает состояние с `fresh=True`, игнорируя кэш. Кэш инвалидируется после + успешной мутации. +- Как и job-состояние `propagate.py`/`bulk.py`, это корректно только при одном воркере uvicorn — + ограничение уже зафиксировано в `docs/PROJECT_DESCRIPTION.md` и здесь не усиливается. + +Блокировка не защищает от изменений, сделанных напрямую на GitHub (преподаватель добавил +коллаборатора руками в момент присоединения студента). Лимиты по этой причине трактуются как +проверка на момент операции, а не как жёсткий инвариант. + +### 7.3. Создание команды + +Под блокировкой лабы: + +1. `teams = list_teams(fresh=True)`; при `None` — ошибка `TEAMS_UNAVAILABLE`. +2. Студент уже в команде → `ALREADY_IN_TEAM` (в ответе — slug и ссылка на его команду). +3. `count-max` задан и `len(teams) >= count-max` → `TEAM_LIMIT_REACHED`. +4. Название занято → `TITLE_TAKEN`. +5. `slug = f"team-{next_team_number(teams)}"`; выполняется `repo_exists(org, f"{prefix}-{slug}")`. + Репозиторий с таким именем уже существует (список организации отстал от реального состояния или + имя занято посторонним репозиторием) → `SLUG_RACE`, студенту предлагается повторить. +6. `RepoProvisioner.provision(org, github_prefix, template_repo, repo_suffix=slug, mode=..., + access_username=username)` — создание репозитория и выдача доступа создателю. +7. При успехе — `update_repo(org, repo_name, {"description": composed_description})`. Этот вызов + нужен в обоих режимах: `generate` не проставляет описание, а форк наследует описание шаблона, + которое здесь заменяется названием команды. Ошибка на этом шаге логируется, но не отменяет + создание: репозиторий рабочий, название можно проставить позже вручную. +8. Инвалидация кэша, возврат `TeamInfo` и `repo_url`. + +Гонка по имени репозитория (два студента одновременно получили один `N`) разрешается блокировкой +лабы: оба запроса выполняются последовательно, второй видит уже созданную команду в свежем списке +и получает следующий номер. Проверка `repo_exists` на шаге 5 — страховка на случай, когда +`list_org_repos` отдаёт неполный список сразу после создания репозитория. Если гонка всё же +дойдёт до обработки `422` внутри `RepoProvisioner` («репозиторий появился параллельно»), +`provision` вернёт успех и студент окажется участником созданной параллельно команды с чужим +названием — известная деградация, недостижимая при одном воркере; менять поведение +`RepoProvisioner` ради неё не требуется. + +### 7.4. Присоединение к команде + +Под блокировкой лабы: + +1. `slug` из пути проверяется по `TEAM_SLUG_RE`. Имя репозитория собирается сервером как + `f"{github-prefix}-{slug}"`; из запроса имя репозитория не принимается никогда. +2. `teams = list_teams(fresh=True)`; команда не найдена → `TEAM_NOT_FOUND`. +3. Студент состоит в другой команде → `ALREADY_IN_TEAM`. +4. Студент уже в этой команде → выполняется только починка доступа (шаг 6) и возвращается `OK`. +5. `size-max` задан и `team.size >= size-max` → `TEAM_FULL`. +6. `RepoProvisioner.provision(..., repo_suffix=slug, access_username=username)` — репозиторий уже + существует, поэтому фактически выполняется `_ensure_access` (плюс `_repair_fork` в fork-режиме). +7. Инвалидация кэша, возврат `repo_url`. + +## 8. Backend: эндпоинты + +### 8.1. Изменённые + +| Эндпоинт | Изменение | +|---|---| +| `GET /join/{course_id}/{lab_id}` | В ответ добавляется `"team": {"enabled": bool, "size_max": int\|null, "count_max": int\|null, "teams_count": int\|null}`. Составы команд здесь не отдаются — эндпоинт публичный. `teams_count` заполняется из кэша, при недоступности GitHub — `null`. | +| `GET /join/{course_id}/{lab_id}/start` | Без изменений. | +| `GET /join/callback` | Для командной лабы после получения `username` репозиторий не создаётся: выставляется cookie `join_session` и происходит редирект на `/join/{course_id}/{lab_id}?status=authenticated`. Для индивидуальной — поведение прежнее. | + +### 8.2. Новые + +Все требуют `join_session` и `@limiter.limit(...)` по образцу существующих `/join`-эндпоинтов. + +| Метод и путь | Назначение | Лимит | +|---|---|---| +| `GET /join/{course_id}/{lab_id}/teams` | Список команд, своя команда, флаг `can_create`, лимиты | `30/minute` | +| `POST /join/{course_id}/{lab_id}/teams` | Создание команды, тело `{"title": str, "description": str \| null}` | `10/minute` | +| `POST /join/{course_id}/{lab_id}/teams/{slug}/join` | Присоединение к команде либо починка доступа, если студент уже в ней | `10/minute` | + +Ответ `GET .../teams`: + +```json +{ + "course_name": "…", "lab_short_name": "ЛР5", + "username": "student1", + "size_max": 4, "count_max": 8, + "can_create": true, + "my_team": "team-2", + "teams": [ + {"slug": "team-1", "title": "Пингвины", "description": "…", + "members": ["alice", "bob"], "pending": ["carol"], + "size": 3, "is_full": false, "is_mine": false, "members_unknown": false, + "repo_url": "https://github.com/org/os-task5-team-1"} + ] +} +``` + +`repo_url` отдаётся только для команды студента; для чужих команд — `null` (ссылка на приватный +репозиторий, к которому у него нет доступа, бесполезна и вводит в заблуждение). Логины участников +показываются: это публичные идентификаторы GitHub, и именно по ним студент находит команду своих +однокурсников. ФИО не показываются — сценарий `/join` не знает группу студента и не открывает +Google Таблицу. + +### 8.3. Коды ошибок + +Отдаются в `detail` как стабильные коды (фронтенд переводит их сам, как уже сделано для +`ERROR_TRANSLATION_KEYS` в `JoinLab/state.js`). + +| Код | HTTP | Причина | +|---|---|---| +| `NOT_A_TEAM_LAB` | 400 | Командный эндпоинт вызван для индивидуальной лабы | +| `SESSION_REQUIRED` | 401 | Нет cookie, подпись невалидна, срок истёк, лаба в cookie не та | +| `TEAMS_UNAVAILABLE` | 502 | Не удалось получить список репозиториев организации | +| `TEAM_NOT_FOUND` | 404 | Нет команды с таким slug | +| `ALREADY_IN_TEAM` | 409 | Студент уже состоит в команде этой лабы | +| `TEAM_FULL` | 409 | Достигнут `size-max` | +| `TEAM_LIMIT_REACHED` | 403 | Достигнут `count-max` | +| `TITLE_TAKEN` | 409 | Название команды уже занято в этой лабе | +| `INVALID_TITLE` | 400 | Название не прошло валидацию §3.4. Валидация выполняется вручную, а не Pydantic-валидатором: FastAPI отдал бы для неё свой формат `422` со списком ошибок вместо стабильного кода | +| `SLUG_RACE` | 409 | Гонка при создании, нужно повторить | +| Коды `ProvisionResult.error_code` | 400/502 | Пробрасываются как есть (`TEMPLATE_NOT_FOUND`, `RATE_LIMITED`, `INVITE_FAILED`, …) — фронтенд уже умеет их переводить | + +## 9. Изменения в существующем коде + +### 9.1. `grading/repo_provisioning.py` + +Единственное содержательное изменение: `provision()` сейчас использует `repo_suffix` и как суффикс +имени репозитория, и как username для выдачи доступа (`_ensure_access(org, repo_name, repo_suffix)`). +Для команд это разные значения. + +```python +def provision(self, org, github_prefix, template_repo, repo_suffix, + mode="template", access_username=None) -> ProvisionResult: + ... + access_error = self._ensure_access(org, repo_name, access_username or repo_suffix) +``` + +Обратная совместимость сохраняется: для индивидуальных лаб параметр не передаётся. + +### 9.2. `grading/github_client.py` + +Новый метод: + +```python +def list_collaborators(self, org, repo, affiliation="direct") -> list[dict] | None: + """GET /repos/{org}/{repo}/collaborators?affiliation=... (все страницы).""" +``` + +Реализуется через существующий `_get_all_pages`. В комментарии к `is_direct_collaborator` +(github_client.py:568) стоит отметка о том, что поведение `affiliation` для одиночной проверки не +документировано и командному варианту его надо перепроверить: для подсчёта состава используется +именно `list_collaborators` с документированным `affiliation`, а `is_direct_collaborator` +продолжает применяться только как быстрая проверка «есть ли доступ» перед выдачей приглашения. + +Метод `remove_collaborator` нужен только для опционального этапа 7 (админка). + +### 9.3. `main.py` + +- Хелпер `_load_lab_for_join` дополняется разбором и валидацией секции `team`; возвращает её + вместе с `course_info`, `lab_config`, `org`. +- В `join_callback` — ветка для командной лабы (§8.1). +- Новые эндпоинты §8.2 и зависимость `require_join_session`. +- Константа `JOIN_SESSION_MAX_AGE = 1800`. + +## 10. Проверка (grading) командных лаб + +Оценка ставится каждому участнику в его собственную строку таблицы. Репозиторий у команды один, +поэтому проверяется он один раз, а результат разносится по строкам участников. + +### 10.1. Определение репозитория студента + +`grading/bulk.py:evaluate_student` получает необязательный параметр `repo_name: str | None = None`; +при `None` имя строится как сейчас (`repo_name_for`). Больше в сигнатуре ничего не меняется. + +Для командных лаб имя приходит из `TeamRegistry.member_index()`. + +### 10.2. Одиночная проверка (`POST /courses/.../labs/{lab_id}/grade`) + +1. Лаба командная → строится индекс участников (`list_teams` + `member_index`). Используется тот + же кэш `TeamRegistry` (§7.2): при проверке подряд нескольких студентов список команд читается + с GitHub один раз. +2. Команда студента не найдена → `404` с сообщением «Вы ещё не состоите в команде для этой + лабораторной работы». +3. `evaluate_student(..., repo_name=team.repo_name)`; `SheetContext` собирается как сейчас, по + строке этого студента, поэтому защита ячейки работает без изменений. +4. Результат пишется только в строку обратившегося студента. Остальным участникам оценку + проставляет групповая проверка (§10.3) либо их собственный запуск проверки. Публичный эндпоинт + не пишет в чужие строки. + +### 10.3. Групповая проверка (`grading/bulk.py`) + +В `run_bulk_grading` для командной лабы меняется планирование и цикл: + +1. Планирование — только режим `by_sheet`: студенты берутся из таблицы по колонке `GitHub`. + Режим `by_file` (сопоставление по `student-name-file`) для командных лаб неприменим — в + репозитории один файл с ФИО на несколько человек; фронтенд не предлагает этот режим, а backend + отвечает `400`, если его всё же запросили. +2. Целям (`_Target`) проставляется `repo` из `member_index`. Студенты без команды попадают в отчёт + со статусом `no_team` (новый статус, аналог существующего `unmatched`) и не проверяются. +3. Цели группируются по репозиторию команды. Для каждой команды `evaluate_student` вызывается + **один раз**, с синтетическим `SheetContext`: `current_cell_value=""` (защита ячейки на этом + шаге не применяется), `student_order=None`, реальные `deadline` и `decimal_separator`. +4. Результат разносится по участникам: для каждого берётся его собственная ячейка из уже + прочитанной сетки (`cell_from_grid`) и применяется `can_overwrite_cell`; при запрете — + `rejected` с текущим значением, иначе — `updated` с тем же `cell_value`, что у команды. + Статусы `error`/`pending` копируются всем участникам как есть. +5. Запись в таблицу — существующим механизмом батчей (`WRITE_BATCH_SIZE`). + +Такой порядок не требует рефакторинга `evaluate_student`: тяжёлая часть (файлы, коммиты, check-runs, +скачивание логов job'ов) выполняется один раз на команду, а не один раз на студента. + +Проверка TASKID для командных лаб не выполняется: номер варианта выводится из порядкового номера +студента в таблице, у команды такого номера нет. В `taskid_column()` (`grading/bulk.py`) +добавляется ранний возврат `None` при наличии секции `team` в конфиге лабы — после этого +`student_order` не читается ни в одиночной, ни в групповой проверке. + +Отчёт о прогоне дополняется колонкой «Команда» (`BulkResult.team: str | None` — название команды). + +## 11. Frontend + +- `frontend/courses-front/src/components/JoinLab/index.jsx` — маршрутизация между состояниями §5. + Чистая логика выбора состояния выносится в существующий `state.js` и покрывается тестами рядом с + `state.test.js`. +- Новые компоненты в том же каталоге: `TeamList.jsx` (карточки команд с кнопкой «Присоединиться»), + `CreateTeamForm.jsx` (поля названия и описания, клиентская валидация по §3.4), `MyTeamCard.jsx`. + Стили — в существующем `styled.js`. +- `frontend/courses-front/src/api/index.js`: `fetchJoinTeams`, `createJoinTeam`, `joinJoinTeam` — + все с `credentials: "include"` и отображением HTTP-статусов в стабильные коды, как это уже + сделано в `fetchJoinLab`. +- Локализация: новые ключи `join.team.*` и `join.errors.*` для кодов §8.3 в + `frontend/courses-front/src/locales/{en,ru,zh}/translation.json`. +- Заполненность команды показывается как «3 из 4»; при отсутствии `size-max` — просто число + участников. Непринявшие приглашение участники помечаются («приглашение отправлено»), чтобы было + видно, почему место занято. +- Кнопка «Присоединиться» блокируется для полных команд и когда студент уже в команде; форма + создания скрывается при достижении `count-max` с пояснением. + +## 12. Инструкция для преподавателя + +Отдельный раздел в `docs/PROJECT_DESCRIPTION.md` (и краткая ссылка на него из `docs/COURSE_CONFIG.md`). +Содержание: + +**Как перевести лабу в командный режим.** Добавить секцию `team` в конфиг лабы (§4). Ссылка для +студентов не меняется: `https:///join/{course_id}/{lab_id}`. + +**Как устроены репозитории команд.** `{github-prefix}-team-{N}` в организации курса, приватные, +участники — прямые коллабораторы с правом `push`. Название команды хранится в поле `description` +репозитория и правится преподавателем прямо на GitHub. + +**Как удалить студента из команды (студент ошибся при выборе).** + +1. Открыть репозиторий команды: `https://github.com/{организация}/{github-prefix}-team-{N}`. +2. Settings → Collaborators and teams. +3. Если студент принял приглашение — напротив его логина нажать `Remove`. +4. Если приглашение ещё не принято — оно отображается в разделе `Pending invitations`, нажать + `Cancel invitation`. Этот шаг обязателен: непринятое приглашение продолжает занимать место в + команде и удерживает студента привязанным к ней. +5. Сообщить студенту, чтобы он снова открыл ссылку `/join/...` — он увидит список команд и сможет + выбрать другую. + +Коммиты, которые студент успел сделать в старом репозитории, остаются в его истории; при +необходимости преподаватель удаляет их обычными средствами Git. + +**Как переименовать команду.** Отредактировать `description` репозитория: `Название — описание`. + +**Как удалить пустую команду.** Удалить репозиторий на GitHub. Номер `N` освободится и будет +переиспользован следующей созданной командой. + +**Как закрыть создание новых команд.** Выставить `team.count-max` равным текущему числу команд. +Вступление в уже созданные неполные команды при этом остаётся доступным: отдельного признака +«формирование команд закрыто» в первой версии нет (§14). Убирать секцию `team` для этого нельзя — +лаба перестанет быть командной и проверка работ перестанет находить репозитории команд. + +## 13. Ограничения, гонки и квоты + +- **Одновременное вступление в последнее свободное место.** Разрешается блокировкой лабы (§7.2). + При нескольких воркерах uvicorn лимиты перестают быть точными — проект и так требует одного + воркера. +- **Действия преподавателя напрямую на GitHub** в момент операции студента лимитами не + контролируются; расхождение исправляется на следующем чтении списка. +- **Квота GitHub API.** Список команд для группы: `1 + 2 × число_команд` запросов, кэшируется на + 30 с. При 10 командах и 30 студентах, открывающих страницу одновременно, — около 21 запроса + вместо 630. Лимит classic PAT — 5000 запросов в час. +- **Вторичные лимиты GitHub** на всплески создания репозиториев и приглашений уже обрабатываются + (`is_rate_limited`, код `RATE_LIMITED`); отдельной паузы между операциями студентов не требуется, + так как операции инициируются людьми, а не циклом. +- **Непринятое приглашение занимает место** — это осознанное решение: иначе один студент мог бы + занять места во всех командах. +- **Модерация названий команд не выполняется.** Текст очищается от управляющих символов и + обрезается по длине; за содержание отвечает преподаватель, который может отредактировать + `description` или удалить репозиторий. Логин создателя команды пишется в лог. + +## 14. Не входит в объём + +- Вариант задания (TASKID) на команду — для командных лаб проверка TASKID отключена (§10.3). + Возможное развитие: выводить номер варианта из `N` в slug команды. +- Минимальный размер команды и проверка «все команды укомплектованы». +- Самостоятельный выход студента из команды — только через преподавателя (§12). +- Перенос уже созданного индивидуального репозитория в командный и обратно. +- Переиспользование состава команд между лабами (аналог «set of teams» в GHC): состав задаётся + заново для каждой лабы, поскольку и репозиторий у каждой лабы свой. +- MOSS/проверка плагиата между командными репозиториями. +- Признак «формирование команд закрыто» (дедлайн формирования команд). Создание новых команд + закрывается через `count-max`, вступление в уже созданные неполные команды остаётся открытым. + +## 15. Тесты + +Новый файл `tests/test_teams.py` (моки GitHub API, как в `tests/test_repo_provisioning.py`): + +- разбор имён репозиториев команд, включая пересечение префиксов (`os-task1` / `os-task10`); +- `next_team_number` при отсутствии команд, при подряд идущих и при «дырке» в нумерации; +- сборка/разбор `description`: с описанием, без описания, с разделителем внутри введённого текста, + при отредактированном вручную описании; +- валидация названия: пустое, слишком длинное, с переводом строки, с разделителем, дубликат; +- состав команды: исключение владельца организации (`permissions.admin`), учёт непринятых + приглашений, недоступность `list_collaborators` для одной команды; +- создание команды: успех; при достигнутом `count-max`; когда студент уже в команде; +- присоединение: успех; в полную команду; в свою же команду (только починка доступа); к + несуществующему slug; при `slug`, не проходящем `TEAM_SLUG_RE`; +- кэш: повторный `GET .../teams` в пределах TTL не ходит в GitHub; мутация сбрасывает кэш. + +Дополнения к существующим файлам: + +- `tests/test_join_endpoints.py`: колбэк для командной лабы выставляет cookie и не создаёт + репозиторий; командные эндпоинты без cookie отвечают `401`; cookie другой лабы не подходит; + username из тела запроса игнорируется. +- `tests/test_repo_provisioning.py`: `access_username` отличается от `repo_suffix` — доступ + выдаётся студенту, имя репозитория собирается из slug. +- `tests/test_bulk_grading.py`: командная лаба — репозиторий проверяется один раз на команду, + оценка попадает всем участникам; защита ячейки применяется индивидуально; студент без команды + получает `no_team`; режим `by_file` отклоняется. + +## 16. Чек-лист приёмки по этапам + +**Этап 1. Конфигурация** +- [ ] В `docs/COURSE_CONFIG.md` описаны `team`, `team.size-max`, `team.count-max`. +- [ ] Некорректные значения (`size-max: 0`, строка вместо числа) дают понятную ошибку конфигурации + на `/join/...`, а не 500. +- [ ] Для командной лабы `taskid-max` игнорируется, в лог пишется предупреждение. +- [ ] Заведены тестовый курс с командной лабой и репозиторий-шаблон. + +**Этап 2. Сессия студента** +- [ ] Колбэк OAuth для командной лабы выставляет `join_session` и не создаёт репозиторий. +- [ ] Cookie `HttpOnly`, `SameSite=Lax`, `path=/join`, срок 30 минут. +- [ ] Командные эндпоинты берут username только из cookie; подделанное тело запроса игнорируется. +- [ ] Cookie, полученная для другой лабы или курса, отклоняется с `401`. + +**Этап 3. Список команд** +- [ ] `GET /join/{course}/{lab}/teams` возвращает команды с названием, описанием, участниками и + непринятыми приглашениями. +- [ ] Владелец организации и логины из `teachers` не попадают в состав команд. +- [ ] `repo_url` отдаётся только для своей команды. +- [ ] Повторный запрос в пределах TTL не порождает новых обращений к GitHub. + +**Этап 4. Создание и присоединение** +- [ ] Первая команда получает `team-1`, следующая — `team-2`; после удаления `team-1` следующая + снова получает `team-1`. +- [ ] Репозиторий создаётся из `template-repo` в обоих режимах `repo-provisioning`, приватный, + создатель добавлен коллаборатором. +- [ ] `description` репозитория содержит название и описание команды. +- [ ] `count-max` блокирует создание, `size-max` блокирует присоединение; непринятое приглашение + учитывается в размере. +- [ ] Студент, состоящий в команде, не может вступить во вторую и не может создать новую. +- [ ] Повторный переход по ссылке участником команды приводит на карточку его команды; кнопка + «Восстановить доступ» пересоздаёт протухшее приглашение (проверено вручную на github.com). + +**Этап 5. Проверка работ** +- [ ] Одиночная проверка находит репозиторий команды по логину студента и пишет оценку в его строку. +- [ ] Студент без команды получает понятное сообщение, а не «репозиторий не найден». +- [ ] Групповая проверка обращается к репозиторию команды один раз и проставляет оценку всем + участникам; защита ячейки срабатывает индивидуально. +- [ ] Студенты без команды помечены в отчёте статусом `no_team`. +- [ ] Проверка индивидуальных лаб не изменилась (`tests/test_grade_lab_characterization.py` зелёный). + +**Этап 6. Frontend и документация** +- [ ] Все состояния §5 отображаются, ошибки §8.3 переведены на ru/en/zh. +- [ ] Полный сценарий пройден вручную двумя тестовыми аккаунтами: первый создаёт команду, второй + присоединяется, оба получают доступ к одному репозиторию. +- [ ] Инструкция §12 добавлена в `docs/PROJECT_DESCRIPTION.md`; проверено, что после удаления + студента из коллабораторов он действительно может выбрать другую команду. +- [ ] `CLAUDE.md` дополнен разделом о командных лабах. +- [ ] `pytest tests/ -v` проходит. + +**Этап 7 (опционально, вне минимального объёма). Управление командами из админки** +- [ ] `GET /admin/courses/{course_id}/labs/{lab_id}/teams` — список команд с составом. +- [ ] `DELETE /admin/courses/{course_id}/labs/{lab_id}/teams/{slug}/members/{username}` — удаление + участника: `remove_collaborator` плюс отмена непринятого приглашения. +- [ ] Раздел «Команды» на странице `/admin/courses/{course_id}/labs` для лаб с секцией `team`. +- [ ] Все эндпоинты защищены зависимостью `require_admin`. From 34136f1d1afdde8fe3ae1b1d30ab007a4af1c9b2 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 19:51:31 +0300 Subject: [PATCH 02/10] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D1=82=D1=8C=20=D1=81=D0=B5=D0=BA=D1=86=D0=B8=D1=8E=20team=20?= =?UTF-8?q?=D0=B2=20=D0=BA=D0=BE=D0=BD=D1=84=D0=B8=D0=B3=20=D0=BB=D0=B0?= =?UTF-8?q?=D0=B1=D0=BE=D1=80=D0=B0=D1=82=D0=BE=D1=80=D0=BD=D0=BE=D0=B9=20?= =?UTF-8?q?=D1=80=D0=B0=D0=B1=D0=BE=D1=82=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Присутствие секции team (даже пустой) делает лабу командной. Ограничения size-max и count-max необязательны, но если заданы - должны быть целыми числами не меньше 1: некорректное значение отдаёт понятную ошибку конфигурации на /join, а не 500. Для командной лабы проверка TASKID не выполняется: номер варианта выводится из порядкового номера студента в таблице, у команды такого номера нет. Сочетание team и taskid-max пишется в лог при старте. Co-Authored-By: Claude Opus 5 --- docs/COURSE_CONFIG.md | 35 +++++++++++++++ grading/__init__.py | 13 ++++++ grading/bulk.py | 11 ++++- grading/teams.py | 87 ++++++++++++++++++++++++++++++++++++ main.py | 65 ++++++++++++++++++++++++--- tests/test_bulk_grading.py | 5 +++ tests/test_join_endpoints.py | 30 +++++++++++++ tests/test_lab_resolution.py | 4 +- tests/test_teams.py | 85 +++++++++++++++++++++++++++++++++++ 9 files changed, 326 insertions(+), 9 deletions(-) create mode 100644 grading/teams.py create mode 100644 tests/test_teams.py diff --git a/docs/COURSE_CONFIG.md b/docs/COURSE_CONFIG.md index db65227..9a7a534 100644 --- a/docs/COURSE_CONFIG.md +++ b/docs/COURSE_CONFIG.md @@ -240,6 +240,41 @@ labs: **Обновление стартового кода уже созданных репозиториев.** Только для `repo-provisioning: fork` - в админке (`/admin/courses/{course_id}/labs`) появляется кнопка «Обновить репозитории студентов», которая рассылает предложение обновления (pull request) во все репозитории-форки этой лабы. Слияние PR остаётся на усмотрение студента - это не принудительный push. Репозитории, созданные до переключения на `fork` (через `generate` или в GitHub Classroom), в рассылку не попадают - для них нет форк-связи с шаблоном. Подробности - в `docs/PROJECT_DESCRIPTION.md`, раздел «Рассылка обновлений стартового кода». +### `team` +**Тип:** `dict` +**Описание:** Признак командной (групповой) лабораторной работы. Присутствие секции - даже пустой (`team: {}`) - переключает ссылку `/join/{course_id}/{lab_id}` на командный сценарий: вместо немедленного создания личного репозитория студент видит список команд лабы и присоединяется к одной из них или создаёт новую. Репозиторий у команды один: `{github-prefix}-team-{N}`, все участники - его прямые коллабораторы. Формат ссылки для студентов не меняется. + +Требования те же, что и для индивидуальной лабы: обязателен `template-repo`; `repo-provisioning` работает в обоих режимах (`template` и `fork`). + +**Пример:** +```yaml +labs: + "5": + github-prefix: os-task5 + short-name: ЛР5 + template-repo: suai-os-2026/os-task5-template + repo-provisioning: fork + team: + size-max: 4 + count-max: 8 +``` + +Подробности - в `docs/PROJECT_DESCRIPTION.md`, раздел «Командные лабораторные работы» (в том числе как удалить студента из команды, переименовать её и закрыть создание новых). + +### `team.size-max` +**Тип:** `int` +**По умолчанию:** без ограничения +**Описание:** Максимальное число участников в одной команде. Учитываются и принятые приглашения, и ожидающие: студент, получивший приглашение, но ещё не открывший его, уже занимает место. По достижении лимита кнопка «Присоединиться» у этой команды недоступна. + +### `team.count-max` +**Тип:** `int` +**По умолчанию:** без ограничения +**Описание:** Максимальное число команд у лабы. По достижении лимита создание новых команд закрывается, вступление в уже созданные неполные команды продолжает работать. + +**Валидация.** `size-max` и `count-max`, если заданы, должны быть целыми числами не меньше 1. Некорректное значение (`0`, строка, дробное) не игнорируется молча: ссылка `/join/...` для такой лабы отдаёт ошибку конфигурации, как и при отсутствующем `template-repo`. + +**`taskid-max` для командной лабы игнорируется:** номер варианта выводится из порядкового номера студента в таблице, у команды такого номера нет, поэтому проверка TASKID для командных лаб не выполняется. При старте бэкенда такое сочетание пишется в лог как предупреждение. + --- ## CI/CD опции diff --git a/grading/__init__.py b/grading/__init__.py index 1b89564..edb6028 100644 --- a/grading/__init__.py +++ b/grading/__init__.py @@ -9,6 +9,7 @@ - sheets_client: Google Sheets helpers - grader: Orchestrator for grading workflow - repo_provisioning: Orchestrator for the /join student repo creation flow +- teams: Team (group) lab assignments - one repository per team - propagate: Orchestrator for propagating template updates via fork PRs (admin) """ @@ -76,6 +77,13 @@ ProvisionStatus, ) +from .teams import ( + TeamConfig, + TeamConfigError, + is_team_lab, + parse_team_config, +) + from .propagate import ( PropagateJob, PropagateResult, @@ -166,6 +174,11 @@ "RepoProvisioner", "ProvisionResult", "ProvisionStatus", + # teams + "TeamConfig", + "TeamConfigError", + "is_team_lab", + "parse_team_config", # propagate "PropagateJob", "PropagateResult", diff --git a/grading/bulk.py b/grading/bulk.py index 43a4042..27df35a 100644 --- a/grading/bulk.py +++ b/grading/bulk.py @@ -31,6 +31,7 @@ from .grader import LabGrader, GradeStatus from .penalty import calculate_penalty, format_grade_with_penalty, PenaltyStrategy from .score import format_grade_with_score, format_score +from .teams import is_team_lab from .sheets_client import ( calculate_lab_column, can_overwrite_cell, @@ -104,8 +105,16 @@ def taskid_column( 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`) + this lab (a team lab, no `task-id-column`, no `taskid-max`, or + `ignore-task-id`) """ + # A variant number is derived from the student's position in the sheet, + # and a team has no such position - the check is off for team labs, and + # `student_order` is then read by neither the single nor the bulk run + # (docs/TEAM_ASSIGNMENTS_PLAN.md §10.3). + if is_team_lab(lab_config): + return None + column = course_info.get("google", {}).get("task-id-column") if column is None: return None diff --git a/grading/teams.py b/grading/teams.py new file mode 100644 index 0000000..2560a6a --- /dev/null +++ b/grading/teams.py @@ -0,0 +1,87 @@ +""" +Team (group) lab assignments: one repository per team, shared by its members. + +Continuation of the /join student repo creation flow (see +docs/REPO_GENERATION_PLAN.md); the full design lives in +docs/TEAM_ASSIGNMENTS_PLAN.md. + +The source of truth about a team is the team's repository itself: its name +carries the team's slug (`{github-prefix}-team-{N}`), its `description` field +carries the human-readable title and description, and its direct +collaborators plus pending invitations are the roster. No new storage is +introduced - the project has no database, and everything a student sees or a +teacher edits is one GitHub entity. + +Like grading/repo_provisioning.py, this module gets a GitHubClient configured +with the server's GITHUB_TOKEN, knows nothing about FastAPI and never touches +Google Sheets. +""" +import logging +from dataclasses import dataclass + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Lab configuration +# --------------------------------------------------------------------------- + + +class TeamConfigError(Exception): + """A lab's `team` section is present but malformed.""" + + +@dataclass(frozen=True) +class TeamConfig: + """Parsed `team` section of a lab config.""" + size_max: int | None = None # Max members per team, None = unlimited + count_max: int | None = None # Max teams per lab, None = unlimited + + +def is_team_lab(lab_config: dict | None) -> bool: + """ + Whether a lab is a team lab. + + The mere presence of the `team` key switches the lab over, even when the + section is empty (`team: {}`) - limits are optional. + """ + return isinstance(lab_config, dict) and "team" in lab_config + + +def _positive_int(raw: dict, key: str) -> int | None: + value = raw.get(key) + if value is None: + return None + # bool is an int subclass, and `size-max: yes` in YAML is a bool. + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + raise TeamConfigError( + f"Некорректное значение team.{key}: {value!r} (ожидается целое число не меньше 1)" + ) + return value + + +def parse_team_config(lab_config: dict) -> TeamConfig | None: + """ + Parse and validate the `team` section of a lab config. + + Returns: + TeamConfig for a team lab, or None for an individual one + + Raises: + TeamConfigError: the section is not a mapping, or a limit is not a + positive integer + """ + if not is_team_lab(lab_config): + return None + + raw = lab_config.get("team") + if raw is None: + # `team:` with nothing under it - a team lab without limits. + raw = {} + if not isinstance(raw, dict): + raise TeamConfigError("Секция team лабораторной работы должна быть словарём") + + return TeamConfig( + size_max=_positive_int(raw, "size-max"), + count_max=_positive_int(raw, "count-max"), + ) diff --git a/main.py b/main.py index 86ccfd1..d093fc0 100644 --- a/main.py +++ b/main.py @@ -49,6 +49,10 @@ get_bulk_job, request_bulk_job_cancel, run_bulk_grading, + TeamConfig, + TeamConfigError, + is_team_lab, + parse_team_config, ) # Configure logging to both file and console @@ -158,6 +162,36 @@ def load_course_index(): return index_data +def warn_about_team_labs(filename: str, course_info: dict) -> None: + """ + Log config problems of team labs that are not fatal on their own. + + A malformed `team` section is reported by /join for that lab (see + _load_lab_for_join); here it only makes it into the startup log, together + with `taskid-max`, which a team lab ignores - the variant number is + derived from the student's position in the sheet and a team has none + (docs/TEAM_ASSIGNMENTS_PLAN.md §4, §10.3). + """ + labs = course_info.get("labs", {}) + if not isinstance(labs, dict): + return + + for lab_key, lab_config in labs.items(): + if not is_team_lab(lab_config): + continue + + try: + parse_team_config(lab_config) + except TeamConfigError as e: + logger.warning(f"{filename}: лаба '{lab_key}' - {e}") + + if lab_config.get("taskid-max") is not None: + logger.warning( + f"{filename}: лаба '{lab_key}' - командная, поэтому taskid-max игнорируется " + "(проверка варианта для командных лаб не выполняется)" + ) + + def validate_course_index(): """Validate that index.yaml is synchronized with course files""" try: @@ -204,6 +238,7 @@ def validate_course_index(): if not isinstance(data, dict) or "course" not in data: print(f"❌ ERROR: Invalid course structure in {entry['file']}") return False + warn_about_team_labs(entry["file"], data["course"]) except Exception as e: print(f"❌ ERROR: Failed to load {entry['file']}: {e}") return False @@ -856,17 +891,20 @@ def load_sheet_context() -> SheetContext: REPO_PROVISIONING_MODES = {"template", "fork"} -def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str]: +def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str, TeamConfig | None]: """ Load course/lab config needed by the /join flow. Returns: - (course_info, lab_config, github_organization) + (course_info, lab_config, github_organization, team_config). The last + element is None for an individual lab and a TeamConfig for a team one + (docs/TEAM_ASSIGNMENTS_PLAN.md §4). Raises: HTTPException: 404 for unknown course/lab, 400 if the lab has no `template-repo` configured, has an unrecognized `repo-provisioning` - value, or the course has no GitHub organization. + value, has a malformed `team` section, or the course has no GitHub + organization. """ course_info = get_course_by_id(course_id) # raises 404 if course unknown @@ -893,11 +931,18 @@ def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str]: ), ) + try: + team_config = parse_team_config(lab_config) + except TeamConfigError as e: + # Same treatment as an unknown repo-provisioning value: a config + # mistake answers with a clear 400, never a 500. + raise HTTPException(status_code=400, detail=f"Некорректная настройка команд: {e}") + org = course_info.get("github", {}).get("organization") if not org: raise HTTPException(status_code=400, detail="Для курса не настроена GitHub организация") - return course_info, lab_config, org + return course_info, lab_config, org, team_config def _oauth_redirect_uri(request: Request) -> str: @@ -1016,12 +1061,20 @@ def _exchange_code_for_username(code: str, redirect_uri: str) -> str | None: @limiter.limit("30/minute") def join_lab_info(request: Request, course_id: str, lab_id: str): """Публичная информация для лендинга страницы присоединения к лабе (без аутентификации).""" - course_info, lab_config, _org = _load_lab_for_join(course_id, lab_id) + course_info, lab_config, _org, team_config = _load_lab_for_join(course_id, lab_id) return { "course_id": course_id, "lab_id": lab_id, "course_name": course_info.get("name", "Unknown"), "lab_short_name": lab_config.get("short-name", lab_id), + # Rosters are deliberately absent - this endpoint is public. Only the + # fact that the lab is a team one, and its limits. + "team": { + "enabled": team_config is not None, + "size_max": team_config.size_max if team_config else None, + "count_max": team_config.count_max if team_config else None, + "teams_count": None, + }, } @@ -1076,7 +1129,7 @@ def join_callback( return RedirectResponse(url=_join_result_redirect(course_id, lab_id, "error", reason="missing_code")) try: - course_info, lab_config, org = _load_lab_for_join(course_id, lab_id) + course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) except HTTPException: return RedirectResponse(url=_join_result_redirect(course_id, lab_id, "error", reason="config")) diff --git a/tests/test_bulk_grading.py b/tests/test_bulk_grading.py index dc5edf6..0e6d782 100644 --- a/tests/test_bulk_grading.py +++ b/tests/test_bulk_grading.py @@ -256,6 +256,11 @@ def test_no_taskid_max_in_lab(self): def test_ignore_task_id(self): assert taskid_column(self.COURSE, {"taskid-max": 20, "ignore-task-id": True}) is None + def test_team_lab_never_checks_taskid(self): + """A team has no position in the sheet to derive a variant from.""" + assert taskid_column(self.COURSE, {"taskid-max": 20, "team": {}}) is None + assert taskid_column(self.COURSE, {"taskid-max": 20, "team": None}) is None + class TestRepoNameFor: def test_builds_conventional_name(self): diff --git a/tests/test_join_endpoints.py b/tests/test_join_endpoints.py index b47a547..41cc917 100644 --- a/tests/test_join_endpoints.py +++ b/tests/test_join_endpoints.py @@ -92,6 +92,36 @@ def test_fork_repo_provisioning_value_is_accepted(self, mock_request, join_cours data = main_module.join_lab_info(mock_request, "test-course", "1") assert data["course_name"] == "Test Course" + def test_individual_lab_reports_teams_disabled(self, mock_request, mock_get_course_by_id): + data = main_module.join_lab_info(mock_request, "test-course", "1") + assert data["team"]["enabled"] is False + assert data["team"]["size_max"] is None + assert data["team"]["count_max"] is None + + def test_team_lab_reports_its_limits(self, mock_request, join_course_config): + join_course_config["labs"]["1"]["team"] = {"size-max": 4, "count-max": 8} + with patch("main.get_course_by_id", return_value=join_course_config): + data = main_module.join_lab_info(mock_request, "test-course", "1") + assert data["team"]["enabled"] is True + assert data["team"]["size_max"] == 4 + assert data["team"]["count_max"] == 8 + + def test_invalid_team_limit_returns_400(self, mock_request, join_course_config): + """A bad limit must be a clear config error, not a 500 (stage 1 checklist).""" + join_course_config["labs"]["1"]["team"] = {"size-max": 0} + with patch("main.get_course_by_id", return_value=join_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_info(mock_request, "test-course", "1") + assert exc_info.value.status_code == 400 + assert "size-max" in exc_info.value.detail + + def test_team_limit_of_wrong_type_returns_400(self, mock_request, join_course_config): + join_course_config["labs"]["1"]["team"] = {"count-max": "восемь"} + with patch("main.get_course_by_id", return_value=join_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_info(mock_request, "test-course", "1") + assert exc_info.value.status_code == 400 + class TestJoinStart: def test_redirects_to_github_authorize_with_signed_state(self, mock_request, mock_get_course_by_id): diff --git a/tests/test_lab_resolution.py b/tests/test_lab_resolution.py index 7ad65bc..7057a44 100644 --- a/tests/test_lab_resolution.py +++ b/tests/test_lab_resolution.py @@ -155,11 +155,11 @@ def test_join_info_returns_the_lab_addressed_by_key(self, monkeypatch): } monkeypatch.setattr(main_module, "get_course_by_id", lambda _cid: course) - _course, lab_config, _org = main_module._load_lab_for_join("os", "01") + _course, lab_config, _org, _team = main_module._load_lab_for_join("os", "01") assert lab_config["short-name"] == "ЛР0.1" assert lab_config["template-repo"] == "org/t01" - _course, lab_config, _org = main_module._load_lab_for_join("os", "1") + _course, lab_config, _org, _team = main_module._load_lab_for_join("os", "1") assert lab_config["short-name"] == "ЛР1" def test_unknown_lab_still_404(self, monkeypatch): diff --git a/tests/test_teams.py b/tests/test_teams.py new file mode 100644 index 0000000..ecd067a --- /dev/null +++ b/tests/test_teams.py @@ -0,0 +1,85 @@ +""" +Tests for team (group) lab assignments (grading/teams.py). + +GitHub API calls are mocked the same way tests/test_repo_provisioning.py does +it. See docs/TEAM_ASSIGNMENTS_PLAN.md §15 for the list this file covers. +""" +import os +import sys + +import pytest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from grading.teams import ( + TeamConfig, + TeamConfigError, + is_team_lab, + parse_team_config, +) + + +class TestIsTeamLab: + """The `team` section is what makes a lab a team lab.""" + + def test_lab_without_section(self): + assert is_team_lab({"github-prefix": "os-task1"}) is False + + def test_empty_section_still_counts(self): + assert is_team_lab({"team": {}}) is True + + def test_null_section_still_counts(self): + """`team:` with nothing under it parses as None in YAML.""" + assert is_team_lab({"team": None}) is True + + def test_section_with_limits(self): + assert is_team_lab({"team": {"size-max": 4}}) is True + + def test_not_a_dict(self): + assert is_team_lab(None) is False + + +class TestParseTeamConfig: + """Validation of the `team` section (§4 of the plan).""" + + def test_individual_lab_yields_none(self): + assert parse_team_config({"github-prefix": "os-task1"}) is None + + def test_empty_section_has_no_limits(self): + assert parse_team_config({"team": {}}) == TeamConfig(size_max=None, count_max=None) + + def test_null_section_has_no_limits(self): + assert parse_team_config({"team": None}) == TeamConfig() + + def test_both_limits(self): + assert parse_team_config({"team": {"size-max": 4, "count-max": 8}}) == TeamConfig(4, 8) + + def test_one_limit_only(self): + assert parse_team_config({"team": {"size-max": 3}}) == TeamConfig(size_max=3) + + def test_zero_is_rejected(self): + with pytest.raises(TeamConfigError) as exc: + parse_team_config({"team": {"size-max": 0}}) + assert "size-max" in str(exc.value) + + def test_negative_is_rejected(self): + with pytest.raises(TeamConfigError): + parse_team_config({"team": {"count-max": -1}}) + + def test_string_is_rejected(self): + with pytest.raises(TeamConfigError) as exc: + parse_team_config({"team": {"count-max": "восемь"}}) + assert "count-max" in str(exc.value) + + def test_bool_is_rejected(self): + """`size-max: yes` is a bool in YAML, and bool is an int subclass.""" + with pytest.raises(TeamConfigError): + parse_team_config({"team": {"size-max": True}}) + + def test_float_is_rejected(self): + with pytest.raises(TeamConfigError): + parse_team_config({"team": {"size-max": 2.5}}) + + def test_section_of_wrong_type_is_rejected(self): + with pytest.raises(TeamConfigError): + parse_team_config({"team": [1, 2]}) From adb3d540a5b44520f2874edfe7e4b93d9f736772 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 19:52:48 +0300 Subject: [PATCH 03/10] =?UTF-8?q?=D0=97=D0=B0=D0=B2=D0=B5=D1=81=D1=82?= =?UTF-8?q?=D0=B8=20=D1=81=D0=B5=D1=81=D1=81=D0=B8=D1=8E=20=D1=81=D1=82?= =?UTF-8?q?=D1=83=D0=B4=D0=B5=D0=BD=D1=82=D0=B0=20=D0=B4=D0=BB=D1=8F=20?= =?UTF-8?q?=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=D0=BD=D1=8B=D1=85=20=D0=BB?= =?UTF-8?q?=D0=B0=D0=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Колбэк OAuth для командной лабы больше не создаёт репозиторий: он выставляет подписанную cookie join_session (HttpOnly, SameSite=Lax, path=/join, 30 минут) и возвращает студента на страницу лабы со статусом authenticated - выбор команды впереди. require_join_session берёт GitHub-логин только из этой cookie и сверяет курс и лабу из неё с путём запроса. Из тела запроса, query-параметров и пути логин не принимается никогда: иначе доступ к приватному репозиторию команды выдавался бы по чужому логину. Co-Authored-By: Claude Opus 5 --- main.py | 98 ++++++++++++++++++++++++++++++++++++ tests/test_join_endpoints.py | 97 +++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) diff --git a/main.py b/main.py index d093fc0..7825c9b 100644 --- a/main.py +++ b/main.py @@ -119,6 +119,11 @@ FRONTEND_URL = os.getenv("FRONTEND_URL", "http://localhost:8080") # Max age (seconds) for the signed OAuth `state` param - see docs/REPO_GENERATION_PLAN.md §3.3. JOIN_STATE_MAX_AGE = 600 +# Student session issued by /join/callback for team labs, so that the team +# endpoints can take the confirmed username from a signed cookie and never +# from the request - see docs/TEAM_ASSIGNMENTS_PLAN.md §6. +JOIN_SESSION_COOKIE = "join_session" +JOIN_SESSION_MAX_AGE = 1800 # Rate limiting configuration limiter = Limiter(key_func=get_remote_address) @@ -984,6 +989,89 @@ def _parse_join_state(state: str | None) -> dict: return payload +def _build_join_session(username: str, course_id: str, lab_id: str) -> str: + """ + Build the signed value of the `join_session` cookie. + + An individual lab finishes inside the OAuth callback, but a team lab needs + a dialogue (show the teams, wait for the choice), so the confirmed + username is carried in a short-lived session instead + (docs/TEAM_ASSIGNMENTS_PLAN.md §6). Same signer and encoding as + _build_join_state. + """ + payload = json.dumps({ + "username": username, + "course_id": course_id, + "lab_id": lab_id, + }).encode("utf-8") + payload_b64 = base64.urlsafe_b64encode(payload).decode("ascii") + return signer.sign(payload_b64.encode("ascii")).decode("ascii") + + +def _parse_join_session(cookie: str | None) -> dict | None: + """ + Verify and decode a `join_session` cookie. + + Returns: + The payload, or None if the cookie is missing, forged or expired + """ + if not cookie: + return None + try: + payload_b64 = signer.unsign(cookie, max_age=JOIN_SESSION_MAX_AGE).decode("ascii") + payload = json.loads(base64.urlsafe_b64decode(payload_b64.encode("ascii"))) + except (BadSignature, ValueError, TypeError, KeyError): + return None + + if not isinstance(payload, dict) or not payload.get("username"): + return None + return payload + + +def _set_join_session_cookie(response: Response, username: str, course_id: str, lab_id: str) -> None: + """Attach the `join_session` cookie to a response (§6 of the plan).""" + response.set_cookie( + key=JOIN_SESSION_COOKIE, + value=_build_join_session(username, course_id, lab_id), + httponly=True, + samesite="lax", + max_age=JOIN_SESSION_MAX_AGE, + path="/join", + secure=False, + ) + + +def require_join_session(request: Request, course_id: str, lab_id: str) -> str: + """ + The confirmed GitHub username of the student behind a team request. + + The username comes from this cookie and from nowhere else - never from the + request body, a query parameter or the path. That is the same requirement + as §3.2 of docs/REPO_GENERATION_PLAN.md: an identity is only ever + established by the server-side `code -> access_token -> GET /user` + exchange. Accepting a username from the request would hand out access to a + private repository under someone else's login. + + The course and lab in the cookie must match the ones in the path, so a + session obtained for one lab cannot act on another. + + Raises: + HTTPException(401): with the stable code SESSION_REQUIRED + """ + payload = _parse_join_session(request.cookies.get(JOIN_SESSION_COOKIE)) + if payload is None: + raise HTTPException(status_code=401, detail="SESSION_REQUIRED") + + if payload.get("course_id") != course_id or payload.get("lab_id") != lab_id: + logger.warning( + "join_session for %s/%s presented for %s/%s", + payload.get("course_id"), payload.get("lab_id"), course_id, lab_id, + ) + raise HTTPException(status_code=401, detail="SESSION_REQUIRED") + + return payload["username"] + + def _join_result_redirect(course_id: str, lab_id: str, status: str, **extra) -> str: """Build the frontend result URL (/join/:courseId/:labId) the student's browser lands on.""" params = {"status": status, **{k: v for k, v in extra.items() if v is not None}} @@ -1143,6 +1231,16 @@ def join_callback( logger.info(f"Confirmed GitHub username '{username}' for join {course_id}/{lab_id}") + if team_config is not None: + # A team lab creates nothing here: the student still has to pick or + # create a team. The confirmed username is carried onward in the + # signed join_session cookie (§8.1 of the team plan). + response = RedirectResponse( + url=_join_result_redirect(course_id, lab_id, "authenticated", username=username) + ) + _set_join_session_cookie(response, username, course_id, lab_id) + return response + github_prefix = lab_config.get("github-prefix") template_repo = lab_config.get("template-repo") repo_provisioning = lab_config.get("repo-provisioning", "template") diff --git a/tests/test_join_endpoints.py b/tests/test_join_endpoints.py index 41cc917..28eca6b 100644 --- a/tests/test_join_endpoints.py +++ b/tests/test_join_endpoints.py @@ -376,6 +376,45 @@ def test_repeat_visit_does_not_recreate_existing_repo(self, mock_request, mock_g assert qs(resp.headers["location"])["status"] == ["success"] assert generate_call.call_count == 0 + @responses.activate + def test_team_lab_sets_a_session_and_creates_nothing(self, mock_request, join_course_config): + """A team lab needs a dialogue, so the callback only authenticates + the student - the repository is created later, by the team endpoints + (docs/TEAM_ASSIGNMENTS_PLAN.md §8.1).""" + join_course_config["labs"]["1"]["team"] = {"size-max": 4} + with patch("main.get_course_by_id", return_value=join_course_config): + state = _get_state(mock_request) + + responses.add( + responses.POST, + "https://github.com/login/oauth/access_token", + json={"access_token": "gho_student_token"}, + status=200, + ) + responses.add( + responses.GET, "https://api.github.com/user", + json={"login": "student1"}, status=200, + ) + generate_call = responses.add( + responses.POST, + "https://api.github.com/repos/test-org/os-task1-template/generate", + json={}, status=201, + ) + + resp = main_module.join_callback(mock_request, code="abc", state=state, error=None) + + params = qs(resp.headers["location"]) + assert params["status"] == ["authenticated"] + assert generate_call.call_count == 0 + + cookie = resp.headers["set-cookie"] + assert "join_session=" in cookie + assert "HttpOnly" in cookie + assert "SameSite=lax" in cookie.replace("samesite", "SameSite") + assert "Path=/join" in cookie + assert f"Max-Age={main_module.JOIN_SESSION_MAX_AGE}" in cookie + assert main_module.JOIN_SESSION_MAX_AGE == 1800 + @responses.activate def test_student_access_token_is_never_exposed_in_redirect(self, mock_request, mock_get_course_by_id): """The student's one-shot OAuth access token must never leak into the final redirect.""" @@ -400,3 +439,61 @@ def test_student_access_token_is_never_exposed_in_redirect(self, mock_request, m resp = main_module.join_callback(mock_request, code="abc", state=state, error=None) assert "gho_super_secret_token" not in resp.headers["location"] + + +class TestJoinSession: + """The signed cookie carrying the confirmed username (§6 of the team plan).""" + + def _request_with_cookie(self, cookie_value): + from starlette.requests import Request + + headers = [] + if cookie_value is not None: + headers.append((b"cookie", f"join_session={cookie_value}".encode())) + scope = { + "type": "http", "method": "GET", "path": "/join", + "headers": headers, "client": ("127.0.0.1", 12345), + } + return Request(scope, lambda: None) + + def test_round_trip(self): + cookie = main_module._build_join_session("student1", "test-course", "1") + request = self._request_with_cookie(cookie) + + assert main_module.require_join_session(request, "test-course", "1") == "student1" + + def test_missing_cookie_is_401(self): + with pytest.raises(HTTPException) as exc_info: + main_module.require_join_session(self._request_with_cookie(None), "test-course", "1") + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "SESSION_REQUIRED" + + def test_forged_cookie_is_401(self): + request = self._request_with_cookie("not-a-signed-value") + with pytest.raises(HTTPException) as exc_info: + main_module.require_join_session(request, "test-course", "1") + assert exc_info.value.status_code == 401 + + def test_expired_cookie_is_401(self): + backdated = time.time() - (main_module.JOIN_SESSION_MAX_AGE + 10) + with patch("itsdangerous.timed.time.time", return_value=backdated): + cookie = main_module._build_join_session("student1", "test-course", "1") + + with pytest.raises(HTTPException) as exc_info: + main_module.require_join_session(self._request_with_cookie(cookie), "test-course", "1") + assert exc_info.value.status_code == 401 + + def test_cookie_of_another_lab_is_rejected(self): + """A session obtained for one lab must not act on another.""" + cookie = main_module._build_join_session("student1", "test-course", "2") + + with pytest.raises(HTTPException) as exc_info: + main_module.require_join_session(self._request_with_cookie(cookie), "test-course", "1") + assert exc_info.value.status_code == 401 + + def test_cookie_of_another_course_is_rejected(self): + cookie = main_module._build_join_session("student1", "other-course", "1") + + with pytest.raises(HTTPException) as exc_info: + main_module.require_join_session(self._request_with_cookie(cookie), "test-course", "1") + assert exc_info.value.status_code == 401 From 8074f180eea04eabb030a13a303d59e47e050572 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 19:56:29 +0300 Subject: [PATCH 04/10] =?UTF-8?q?=D0=9F=D0=BE=D0=BA=D0=B0=D0=B7=D0=B0?= =?UTF-8?q?=D1=82=D1=8C=20=D1=81=D1=82=D1=83=D0=B4=D0=B5=D0=BD=D1=82=D1=83?= =?UTF-8?q?=20=D1=81=D0=BF=D0=B8=D1=81=D0=BE=D0=BA=20=D0=BA=D0=BE=D0=BC?= =?UTF-8?q?=D0=B0=D0=BD=D0=B4=20=D0=BB=D0=B0=D0=B1=D0=BE=D1=80=D0=B0=D1=82?= =?UTF-8?q?=D0=BE=D1=80=D0=BD=D0=BE=D0=B9=20=D1=80=D0=B0=D0=B1=D0=BE=D1=82?= =?UTF-8?q?=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Команда - это репозиторий {github-prefix}-team-{N}: имя даёт slug, поле description - название и описание, прямые коллабораторы и непринятые приглашения - состав. Отдельного хранилища не появляется. Участником считается коллаборатор с правом push и без admin - фильтр отсекает владельцев организации, попадающих в список по организационной роли; логины из github.teachers исключаются дополнительно. Недоступный состав одной команды помечается флагом и не ломает весь экран. Список кэшируется на 30 секунд: группа в 30 человек, одновременно открывшая страницу, тратит один набор запросов вместо тридцати. Ссылка на репозиторий отдаётся только для своей команды. Co-Authored-By: Claude Opus 5 --- .DS_Store | Bin 0 -> 6148 bytes grading/__init__.py | 12 ++ grading/github_client.py | 33 +++- grading/teams.py | 278 ++++++++++++++++++++++++++++++- main.py | 149 ++++++++++++++++- tests/test_github_client.py | 43 +++++ tests/test_join_endpoints.py | 200 +++++++++++++++++++++++ tests/test_teams.py | 307 +++++++++++++++++++++++++++++++++++ 8 files changed, 1016 insertions(+), 6 deletions(-) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..68aae905f9dd62de78f5fb2e2a9c162007003972 GIT binary patch literal 6148 zcmeH~JqiLr422WjLa^D=avBfd4F=H@cmdJHO4vf|=jgutAh=qK$O|OjBr{>zSL|#= zM7Q^0Bhrh=0&bMGg^4NhP6ip}EVs*WJD literal 0 HcmV?d00001 diff --git a/grading/__init__.py b/grading/__init__.py index edb6028..1f319cf 100644 --- a/grading/__init__.py +++ b/grading/__init__.py @@ -80,8 +80,14 @@ from .teams import ( TeamConfig, TeamConfigError, + TeamInfo, + TeamRegistry, + TEAMS_CACHE_TTL_SECONDS, + TEAM_SLUG_RE, is_team_lab, parse_team_config, + parse_description, + reset_teams_state, ) from .propagate import ( @@ -177,8 +183,14 @@ # teams "TeamConfig", "TeamConfigError", + "TeamInfo", + "TeamRegistry", + "TEAMS_CACHE_TTL_SECONDS", + "TEAM_SLUG_RE", "is_team_lab", "parse_team_config", + "parse_description", + "reset_teams_state", # propagate "PropagateJob", "PropagateResult", diff --git a/grading/github_client.py b/grading/github_client.py index 56bfd1c..f6b41ad 100644 --- a/grading/github_client.py +++ b/grading/github_client.py @@ -572,9 +572,10 @@ def is_direct_collaborator(self, org: str, repo: str, username: str) -> bool: Note: GitHub's docs don't document an `affiliation` param for this single-user "check collaborator" endpoint (only for the list-collaborators one) - it's used here anyway per docs/REPO_GENERATION_PLAN.md §4, which - specifies this exact call. It's harmless for the current one-student-one-repo - model; a future team-lab variant relying on "direct only" here should - double check GitHub's actual behavior first. + specifies this exact call. Team labs deliberately do NOT count a roster + with it: list_collaborators below takes the documented `affiliation` + param, and this one stays what it always was - a quick "does this user + already have access" check before issuing an invitation. Args: org: Organization or user name @@ -590,6 +591,32 @@ def is_direct_collaborator(self, org: str, repo: str, username: str) -> bool: ) return resp.status_code == 204 + def list_collaborators( + self, + org: str, + repo: str, + affiliation: str = "direct", + ) -> list[dict[str, Any]] | None: + """ + List a repository's collaborators (all pages). + + See https://docs.github.com/en/rest/collaborators/collaborators + Used to read a team's roster: `affiliation` is documented for this + endpoint (unlike the single-user check above), and each entry carries + a `permissions` object, which is what separates students (push) from + organization owners (admin). + + Args: + org: Organization or user name + repo: Repository name + affiliation: "direct" (default), "outside" or "all" + + Returns: + List of collaborator dicts, or None on error + """ + url = f"{self.BASE_URL}/repos/{org}/{repo}/collaborators" + return self._get_all_pages(url, params={"affiliation": affiliation}) + def list_invitations(self, org: str, repo: str) -> list[dict[str, Any]] | None: """ List pending repository invitations. diff --git a/grading/teams.py b/grading/teams.py index 2560a6a..f1f786a 100644 --- a/grading/teams.py +++ b/grading/teams.py @@ -17,10 +17,30 @@ Google Sheets. """ import logging -from dataclasses import dataclass +import re +import threading +import time +from dataclasses import dataclass, field + +from .github_client import GitHubClient +from .repo_provisioning import RepoProvisioner logger = logging.getLogger(__name__) +# Slug of a team, as it appears in URLs and after the lab's github-prefix. +# Never built from student input: the number comes from the repositories that +# already exist (see next_team_number). +TEAM_SLUG_RE = re.compile(r"^team-\d+$") + +# How long a collected team list stays usable for read-only operations. A +# group of 30 students opening the picker page at once then costs one set of +# GitHub requests instead of thirty. +TEAMS_CACHE_TTL_SECONDS = 30 + +# Separates the title from the description inside the repository description. +# Space, em dash (U+2014), space. +DESCRIPTION_SEPARATOR = " — " + # --------------------------------------------------------------------------- # Lab configuration @@ -85,3 +105,259 @@ def parse_team_config(lab_config: dict) -> TeamConfig | None: size_max=_positive_int(raw, "size-max"), count_max=_positive_int(raw, "count-max"), ) + + +# --------------------------------------------------------------------------- +# Team title and description +# --------------------------------------------------------------------------- + +def parse_description(raw: str | None) -> tuple[str, str]: + """ + Split a repository description back into (title, description). + + A description edited by hand on GitHub may have no separator at all - the + whole string is then shown as the title, degrading without an error. + """ + text = (raw or "").strip() + if not text: + return "", "" + title, separator, description = text.partition(DESCRIPTION_SEPARATOR) + if not separator: + return text, "" + return title.strip(), description.strip() + + +# --------------------------------------------------------------------------- +# Teams +# --------------------------------------------------------------------------- + + + +@dataclass +class TeamInfo: + """One team of a lab, as read from its repository.""" + slug: str # "team-3" + number: int # 3 + repo_name: str # "os-task5-team-3" + repo_url: str + title: str = "" + description: str = "" + members: list[str] = field(default_factory=list) # accepted collaborators + pending: list[str] = field(default_factory=list) # invited, not accepted + members_unknown: bool = False # roster could not be read (see §7.1) + + @property + def size(self) -> int: + """Members occupying a place: a pending invitation holds one too.""" + return len(self.members) + len(self.pending) + + def has_member(self, username: str) -> bool: + target = (username or "").casefold() + return any( + login.casefold() == target for login in (*self.members, *self.pending) + ) + + +# Module-level, exactly like the job stores in propagate.py / bulk.py: a +# TeamRegistry is built per request, so shared state cannot live on the +# instance. Correct only while the backend runs a single uvicorn worker - +# a constraint docs/PROJECT_DESCRIPTION.md already states. +_teams_cache: dict[tuple[str, str], tuple[float, list[TeamInfo]]] = {} +_cache_lock = threading.Lock() + +_lab_locks: dict[tuple[str, str], threading.Lock] = {} +_lab_locks_mutex = threading.Lock() + + +def lab_lock(course_id: str, lab_id: str) -> threading.Lock: + """The mutation lock of one lab, created on first use.""" + key = (course_id, lab_id) + with _lab_locks_mutex: + lock = _lab_locks.get(key) + if lock is None: + lock = threading.Lock() + _lab_locks[key] = lock + return lock + + +def reset_teams_state() -> None: + """Drop every cached team list and lock (used by tests).""" + with _cache_lock: + _teams_cache.clear() + with _lab_locks_mutex: + _lab_locks.clear() + + +class TeamRegistry: + """ + Reads and mutates the teams of one lab, on top of their repositories. + + Mirrors RepoProvisioner: takes a GitHubClient built with the server's + GITHUB_TOKEN, never the student's OAuth token. + """ + + def __init__(self, github_client: GitHubClient, provisioner: RepoProvisioner | None = None): + self.github = github_client + self.provisioner = provisioner or RepoProvisioner(github_client) + + # -- reading ---------------------------------------------------------- + + def cached_teams(self, org: str, github_prefix: str) -> list[TeamInfo] | None: + """Teams collected less than TEAMS_CACHE_TTL_SECONDS ago, if any.""" + with _cache_lock: + entry = _teams_cache.get((org, github_prefix)) + if entry is None: + return None + collected_at, teams = entry + if time.time() - collected_at >= TEAMS_CACHE_TTL_SECONDS: + return None + return teams + + def invalidate(self, org: str, github_prefix: str) -> None: + """Forget the cached team list after a successful mutation.""" + with _cache_lock: + _teams_cache.pop((org, github_prefix), None) + + def list_teams( + self, + org: str, + github_prefix: str, + teachers: tuple[str, ...] | list[str] = (), + fresh: bool = False, + ) -> list[TeamInfo] | None: + """ + Collect the teams of one lab from the organization's repositories. + + Costs 1 (paginated) request for the organization plus 2 per team. + + Args: + org: GitHub organization owning student repositories + github_prefix: Lab's github-prefix + teachers: `course.github.teachers` - a mixed list of names and + logins, used only to keep a teacher out of a team roster + fresh: Bypass the cache (mandatory inside a mutation, §7.2) + + Returns: + Teams ordered by number, or None if the organization's repository + list is unavailable + """ + if not fresh: + cached = self.cached_teams(org, github_prefix) + if cached is not None: + return cached + + repos = self.github.list_org_repos(org) + if repos is None: + logger.error(f"Could not list repositories of {org} to collect teams") + return None + + pattern = re.compile(rf"^{re.escape(github_prefix)}-(team-(\d+))$") + excluded = {str(name).casefold() for name in (teachers or ())} + + teams: list[TeamInfo] = [] + for repo in repos: + name = repo.get("name", "") + match = pattern.match(name) + if not match: + continue + + title, description = parse_description(repo.get("description")) + members, pending, unknown = self._read_roster(org, name, excluded) + teams.append(TeamInfo( + slug=match.group(1), + number=int(match.group(2)), + repo_name=name, + repo_url=f"https://github.com/{org}/{name}", + title=title, + description=description, + members=members, + pending=pending, + members_unknown=unknown, + )) + + teams.sort(key=lambda team: (team.number, team.slug)) + + with _cache_lock: + _teams_cache[(org, github_prefix)] = (time.time(), teams) + return teams + + def _read_roster( + self, + org: str, + repo_name: str, + excluded: set[str], + ) -> tuple[list[str], list[str], bool]: + """ + Read one team's roster. + + A member is a direct collaborator with push but not admin permission: + organization owners show up in the collaborator list through their + organization role, and that filter is what keeps them out. Logins from + `course.github.teachers` are excluded on top of it - that list mixes + names and logins, so it is a helper, not the main criterion. + + Returns: + (members, pending, members_unknown). A GitHub failure for one team + yields empty lists and members_unknown=True, so that a single + unreadable team does not break the whole page. + """ + collaborators = self.github.list_collaborators(org, repo_name, affiliation="direct") + invitations = self.github.list_invitations(org, repo_name) + if collaborators is None or invitations is None: + logger.warning(f"Could not read the roster of {org}/{repo_name}") + return [], [], True + + members: list[str] = [] + for collaborator in collaborators: + login = collaborator.get("login") or "" + if not login or login.casefold() in excluded: + continue + permissions = collaborator.get("permissions") or {} + if not permissions.get("push") or permissions.get("admin"): + continue + members.append(login) + + pending: list[str] = [] + for invitation in invitations: + login = (invitation.get("invitee") or {}).get("login") or "" + if not login or login.casefold() in excluded: + continue + pending.append(login) + + return members, pending, False + + @staticmethod + def find_member_team(teams: list[TeamInfo], username: str) -> TeamInfo | None: + """The team `username` belongs to, or None.""" + for team in teams: + if team.has_member(username): + return team + return None + + @staticmethod + def member_index(teams: list[TeamInfo]) -> dict[str, TeamInfo]: + """ + Map every member's casefolded login to their team. + + Pending invitees are included: the repository is the team's, and the + grade belongs in the row of everyone assigned to it. + """ + index: dict[str, TeamInfo] = {} + for team in teams: + for login in (*team.members, *team.pending): + index.setdefault(login.casefold(), team) + return index + + @staticmethod + def next_team_number(teams: list[TeamInfo]) -> int: + """ + The smallest free positive number. + + Deleting a team frees its number for the next one, which is why no + separate counter is stored. + """ + used = {team.number for team in teams} + number = 1 + while number in used: + number += 1 + return number diff --git a/main.py b/main.py index 7825c9b..8499ba4 100644 --- a/main.py +++ b/main.py @@ -51,6 +51,8 @@ run_bulk_grading, TeamConfig, TeamConfigError, + TeamInfo, + TeamRegistry, is_team_lab, parse_team_config, ) @@ -1149,7 +1151,16 @@ def _exchange_code_for_username(code: str, redirect_uri: str) -> str | None: @limiter.limit("30/minute") def join_lab_info(request: Request, course_id: str, lab_id: str): """Публичная информация для лендинга страницы присоединения к лабе (без аутентификации).""" - course_info, lab_config, _org, team_config = _load_lab_for_join(course_id, lab_id) + course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) + + teams_count = None + if team_config is not None: + # Read from the cache only, never fetching: this endpoint is public + # and unauthenticated, and the count is decorative - the authenticated + # /teams endpoint below is what actually collects the teams (§8.1). + cached = _team_registry().cached_teams(org, lab_config.get("github-prefix", "")) + teams_count = len(cached) if cached is not None else None + return { "course_id": course_id, "lab_id": lab_id, @@ -1161,7 +1172,7 @@ def join_lab_info(request: Request, course_id: str, lab_id: str): "enabled": team_config is not None, "size_max": team_config.size_max if team_config else None, "count_max": team_config.count_max if team_config else None, - "teams_count": None, + "teams_count": teams_count, }, } @@ -1266,6 +1277,140 @@ def join_callback( ) +# --------------------------------------------------------------------------- +# /join: team (group) lab assignments - one repository per team +# See docs/TEAM_ASSIGNMENTS_PLAN.md for the full design. +# --------------------------------------------------------------------------- + + +def _team_registry() -> TeamRegistry: + """TeamRegistry on the server's token - never the student's OAuth token.""" + return TeamRegistry(GitHubClient(GITHUB_TOKEN)) + + +def _course_teachers(course_info: dict) -> list[str]: + """`course.github.teachers` - a mixed list of names and GitHub logins.""" + teachers = course_info.get("github", {}).get("teachers") or [] + return [str(entry) for entry in teachers if entry] + + +def _load_team_lab(course_id: str, lab_id: str) -> tuple[dict, dict, str, TeamConfig]: + """ + Like _load_lab_for_join, but only for a lab that really is a team lab. + + Raises: + HTTPException(400): NOT_A_TEAM_LAB for an individual lab, or + LAB_NOT_CONFIGURED when the lab has no github-prefix to build team + repository names from + """ + course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) + if team_config is None: + raise HTTPException(status_code=400, detail="NOT_A_TEAM_LAB") + if not lab_config.get("github-prefix"): + raise HTTPException(status_code=400, detail="LAB_NOT_CONFIGURED") + return course_info, lab_config, org, team_config + + +# Provisioning failures that a student can retry (GitHub-side or transient) +# answer 502; the rest are configuration mistakes and answer 400. +_TEAM_GATEWAY_ERROR_CODES = { + "TEAMS_UNAVAILABLE", + "RATE_LIMITED", + "CREATE_FAILED", + "FORK_TIMEOUT", + "FORK_CHECK_FAILED", + "ACTIONS_ENABLE_FAILED", + "INVITATIONS_FETCH_FAILED", + "REINVITE_DELETE_FAILED", + "INVITE_FAILED", + "PROVISION_FAILED", +} + +# Codes with an HTTP status of their own (§8.3 of the plan). +_TEAM_ERROR_STATUS = { + "NOT_A_TEAM_LAB": 400, + "INVALID_TITLE": 400, + "LAB_NOT_CONFIGURED": 400, + "TEAM_LIMIT_REACHED": 403, + "TEAM_NOT_FOUND": 404, + "ALREADY_IN_TEAM": 409, + "TEAM_FULL": 409, + "TITLE_TAKEN": 409, + "SLUG_RACE": 409, +} + + +def _team_error_status(error_code: str | None) -> int: + if error_code in _TEAM_ERROR_STATUS: + return _TEAM_ERROR_STATUS[error_code] + return 502 if error_code in _TEAM_GATEWAY_ERROR_CODES else 400 + + +def _team_payload(team: TeamInfo, is_mine: bool, size_max: int | None) -> dict: + """ + One team as the student's picker sees it. + + `repo_url` is only filled in for the student's own team: a link to a + private repository they have no access to is useless and misleading. + Member logins are shown - they are public GitHub identifiers, and they are + how a student recognizes their groupmates' team. Full names are not: the + /join flow does not know the student's group and never opens the + spreadsheet. + """ + return { + "slug": team.slug, + "title": team.title, + "description": team.description, + "members": list(team.members), + "pending": list(team.pending), + "size": team.size, + "is_full": size_max is not None and team.size >= size_max, + "is_mine": is_mine, + "members_unknown": team.members_unknown, + "repo_url": team.repo_url if is_mine else None, + } + + +@app.get("/join/{course_id}/{lab_id}/teams") +@limiter.limit("30/minute") +def join_lab_teams(request: Request, course_id: str, lab_id: str): + """Список команд лабы, команда студента и лимиты (см. §8.2 плана).""" + course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + username = require_join_session(request, course_id, lab_id) + + registry = _team_registry() + teams = registry.list_teams( + org, lab_config["github-prefix"], _course_teachers(course_info) + ) + if teams is None: + raise HTTPException(status_code=502, detail="TEAMS_UNAVAILABLE") + + my_team = registry.find_member_team(teams, username) + + return { + "course_id": course_id, + "lab_id": lab_id, + "course_name": course_info.get("name", "Unknown"), + "lab_short_name": lab_config.get("short-name", lab_id), + "username": username, + "size_max": team_config.size_max, + "count_max": team_config.count_max, + "can_create": ( + my_team is None + and (team_config.count_max is None or len(teams) < team_config.count_max) + ), + "my_team": my_team.slug if my_team else None, + "teams": [ + _team_payload( + team, + is_mine=my_team is not None and team.slug == my_team.slug, + size_max=team_config.size_max, + ) + for team in teams + ], + } + + # --------------------------------------------------------------------------- # Admin: propagate template repository updates to student repos via fork PRs # (issue #52). Only meaningful for labs with repo-provisioning: fork - a real diff --git a/tests/test_github_client.py b/tests/test_github_client.py index 72b73f2..1506c40 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -741,3 +741,46 @@ def test_update_ref_patches_with_force(self): assert resp.status_code == 200 assert json.loads(call.calls[0].request.body) == {"sha": "abc123", "force": True} + + +class TestListCollaborators: + """Roster reading for team labs (docs/TEAM_ASSIGNMENTS_PLAN.md §9.2).""" + + @responses.activate + def test_returns_collaborators_with_permissions(self): + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/os-task5-team-1/collaborators", + json=[ + {"login": "alice", "permissions": {"push": True, "admin": False}}, + {"login": "owner", "permissions": {"push": True, "admin": True}}, + ], + status=200, + ) + client = GitHubClient("test_token") + + result = client.list_collaborators("test-org", "os-task5-team-1") + + assert [entry["login"] for entry in result] == ["alice", "owner"] + assert responses.calls[0].request.params["affiliation"] == "direct" + + @responses.activate + def test_affiliation_is_passed_through(self): + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/repo/collaborators", + json=[], + status=200, + ) + GitHubClient("test_token").list_collaborators("test-org", "repo", affiliation="all") + + assert responses.calls[0].request.params["affiliation"] == "all" + + @responses.activate + def test_error_returns_none(self): + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/repo/collaborators", + status=404, + ) + assert GitHubClient("test_token").list_collaborators("test-org", "repo") is None diff --git a/tests/test_join_endpoints.py b/tests/test_join_endpoints.py index 28eca6b..f4e3d52 100644 --- a/tests/test_join_endpoints.py +++ b/tests/test_join_endpoints.py @@ -497,3 +497,203 @@ def test_cookie_of_another_course_is_rejected(self): with pytest.raises(HTTPException) as exc_info: main_module.require_join_session(self._request_with_cookie(cookie), "test-course", "1") assert exc_info.value.status_code == 401 + + +@pytest.fixture +def team_course_config(join_course_config): + """join_course_config with lab '1' turned into a team lab.""" + join_course_config["labs"]["1"]["team"] = {"size-max": 3, "count-max": 2} + join_course_config["github"]["teachers"] = ["Mark Polyak", "teacher1"] + return join_course_config + + +@pytest.fixture(autouse=True) +def clean_teams_state(): + from grading.teams import reset_teams_state + + reset_teams_state() + yield + reset_teams_state() + + +def _session_request(username="student1", course_id="test-course", lab_id="1"): + """A Request carrying a valid join_session cookie.""" + from starlette.requests import Request + + cookie = main_module._build_join_session(username, course_id, lab_id) + scope = { + "type": "http", "method": "GET", "path": "/join", + "headers": [(b"cookie", f"join_session={cookie}".encode())], + "client": ("127.0.0.1", 12345), + } + request = Request(scope, lambda: None) + request.state.view_rate_limit = None + return request + + +def _team_repo_responses(org="test-org", prefix="test-task1"): + """Register org repos plus a roster for two teams.""" + responses.add( + responses.GET, + f"https://api.github.com/orgs/{org}/repos", + json=[ + {"name": f"{prefix}-team-1", "description": "Пингвины — учим планировщик"}, + {"name": f"{prefix}-team-2", "description": "Тюлени"}, + {"name": f"{prefix}-student9", "description": "личный репозиторий"}, + ], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{prefix}-team-1/collaborators", + json=[ + {"login": "alice", "permissions": {"push": True, "admin": False}}, + {"login": "teacher1", "permissions": {"push": True, "admin": True}}, + ], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{prefix}-team-1/invitations", + json=[{"id": 1, "invitee": {"login": "carol"}}], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{prefix}-team-2/collaborators", + json=[{"login": "student1", "permissions": {"push": True, "admin": False}}], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{prefix}-team-2/invitations", + json=[], + status=200, + ) + + +class TestJoinLabTeams: + """GET /join/{course}/{lab}/teams (§8.2 of the team plan).""" + + @responses.activate + def test_lists_teams_with_rosters(self, team_course_config): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request(), "test-course", "1") + + assert data["username"] == "student1" + assert data["size_max"] == 3 and data["count_max"] == 2 + assert [team["slug"] for team in data["teams"]] == ["team-1", "team-2"] + + first = data["teams"][0] + assert first["title"] == "Пингвины" + assert first["description"] == "учим планировщик" + # The organization owner (admin) and the teacher stay out of the roster + assert first["members"] == ["alice"] + assert first["pending"] == ["carol"] + assert first["size"] == 2 + + @responses.activate + def test_repo_url_only_for_my_own_team(self, team_course_config): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request(), "test-course", "1") + + assert data["my_team"] == "team-2" + assert data["teams"][0]["repo_url"] is None + assert data["teams"][0]["is_mine"] is False + assert data["teams"][1]["repo_url"] == "https://github.com/test-org/test-task1-team-2" + assert data["teams"][1]["is_mine"] is True + + @responses.activate + def test_member_of_a_team_cannot_create_another(self, team_course_config): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request(), "test-course", "1") + assert data["can_create"] is False + + @responses.activate + def test_count_max_closes_creation(self, team_course_config): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request("dave"), "test-course", "1") + + assert data["my_team"] is None + # count-max is 2 and two teams already exist + assert data["can_create"] is False + + @responses.activate + def test_stranger_can_create_while_below_count_max(self, team_course_config): + team_course_config["labs"]["1"]["team"]["count-max"] = 5 + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request("dave"), "test-course", "1") + assert data["can_create"] is True + + @responses.activate + def test_is_full_reflects_size_max(self, team_course_config): + team_course_config["labs"]["1"]["team"]["size-max"] = 2 + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_lab_teams(_session_request("dave"), "test-course", "1") + + assert data["teams"][0]["is_full"] is True # alice + pending carol + assert data["teams"][1]["is_full"] is False + + @responses.activate + def test_repeat_request_within_ttl_does_not_hit_github_again(self, team_course_config): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + main_module.join_lab_teams(_session_request(), "test-course", "1") + calls_after_first = len(responses.calls) + main_module.join_lab_teams(_session_request(), "test-course", "1") + + assert len(responses.calls) == calls_after_first + + @responses.activate + def test_unavailable_org_repos_return_502(self, team_course_config): + responses.add(responses.GET, "https://api.github.com/orgs/test-org/repos", status=500) + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_teams(_session_request(), "test-course", "1") + + assert exc_info.value.status_code == 502 + assert exc_info.value.detail == "TEAMS_UNAVAILABLE" + + def test_without_a_session_returns_401(self, team_course_config, mock_request): + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_teams(mock_request, "test-course", "1") + + assert exc_info.value.status_code == 401 + assert exc_info.value.detail == "SESSION_REQUIRED" + + def test_session_of_another_lab_returns_401(self, team_course_config): + request = _session_request(lab_id="2") + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_teams(request, "test-course", "1") + + assert exc_info.value.status_code == 401 + + def test_individual_lab_returns_not_a_team_lab(self, join_course_config): + with patch("main.get_course_by_id", return_value=join_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_lab_teams(_session_request(), "test-course", "1") + + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "NOT_A_TEAM_LAB" + + @responses.activate + def test_teams_count_appears_in_the_public_info_after_a_read( + self, team_course_config, mock_request + ): + _team_repo_responses() + with patch("main.get_course_by_id", return_value=team_course_config): + before = main_module.join_lab_info(mock_request, "test-course", "1") + assert before["team"]["teams_count"] is None + + main_module.join_lab_teams(_session_request(), "test-course", "1") + after = main_module.join_lab_info(mock_request, "test-course", "1") + + assert after["team"]["teams_count"] == 2 diff --git a/tests/test_teams.py b/tests/test_teams.py index ecd067a..9d7822a 100644 --- a/tests/test_teams.py +++ b/tests/test_teams.py @@ -6,16 +6,24 @@ """ import os import sys +import time +from types import SimpleNamespace +from unittest.mock import patch import pytest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from grading.teams import ( + TEAMS_CACHE_TTL_SECONDS, TeamConfig, TeamConfigError, + TeamInfo, + TeamRegistry, is_team_lab, + parse_description, parse_team_config, + reset_teams_state, ) @@ -83,3 +91,302 @@ def test_float_is_rejected(self): def test_section_of_wrong_type_is_rejected(self): with pytest.raises(TeamConfigError): parse_team_config({"team": [1, 2]}) + + +class TestParseDescription: + """Title and description are stored in the repository's description field.""" + + def test_title_and_description(self): + assert parse_description("Пингвины — учим планировщик") == ( + "Пингвины", "учим планировщик" + ) + + def test_title_only(self): + assert parse_description("Пингвины") == ("Пингвины", "") + + def test_empty(self): + assert parse_description("") == ("", "") + assert parse_description(None) == ("", "") + + def test_separator_inside_the_description_survives(self): + """Only the first separator splits; the title never contains one.""" + assert parse_description("Пингвины — первый — второй") == ( + "Пингвины", "первый — второй" + ) + + def test_hand_edited_description_without_separator(self): + """A teacher editing the field by hand must degrade, not error.""" + title, description = parse_description("что-то написанное вручную") + assert title == "что-то написанное вручную" + assert description == "" + + def test_plain_dash_is_not_the_separator(self): + assert parse_description("Кто-то - что-то") == ("Кто-то - что-то", "") + + +def _repo(name, description=None): + return {"name": name, "description": description} + + +def _collaborator(login, push=True, admin=False): + return {"login": login, "permissions": {"push": push, "admin": admin}} + + +def _invitation(login): + return {"invitee": {"login": login}} + + +class FakeGitHub: + """Minimal stand-in for GitHubClient covering the calls TeamRegistry makes.""" + + def __init__(self, repos=None, collaborators=None, invitations=None): + self.repos = repos + self.collaborators = collaborators or {} + self.invitations = invitations or {} + self.existing_repos = set() + self.updated = [] + self.org_repo_calls = 0 + self.roster_calls = 0 + + def list_org_repos(self, org): + self.org_repo_calls += 1 + return self.repos + + def list_collaborators(self, org, repo, affiliation="direct"): + self.roster_calls += 1 + return self.collaborators.get(repo, []) + + def list_invitations(self, org, repo): + return self.invitations.get(repo, []) + + def repo_exists(self, org, repo): + return repo in self.existing_repos + + def update_repo(self, org, repo, payload): + self.updated.append((repo, payload)) + return SimpleNamespace(status_code=200, text="") + + +@pytest.fixture(autouse=True) +def clean_teams_state(): + """The cache and the lab locks are module-level, like the job stores.""" + reset_teams_state() + yield + reset_teams_state() + + +class TestListTeams: + """Collecting the teams of a lab from the organization's repositories.""" + + def test_matches_only_this_labs_team_repos(self): + github = FakeGitHub(repos=[ + _repo("os-task5-team-1", "Пингвины"), + _repo("os-task5-team-2", "Тюлени — вторая команда"), + _repo("os-task5-student1"), + _repo("os-task4-team-1"), + _repo("unrelated"), + ]) + teams = TeamRegistry(github).list_teams("test-org", "os-task5") + + assert [team.slug for team in teams] == ["team-1", "team-2"] + assert teams[0].title == "Пингвины" + assert teams[1].description == "вторая команда" + assert teams[0].repo_url == "https://github.com/test-org/os-task5-team-1" + + def test_prefix_collision_with_longer_lab_number(self): + """os-task1 must not swallow os-task10, as in filter_lab_repos.""" + github = FakeGitHub(repos=[_repo("os-task1-team-1"), _repo("os-task10-team-2")]) + + assert [t.slug for t in TeamRegistry(github).list_teams("o", "os-task1")] == ["team-1"] + assert [t.slug for t in TeamRegistry(github).list_teams("o", "os-task10")] == ["team-2"] + + def test_individual_repos_are_not_teams(self): + github = FakeGitHub(repos=[_repo("os-task5-team"), _repo("os-task5-teamwork")]) + assert TeamRegistry(github).list_teams("o", "os-task5") == [] + + def test_teams_are_ordered_by_number(self): + github = FakeGitHub(repos=[ + _repo("os-task5-team-10"), _repo("os-task5-team-2"), _repo("os-task5-team-1"), + ]) + teams = TeamRegistry(github).list_teams("o", "os-task5") + assert [team.number for team in teams] == [1, 2, 10] + + def test_unavailable_org_repos_yield_none(self): + assert TeamRegistry(FakeGitHub(repos=None)).list_teams("o", "os-task5") is None + + +class TestTeamRoster: + """Who counts as a member of a team (§7.1).""" + + def test_members_and_pending(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины")], + collaborators={"os-task5-team-1": [_collaborator("alice"), _collaborator("bob")]}, + invitations={"os-task5-team-1": [_invitation("carol")]}, + ) + team = TeamRegistry(github).list_teams("o", "os-task5")[0] + + assert team.members == ["alice", "bob"] + assert team.pending == ["carol"] + assert team.size == 3 + + def test_organization_owner_is_excluded(self): + """Owners appear as collaborators through their organization role.""" + github = FakeGitHub( + repos=[_repo("os-task5-team-1")], + collaborators={"os-task5-team-1": [ + _collaborator("owner", push=True, admin=True), + _collaborator("alice"), + ]}, + ) + team = TeamRegistry(github).list_teams("o", "os-task5")[0] + assert team.members == ["alice"] + + def test_read_only_collaborator_is_excluded(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1")], + collaborators={"os-task5-team-1": [_collaborator("viewer", push=False)]}, + ) + assert TeamRegistry(github).list_teams("o", "os-task5")[0].members == [] + + def test_teachers_are_excluded_case_insensitively(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1")], + collaborators={"os-task5-team-1": [_collaborator("MarkPolyak"), _collaborator("alice")]}, + invitations={"os-task5-team-1": [_invitation("markpolyak")]}, + ) + team = TeamRegistry(github).list_teams( + "o", "os-task5", teachers=["Mark Polyak", "markpolyak"] + ) + assert team[0].members == ["alice"] + assert team[0].pending == [] + + def test_unreadable_roster_marks_only_that_team(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1"), _repo("os-task5-team-2")], + collaborators={"os-task5-team-2": [_collaborator("bob")]}, + ) + github.list_collaborators = lambda org, repo, affiliation="direct": ( + None if repo == "os-task5-team-1" else [_collaborator("bob")] + ) + teams = TeamRegistry(github).list_teams("o", "os-task5") + + assert teams[0].members_unknown is True + assert teams[0].members == [] and teams[0].size == 0 + assert teams[1].members_unknown is False + assert teams[1].members == ["bob"] + + def test_has_member_is_case_insensitive(self): + team = TeamInfo( + slug="team-1", number=1, repo_name="r", repo_url="u", + members=["Alice"], pending=["Carol"], + ) + assert team.has_member("alice") is True + assert team.has_member("carol") is True + assert team.has_member("bob") is False + + +class TestMemberLookup: + TEAMS = [ + TeamInfo(slug="team-1", number=1, repo_name="os-task5-team-1", repo_url="u1", + members=["alice"], pending=["carol"]), + TeamInfo(slug="team-2", number=2, repo_name="os-task5-team-2", repo_url="u2", + members=["bob"]), + ] + + def test_find_member_team(self): + assert TeamRegistry.find_member_team(self.TEAMS, "BOB").slug == "team-2" + + def test_find_member_team_for_a_pending_invitee(self): + assert TeamRegistry.find_member_team(self.TEAMS, "carol").slug == "team-1" + + def test_find_member_team_for_a_stranger(self): + assert TeamRegistry.find_member_team(self.TEAMS, "dave") is None + + def test_member_index_covers_members_and_pending(self): + index = TeamRegistry.member_index(self.TEAMS) + assert index["alice"].repo_name == "os-task5-team-1" + assert index["carol"].repo_name == "os-task5-team-1" + assert index["bob"].repo_name == "os-task5-team-2" + assert "dave" not in index + + +class TestNextTeamNumber: + def test_first_team(self): + assert TeamRegistry.next_team_number([]) == 1 + + def test_consecutive_numbers(self): + teams = [ + TeamInfo(slug="team-1", number=1, repo_name="r1", repo_url="u"), + TeamInfo(slug="team-2", number=2, repo_name="r2", repo_url="u"), + ] + assert TeamRegistry.next_team_number(teams) == 3 + + def test_gap_is_reused(self): + """Deleting a team frees its number for the next one created.""" + teams = [ + TeamInfo(slug="team-2", number=2, repo_name="r2", repo_url="u"), + TeamInfo(slug="team-3", number=3, repo_name="r3", repo_url="u"), + ] + assert TeamRegistry.next_team_number(teams) == 1 + + +class TestTeamsCache: + """One set of GitHub requests per lab per TTL (§7.2).""" + + def test_second_read_within_ttl_does_not_hit_github(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + registry = TeamRegistry(github) + + registry.list_teams("o", "os-task5") + registry.list_teams("o", "os-task5") + + assert github.org_repo_calls == 1 + + def test_a_second_registry_shares_the_cache(self): + """A registry is built per request, so the cache lives in the module.""" + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + TeamRegistry(github).list_teams("o", "os-task5") + TeamRegistry(github).list_teams("o", "os-task5") + + assert github.org_repo_calls == 1 + + def test_fresh_bypasses_the_cache(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + registry = TeamRegistry(github) + + registry.list_teams("o", "os-task5") + registry.list_teams("o", "os-task5", fresh=True) + + assert github.org_repo_calls == 2 + + def test_expired_entry_is_refetched(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + registry = TeamRegistry(github) + registry.list_teams("o", "os-task5") + + with patch("grading.teams.time.time", return_value=time.time() + TEAMS_CACHE_TTL_SECONDS + 1): + registry.list_teams("o", "os-task5") + + assert github.org_repo_calls == 2 + + def test_invalidate_drops_the_entry(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + registry = TeamRegistry(github) + registry.list_teams("o", "os-task5") + registry.invalidate("o", "os-task5") + + assert registry.cached_teams("o", "os-task5") is None + registry.list_teams("o", "os-task5") + assert github.org_repo_calls == 2 + + def test_other_labs_are_cached_separately(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1")]) + registry = TeamRegistry(github) + registry.list_teams("o", "os-task5") + registry.list_teams("o", "os-task6") + + assert github.org_repo_calls == 2 + + def test_cached_teams_is_empty_before_the_first_read(self): + assert TeamRegistry(FakeGitHub()).cached_teams("o", "os-task5") is None From 9bec9a5974b1758383546d381e87fee2c974980a Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 20:00:40 +0300 Subject: [PATCH 05/10] =?UTF-8?q?=D0=94=D0=B0=D1=82=D1=8C=20=D1=81=D1=82?= =?UTF-8?q?=D1=83=D0=B4=D0=B5=D0=BD=D1=82=D1=83=20=D1=81=D0=BE=D0=B7=D0=B4?= =?UTF-8?q?=D0=B0=D1=82=D1=8C=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=D1=83?= =?UTF-8?q?=20=D0=B8=20=D0=B2=D1=81=D1=82=D1=83=D0=BF=D0=B8=D1=82=D1=8C=20?= =?UTF-8?q?=D0=B2=20=D0=BD=D0=B5=D1=91?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Изменяющие операции идут под блокировкой своей лабы и внутри неё перечитывают список команд с fresh=True, минуя кэш: два студента не могут занять один номер или одно название. После успешной мутации кэш сбрасывается. Номер команды - наименьший свободный среди существующих репозиториев, поэтому отдельного счётчика не нужно, а удалённая команда освобождает свой номер. Slug из запроса проверяется по TEAM_SLUG_RE, имя репозитория собирает сервер: имя репозитория из запроса не принимается никогда. RepoProvisioner.provision получил необязательный access_username: репозиторий называется по команде, а доступ выдаётся студенту. Для индивидуальных лаб параметр не передаётся и поведение не меняется. Co-Authored-By: Claude Opus 5 --- grading/__init__.py | 12 ++ grading/repo_provisioning.py | 19 +- grading/teams.py | 279 +++++++++++++++++++++++++- main.py | 84 ++++++++ tests/test_join_endpoints.py | 252 ++++++++++++++++++++++++ tests/test_repo_provisioning.py | 76 ++++++++ tests/test_teams.py | 334 ++++++++++++++++++++++++++++++++ 7 files changed, 1049 insertions(+), 7 deletions(-) diff --git a/grading/__init__.py b/grading/__init__.py index 1f319cf..8f5d5ad 100644 --- a/grading/__init__.py +++ b/grading/__init__.py @@ -80,12 +80,18 @@ from .teams import ( TeamConfig, TeamConfigError, + TeamActionResult, + TeamActionStatus, TeamInfo, TeamRegistry, + TeamTitleError, TEAMS_CACHE_TTL_SECONDS, TEAM_SLUG_RE, is_team_lab, parse_team_config, + clean_team_title, + clean_team_description, + compose_description, parse_description, reset_teams_state, ) @@ -183,12 +189,18 @@ # teams "TeamConfig", "TeamConfigError", + "TeamActionResult", + "TeamActionStatus", "TeamInfo", "TeamRegistry", + "TeamTitleError", "TEAMS_CACHE_TTL_SECONDS", "TEAM_SLUG_RE", "is_team_lab", "parse_team_config", + "clean_team_title", + "clean_team_description", + "compose_description", "parse_description", "reset_teams_state", # propagate diff --git a/grading/repo_provisioning.py b/grading/repo_provisioning.py index e7f6f88..6931a57 100644 --- a/grading/repo_provisioning.py +++ b/grading/repo_provisioning.py @@ -60,24 +60,31 @@ def provision( template_repo: str, repo_suffix: str, mode: str = "template", + access_username: str | None = None, ) -> ProvisionResult: """ Ensure `{github_prefix}-{repo_suffix}` exists in `org` (created from `template_repo` if missing) and that the student has collaborator access to it. - `repo_suffix` is named generically (not `username`) so that a future - team-lab variant could pass a team name instead - the algorithm below - always handles exactly one suffix per call either way (see - docs/REPO_GENERATION_PLAN.md §9). + `repo_suffix` is named generically (not `username`) because a team lab + passes a team slug here instead - the algorithm below always handles + exactly one suffix per call either way (see + docs/REPO_GENERATION_PLAN.md §9). For an individual lab the suffix and + the student's username are the same value, which is why + `access_username` is optional and defaults to the suffix; a team lab + passes the two separately (docs/TEAM_ASSIGNMENTS_PLAN.md §9.1). Args: org: GitHub organization that owns student repositories github_prefix: Repo name prefix from lab config template_repo: Template repository as "owner/repo" - repo_suffix: Suffix identifying the student (their GitHub username) + repo_suffix: Suffix identifying the repository - the student's + GitHub username, or a team slug like "team-3" mode: "template" (default, current behavior - GitHub's `generate` API) or "fork" (a real fork of the template, see issue #51) + access_username: Student to grant access to. Defaults to + `repo_suffix`, preserving the individual-lab behavior. Returns: ProvisionResult describing success or the specific failure @@ -97,7 +104,7 @@ def provision( if create_error: return create_error - access_error = self._ensure_access(org, repo_name, repo_suffix) + access_error = self._ensure_access(org, repo_name, access_username or repo_suffix) if access_error: return access_error diff --git a/grading/teams.py b/grading/teams.py index f1f786a..8ebb8f7 100644 --- a/grading/teams.py +++ b/grading/teams.py @@ -21,9 +21,10 @@ import threading import time from dataclasses import dataclass, field +from enum import Enum from .github_client import GitHubClient -from .repo_provisioning import RepoProvisioner +from .repo_provisioning import RepoProvisioner, ProvisionStatus logger = logging.getLogger(__name__) @@ -37,6 +38,12 @@ # GitHub requests instead of thirty. TEAMS_CACHE_TTL_SECONDS = 30 +# Title/description limits (docs/TEAM_ASSIGNMENTS_PLAN.md §3.4). The composed +# string stays well below GitHub's 350-character limit on `description`. +TITLE_MIN_LENGTH = 3 +TITLE_MAX_LENGTH = 60 +DESCRIPTION_MAX_LENGTH = 200 + # Separates the title from the description inside the repository description. # Space, em dash (U+2014), space. DESCRIPTION_SEPARATOR = " — " @@ -111,6 +118,64 @@ def parse_team_config(lab_config: dict) -> TeamConfig | None: # Team title and description # --------------------------------------------------------------------------- +class TeamTitleError(Exception): + """Team title or description failed validation (§3.4 of the plan).""" + + +def _clean_text(value: str | None) -> str: + """Strip, drop control characters and collapse whitespace runs.""" + if not value: + return "" + # Whitespace (a newline included) survives as a separator and is collapsed + # below; every other non-printable character is dropped outright. + without_controls = "".join(ch for ch in value if ch.isspace() or ch.isprintable()) + return " ".join(without_controls.split()) + + +def clean_team_title(title: str | None) -> str: + """ + Validate and normalize a team title. + + Raises: + TeamTitleError: empty, too short, too long, or containing the + title/description separator (which would break parsing back apart) + """ + cleaned = _clean_text(title) + if len(cleaned) < TITLE_MIN_LENGTH: + raise TeamTitleError( + f"Название команды должно содержать не меньше {TITLE_MIN_LENGTH} символов" + ) + if len(cleaned) > TITLE_MAX_LENGTH: + raise TeamTitleError( + f"Название команды не должно быть длиннее {TITLE_MAX_LENGTH} символов" + ) + if DESCRIPTION_SEPARATOR in cleaned: + raise TeamTitleError("Название команды не должно содержать « — »") + return cleaned + + +def clean_team_description(description: str | None) -> str: + """ + Validate and normalize an optional team description. + + Raises: + TeamTitleError: longer than DESCRIPTION_MAX_LENGTH after cleaning + """ + cleaned = _clean_text(description) + if len(cleaned) > DESCRIPTION_MAX_LENGTH: + raise TeamTitleError( + f"Описание команды не должно быть длиннее {DESCRIPTION_MAX_LENGTH} символов" + ) + return cleaned + + +def compose_description(title: str, description: str) -> str: + """Build the repository `description` field out of title and description.""" + if description: + return f"{title}{DESCRIPTION_SEPARATOR}{description}" + return title + + def parse_description(raw: str | None) -> tuple[str, str]: """ Split a repository description back into (title, description). @@ -158,6 +223,31 @@ def has_member(self, username: str) -> bool: ) +class TeamActionStatus(Enum): + OK = "ok" + ERROR = "error" + + +@dataclass +class TeamActionResult: + """Outcome of creating or joining a team.""" + status: TeamActionStatus + team: TeamInfo | None = None + repo_url: str | None = None + message: str = "" + error_code: str | None = None + + +def _error(code: str, message: str, team: TeamInfo | None = None) -> TeamActionResult: + return TeamActionResult( + status=TeamActionStatus.ERROR, + error_code=code, + message=message, + team=team, + repo_url=team.repo_url if team else None, + ) + + # Module-level, exactly like the job stores in propagate.py / bulk.py: a # TeamRegistry is built per request, so shared state cannot live on the # instance. Correct only while the backend runs a single uvicorn worker - @@ -361,3 +451,190 @@ def next_team_number(teams: list[TeamInfo]) -> int: while number in used: number += 1 return number + + # -- mutations -------------------------------------------------------- + + def create_team( + self, + course_id: str, + lab_id: str, + org: str, + github_prefix: str, + template_repo: str, + username: str, + title: str | None, + description: str | None = None, + mode: str = "template", + teachers: tuple[str, ...] | list[str] = (), + team_config: TeamConfig | None = None, + ) -> TeamActionResult: + """ + Create a team repository and make its creator a collaborator (§7.3). + + The whole sequence runs under the lab's lock and re-reads the team + list with fresh=True inside it, so two students cannot take the same + number or the same title. + """ + config = team_config or TeamConfig() + + try: + clean_title = clean_team_title(title) + clean_description = clean_team_description(description) + except TeamTitleError as e: + return _error("INVALID_TITLE", str(e)) + + with lab_lock(course_id, lab_id): + teams = self.list_teams(org, github_prefix, teachers, fresh=True) + if teams is None: + return _error("TEAMS_UNAVAILABLE", "Не удалось получить список команд") + + existing = self.find_member_team(teams, username) + if existing is not None: + return _error( + "ALREADY_IN_TEAM", + "Вы уже состоите в команде этой лабораторной работы", + team=existing, + ) + + if config.count_max is not None and len(teams) >= config.count_max: + return _error( + "TEAM_LIMIT_REACHED", + "Достигнуто максимальное число команд для этой лабораторной работы", + ) + + if any(team.title.casefold() == clean_title.casefold() for team in teams): + return _error("TITLE_TAKEN", "Команда с таким названием уже существует") + + slug = f"team-{self.next_team_number(teams)}" + repo_name = f"{github_prefix}-{slug}" + if self.github.repo_exists(org, repo_name): + # The organization listing lagged behind reality, or the name + # belongs to an unrelated repository. Either way, retrying + # picks the next free number. + logger.warning(f"{org}/{repo_name} already exists while creating a team") + return _error("SLUG_RACE", "Не удалось занять имя репозитория, повторите попытку") + + logger.info( + f"Student {username} creates team {slug} ({clean_title!r}) in {course_id}/{lab_id}" + ) + provision = self.provisioner.provision( + org, github_prefix, template_repo, slug, + mode=mode, access_username=username, + ) + if provision.status != ProvisionStatus.OK: + self.invalidate(org, github_prefix) + return _error( + provision.error_code or "PROVISION_FAILED", + provision.message or "Не удалось создать репозиторий команды", + ) + + # `generate` does not set a description, and a fork inherits the + # template's - both need replacing with the team's name. A failure + # here is logged but does not undo a working repository. + composed = compose_description(clean_title, clean_description) + resp = self.github.update_repo(org, repo_name, {"description": composed}) + if resp.status_code != 200: + logger.error( + f"Could not set the description of {org}/{repo_name}: " + f"{resp.status_code} {resp.text[:500]}" + ) + + team = TeamInfo( + slug=slug, + number=int(slug.removeprefix("team-")), + repo_name=repo_name, + repo_url=provision.repo_url or f"https://github.com/{org}/{repo_name}", + title=clean_title, + description=clean_description, + members=[], + # The invitation has just been issued, so the creator is + # pending in the common case. The cache is dropped right + # below, so the next read reports the real roster anyway. + pending=[username], + ) + self.invalidate(org, github_prefix) + return TeamActionResult( + status=TeamActionStatus.OK, + team=team, + repo_url=team.repo_url, + message="Команда создана", + ) + + def join_team( + self, + course_id: str, + lab_id: str, + org: str, + github_prefix: str, + template_repo: str, + username: str, + slug: str, + mode: str = "template", + teachers: tuple[str, ...] | list[str] = (), + team_config: TeamConfig | None = None, + ) -> TeamActionResult: + """ + Add a student to an existing team, or repair their access to the team + they are already in (§7.4). + + The repository name is always assembled by the server from the lab's + prefix and a slug matching TEAM_SLUG_RE - a repository name is never + accepted from the request. + """ + config = team_config or TeamConfig() + + if not TEAM_SLUG_RE.match(slug or ""): + return _error("TEAM_NOT_FOUND", "Команда не найдена") + + with lab_lock(course_id, lab_id): + teams = self.list_teams(org, github_prefix, teachers, fresh=True) + if teams is None: + return _error("TEAMS_UNAVAILABLE", "Не удалось получить список команд") + + team = next((candidate for candidate in teams if candidate.slug == slug), None) + if team is None: + return _error("TEAM_NOT_FOUND", "Команда не найдена") + + current = self.find_member_team(teams, username) + if current is not None and current.slug != team.slug: + return _error( + "ALREADY_IN_TEAM", + "Вы уже состоите в другой команде этой лабораторной работы", + team=current, + ) + + already_in_this_team = current is not None + if not already_in_this_team: + if team.members_unknown: + return _error( + "TEAMS_UNAVAILABLE", + "Не удалось прочитать состав команды. Попробуйте ещё раз позже", + team=team, + ) + if config.size_max is not None and team.size >= config.size_max: + return _error("TEAM_FULL", "В команде нет свободных мест", team=team) + + logger.info( + f"Student {username} joins team {slug} in {course_id}/{lab_id} " + f"(access repair: {already_in_this_team})" + ) + # The repository already exists, so this is effectively + # _ensure_access (plus _repair_fork in fork mode). + provision = self.provisioner.provision( + org, github_prefix, template_repo, slug, + mode=mode, access_username=username, + ) + if provision.status != ProvisionStatus.OK: + return _error( + provision.error_code or "PROVISION_FAILED", + provision.message or "Не удалось предоставить доступ к репозиторию команды", + team=team, + ) + + self.invalidate(org, github_prefix) + return TeamActionResult( + status=TeamActionStatus.OK, + team=team, + repo_url=team.repo_url, + message="Доступ к репозиторию команды выдан", + ) diff --git a/main.py b/main.py index 8499ba4..59f94d8 100644 --- a/main.py +++ b/main.py @@ -49,6 +49,7 @@ get_bulk_job, request_bulk_job_cancel, run_bulk_grading, + TeamActionStatus, TeamConfig, TeamConfigError, TeamInfo, @@ -1411,6 +1412,89 @@ def join_lab_teams(request: Request, course_id: str, lab_id: str): } +class CreateTeamRequest(BaseModel): + """ + Body of POST /join/{course_id}/{lab_id}/teams. + + There is deliberately no `username` field: the student's identity comes + from the signed join_session cookie and nowhere else (§6 of the plan). + Title validation is done by hand in grading/teams.py rather than by a + pydantic validator - FastAPI would answer its own 422 with a list of + errors instead of the stable INVALID_TITLE code the frontend translates. + """ + title: str | None = None + description: str | None = None + + +def _team_action_response(result, status_code: int = 200) -> JSONResponse | dict: + """Turn a TeamActionResult into an HTTP response (§8.3).""" + if result.status == TeamActionStatus.OK: + return { + "status": "ok", + "slug": result.team.slug if result.team else None, + "repo_url": result.repo_url, + "message": result.message, + } + + payload = {"detail": result.error_code or "PROVISION_FAILED"} + if result.team is not None: + # ALREADY_IN_TEAM is actionable only if the student is told which team + # is theirs, so the slug and the link travel next to the stable code. + payload["my_team"] = result.team.slug + payload["repo_url"] = result.repo_url + return JSONResponse(status_code=_team_error_status(result.error_code), content=payload) + + +@app.post("/join/{course_id}/{lab_id}/teams") +@limiter.limit("10/minute") +def create_join_team(request: Request, course_id: str, lab_id: str, body: CreateTeamRequest): + """Создаёт команду и выдаёт доступ к её репозиторию создателю (§7.3 плана).""" + course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + username = require_join_session(request, course_id, lab_id) + + result = _team_registry().create_team( + course_id=course_id, + lab_id=lab_id, + org=org, + github_prefix=lab_config["github-prefix"], + template_repo=lab_config["template-repo"], + username=username, + title=body.title, + description=body.description, + mode=lab_config.get("repo-provisioning", "template"), + teachers=_course_teachers(course_info), + team_config=team_config, + ) + return _team_action_response(result) + + +@app.post("/join/{course_id}/{lab_id}/teams/{slug}/join") +@limiter.limit("10/minute") +def join_join_team(request: Request, course_id: str, lab_id: str, slug: str): + """ + Присоединяет студента к команде либо чинит его доступ, если он уже в ней. + + Имя репозитория собирается сервером из префикса лабы и slug'а, прошедшего + TEAM_SLUG_RE; из запроса имя репозитория не принимается никогда (§7.4). + """ + course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + username = require_join_session(request, course_id, lab_id) + + result = _team_registry().join_team( + course_id=course_id, + lab_id=lab_id, + org=org, + github_prefix=lab_config["github-prefix"], + template_repo=lab_config["template-repo"], + username=username, + slug=slug, + mode=lab_config.get("repo-provisioning", "template"), + teachers=_course_teachers(course_info), + team_config=team_config, + ) + return _team_action_response(result) + + # --------------------------------------------------------------------------- # Admin: propagate template repository updates to student repos via fork PRs # (issue #52). Only meaningful for labs with repo-provisioning: fork - a real diff --git a/tests/test_join_endpoints.py b/tests/test_join_endpoints.py index f4e3d52..7a7161b 100644 --- a/tests/test_join_endpoints.py +++ b/tests/test_join_endpoints.py @@ -8,6 +8,7 @@ See docs/REPO_GENERATION_PLAN.md §7, §10, §11 (stage 2/3/4 acceptance). """ +import json import sys import os import time @@ -697,3 +698,254 @@ def test_teams_count_appears_in_the_public_info_after_a_read( after = main_module.join_lab_info(mock_request, "test-course", "1") assert after["team"]["teams_count"] == 2 + + +def _created_team_responses(username="dave", org="test-org", prefix="test-task1", slug="team-3"): + """GitHub calls made while creating a team from a template.""" + repo = f"{prefix}-{slug}" + responses.add(responses.GET, f"https://api.github.com/repos/{org}/{repo}", status=404) + responses.add( + responses.POST, + f"https://api.github.com/repos/{org}/os-task1-template/generate", + json={}, status=201, + ) + responses.add( + responses.GET, f"https://api.github.com/repos/{org}/{repo}/collaborators/{username}", + status=404, + ) + responses.add( + responses.GET, f"https://api.github.com/repos/{org}/{repo}/invitations", + json=[], status=200, + ) + responses.add( + responses.PUT, f"https://api.github.com/repos/{org}/{repo}/collaborators/{username}", + status=201, + ) + responses.add(responses.PATCH, f"https://api.github.com/repos/{org}/{repo}", status=200) + + +class TestCreateJoinTeam: + """POST /join/{course}/{lab}/teams.""" + + @responses.activate + def test_creates_a_team_and_returns_its_repository(self, team_course_config): + team_course_config["labs"]["1"]["team"]["count-max"] = 5 + _team_repo_responses() + _created_team_responses() + body = main_module.CreateTeamRequest(title="Моржи", description="третья команда") + + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.create_join_team( + _session_request("dave"), "test-course", "1", body, + ) + + assert data["status"] == "ok" + assert data["slug"] == "team-3" + assert data["repo_url"] == "https://github.com/test-org/test-task1-team-3" + + patches = [ + call for call in responses.calls + if call.request.method == "PATCH" + and call.request.url.endswith("/test-task1-team-3") + ] + assert json.loads(patches[0].request.body)["description"] == "Моржи — третья команда" + + @responses.activate + def test_invalid_title_returns_400_with_a_stable_code(self, team_course_config): + team_course_config["labs"]["1"]["team"]["count-max"] = 5 + _team_repo_responses() + body = main_module.CreateTeamRequest(title="ab") + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.create_join_team( + _session_request("dave"), "test-course", "1", body, + ) + + assert response.status_code == 400 + assert json.loads(response.body)["detail"] == "INVALID_TITLE" + + @responses.activate + def test_count_max_returns_403(self, team_course_config): + _team_repo_responses() # two teams already exist, count-max is 2 + body = main_module.CreateTeamRequest(title="Моржи") + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.create_join_team( + _session_request("dave"), "test-course", "1", body, + ) + + assert response.status_code == 403 + assert json.loads(response.body)["detail"] == "TEAM_LIMIT_REACHED" + + @responses.activate + def test_member_of_a_team_gets_409_with_their_team(self, team_course_config): + team_course_config["labs"]["1"]["team"]["count-max"] = 5 + _team_repo_responses() + body = main_module.CreateTeamRequest(title="Моржи") + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.create_join_team( + _session_request("student1"), "test-course", "1", body, + ) + + assert response.status_code == 409 + payload = json.loads(response.body) + assert payload["detail"] == "ALREADY_IN_TEAM" + assert payload["my_team"] == "team-2" + + def test_without_a_session_returns_401(self, team_course_config, mock_request): + body = main_module.CreateTeamRequest(title="Моржи") + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.create_join_team(mock_request, "test-course", "1", body) + assert exc_info.value.status_code == 401 + + def test_the_request_body_has_no_username_field(self): + """The identity comes from the cookie only, so there is nothing to forge.""" + assert "username" not in main_module.CreateTeamRequest.model_fields + body = main_module.CreateTeamRequest(title="Моржи", username="victim") + assert not hasattr(body, "username") + + @responses.activate + def test_access_is_granted_to_the_session_user_only(self, team_course_config): + """Whatever the body says, the invitation goes to the cookie's user.""" + team_course_config["labs"]["1"]["team"]["count-max"] = 5 + _team_repo_responses() + _created_team_responses(username="dave") + body = main_module.CreateTeamRequest(title="Моржи") + + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.create_join_team( + _session_request("dave"), "test-course", "1", body, + ) + + assert data["status"] == "ok" + invited = [ + call for call in responses.calls + if call.request.method == "PUT" and "/collaborators/" in call.request.url + ] + assert invited and invited[0].request.url.endswith("/collaborators/dave") + + +class TestJoinJoinTeam: + """POST /join/{course}/{lab}/teams/{slug}/join.""" + + def _access_responses(self, org="test-org", repo="test-task1-team-1", username="dave"): + responses.add(responses.GET, f"https://api.github.com/repos/{org}/{repo}", status=200) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo}/collaborators/{username}", + status=404, + ) + responses.add( + responses.GET, f"https://api.github.com/repos/{org}/{repo}/invitations", + json=[], status=200, + ) + responses.add( + responses.PUT, + f"https://api.github.com/repos/{org}/{repo}/collaborators/{username}", + status=201, + ) + + @responses.activate + def test_joins_a_team(self, team_course_config): + _team_repo_responses() + self._access_responses() + + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_join_team( + _session_request("dave"), "test-course", "1", "team-1", + ) + + assert data["status"] == "ok" + assert data["repo_url"] == "https://github.com/test-org/test-task1-team-1" + + @responses.activate + def test_full_team_returns_409(self, team_course_config): + team_course_config["labs"]["1"]["team"]["size-max"] = 2 + _team_repo_responses() + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.join_join_team( + _session_request("dave"), "test-course", "1", "team-1", + ) + + assert response.status_code == 409 + assert json.loads(response.body)["detail"] == "TEAM_FULL" + + @responses.activate + def test_unknown_slug_returns_404(self, team_course_config): + _team_repo_responses() + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.join_join_team( + _session_request("dave"), "test-course", "1", "team-9", + ) + + assert response.status_code == 404 + assert json.loads(response.body)["detail"] == "TEAM_NOT_FOUND" + + def test_slug_outside_the_pattern_never_reaches_github(self, team_course_config): + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.join_join_team( + _session_request("dave"), "test-course", "1", "../os-task1-student9", + ) + + assert response.status_code == 404 + assert json.loads(response.body)["detail"] == "TEAM_NOT_FOUND" + + @responses.activate + def test_own_team_repairs_access(self, team_course_config): + """The "restore access" button re-issues a stale invitation.""" + _team_repo_responses() + responses.add( + responses.GET, "https://api.github.com/repos/test-org/test-task1-team-2", status=200, + ) + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/test-task1-team-2/collaborators/student1", + status=404, + ) + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/test-task1-team-2/invitations", + json=[{"id": 7, "invitee": {"login": "student1"}}], status=200, + ) + responses.add( + responses.DELETE, + "https://api.github.com/repos/test-org/test-task1-team-2/invitations/7", + status=204, + ) + responses.add( + responses.PUT, + "https://api.github.com/repos/test-org/test-task1-team-2/collaborators/student1", + status=201, + ) + + with patch("main.get_course_by_id", return_value=team_course_config): + data = main_module.join_join_team( + _session_request("student1"), "test-course", "1", "team-2", + ) + + assert data["status"] == "ok" + assert any(call.request.method == "DELETE" for call in responses.calls) + + @responses.activate + def test_member_of_another_team_returns_409(self, team_course_config): + _team_repo_responses() + + with patch("main.get_course_by_id", return_value=team_course_config): + response = main_module.join_join_team( + _session_request("alice"), "test-course", "1", "team-2", + ) + + assert response.status_code == 409 + payload = json.loads(response.body) + assert payload["detail"] == "ALREADY_IN_TEAM" + assert payload["my_team"] == "team-1" + + def test_without_a_session_returns_401(self, team_course_config, mock_request): + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + main_module.join_join_team(mock_request, "test-course", "1", "team-1") + assert exc_info.value.status_code == 401 diff --git a/tests/test_repo_provisioning.py b/tests/test_repo_provisioning.py index 2c50c28..22962e1 100644 --- a/tests/test_repo_provisioning.py +++ b/tests/test_repo_provisioning.py @@ -710,3 +710,79 @@ def test_is_template_clear_failure_is_not_fatal(self): result = make_provisioner().provision(ORG, GITHUB_PREFIX, TEMPLATE_REPO, USERNAME, mode="fork") assert result.status == ProvisionStatus.OK + + +class TestAccessUsername: + """ + Team labs name the repository after the team but grant access to one + student, so the two values separate (docs/TEAM_ASSIGNMENTS_PLAN.md §9.1). + """ + + @responses.activate + def test_repo_is_named_after_the_suffix_and_access_goes_to_the_student(self): + slug = "team-3" + repo_name = f"{GITHUB_PREFIX}-{slug}" + repo_url = f"https://api.github.com/repos/{ORG}/{repo_name}" + + responses.add(responses.GET, repo_url, status=404) + responses.add( + responses.POST, + f"https://api.github.com/repos/{ORG}/os-task1-template/generate", + json={}, status=201, + ) + responses.add(responses.GET, f"{repo_url}/collaborators/{USERNAME}", status=404) + responses.add(responses.GET, f"{repo_url}/invitations", json=[], status=200) + invite = responses.add( + responses.PUT, f"{repo_url}/collaborators/{USERNAME}", status=201 + ) + + result = make_provisioner().provision( + ORG, GITHUB_PREFIX, TEMPLATE_REPO, slug, access_username=USERNAME, + ) + + assert result.status == ProvisionStatus.OK + assert result.repo_name == repo_name + assert invite.call_count == 1 + # Nothing was ever addressed to a repository named after the student + assert not any(f"{GITHUB_PREFIX}-{USERNAME}" in call.request.url for call in responses.calls) + + @responses.activate + def test_omitting_it_keeps_the_individual_lab_behaviour(self): + """The suffix is the username for an individual lab - unchanged.""" + repo_url = f"https://api.github.com/repos/{ORG}/{REPO_NAME}" + responses.add(responses.GET, repo_url, status=200) + responses.add(responses.GET, f"{repo_url}/collaborators/{USERNAME}", status=204) + + result = make_provisioner().provision(ORG, GITHUB_PREFIX, TEMPLATE_REPO, USERNAME) + + assert result.status == ProvisionStatus.OK + assert result.repo_name == REPO_NAME + + @responses.activate + def test_repairs_access_on_an_existing_team_repository(self): + """Joining an existing team is effectively _ensure_access.""" + slug = "team-1" + repo_name = f"{GITHUB_PREFIX}-{slug}" + repo_url = f"https://api.github.com/repos/{ORG}/{repo_name}" + + responses.add(responses.GET, repo_url, status=200) + generate = responses.add( + responses.POST, + f"https://api.github.com/repos/{ORG}/os-task1-template/generate", + json={}, status=201, + ) + responses.add(responses.GET, f"{repo_url}/collaborators/dave", status=404) + responses.add( + responses.GET, f"{repo_url}/invitations", + json=[{"id": 5, "invitee": {"login": "dave"}}], status=200, + ) + responses.add(responses.DELETE, f"{repo_url}/invitations/5", status=204) + responses.add(responses.PUT, f"{repo_url}/collaborators/dave", status=201) + + result = make_provisioner().provision( + ORG, GITHUB_PREFIX, TEMPLATE_REPO, slug, access_username="dave", + ) + + assert result.status == ProvisionStatus.OK + assert generate.call_count == 0 + assert any(call.request.method == "DELETE" for call in responses.calls) diff --git a/tests/test_teams.py b/tests/test_teams.py index 9d7822a..d31e1dc 100644 --- a/tests/test_teams.py +++ b/tests/test_teams.py @@ -14,12 +14,20 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from grading.repo_provisioning import ProvisionResult, ProvisionStatus from grading.teams import ( + DESCRIPTION_MAX_LENGTH, TEAMS_CACHE_TTL_SECONDS, + TITLE_MAX_LENGTH, + TeamActionStatus, TeamConfig, TeamConfigError, TeamInfo, TeamRegistry, + TeamTitleError, + clean_team_description, + clean_team_title, + compose_description, is_team_lab, parse_description, parse_team_config, @@ -390,3 +398,329 @@ def test_other_labs_are_cached_separately(self): def test_cached_teams_is_empty_before_the_first_read(self): assert TeamRegistry(FakeGitHub()).cached_teams("o", "os-task5") is None + + +class TestCleanTeamTitle: + """Title validation (§3.4). Manual, so the error code stays stable.""" + + def test_plain_title(self): + assert clean_team_title("Пингвины") == "Пингвины" + + def test_strips_and_collapses_whitespace(self): + assert clean_team_title(" Весёлые пингвины ") == "Весёлые пингвины" + + def test_newline_becomes_a_space(self): + assert clean_team_title("Пингвины\nи тюлени") == "Пингвины и тюлени" + + def test_empty_is_rejected(self): + with pytest.raises(TeamTitleError): + clean_team_title("") + with pytest.raises(TeamTitleError): + clean_team_title(" ") + with pytest.raises(TeamTitleError): + clean_team_title(None) + + def test_too_short_is_rejected(self): + with pytest.raises(TeamTitleError): + clean_team_title("ab") + + def test_too_long_is_rejected(self): + with pytest.raises(TeamTitleError): + clean_team_title("я" * (TITLE_MAX_LENGTH + 1)) + + def test_maximum_length_is_accepted(self): + assert len(clean_team_title("я" * TITLE_MAX_LENGTH)) == TITLE_MAX_LENGTH + + def test_separator_in_the_title_is_rejected(self): + """It would make the stored description unparseable.""" + with pytest.raises(TeamTitleError): + clean_team_title("Пингвины — лучшие") + + def test_control_characters_are_dropped(self): + assert clean_team_title("Пинг\x00вины") == "Пингвины" + + +class TestCleanTeamDescription: + def test_optional(self): + assert clean_team_description(None) == "" + assert clean_team_description("") == "" + + def test_collapses_whitespace(self): + assert clean_team_description(" учим\n планировщик ") == "учим планировщик" + + def test_too_long_is_rejected(self): + with pytest.raises(TeamTitleError): + clean_team_description("я" * (DESCRIPTION_MAX_LENGTH + 1)) + + def test_separator_is_allowed_in_the_description(self): + assert clean_team_description("первый — второй") == "первый — второй" + + +class TestComposeDescription: + def test_with_description(self): + assert compose_description("Пингвины", "учим планировщик") == ( + "Пингвины — учим планировщик" + ) + + def test_without_description(self): + assert compose_description("Пингвины", "") == "Пингвины" + + def test_round_trip(self): + composed = compose_description("Пингвины", "учим планировщик") + assert parse_description(composed) == ("Пингвины", "учим планировщик") + + +class FakeProvisioner: + """Records provision() calls instead of talking to GitHub.""" + + def __init__(self, result=None): + self.calls = [] + self.result = result + + def provision(self, org, github_prefix, template_repo, repo_suffix, + mode="template", access_username=None): + self.calls.append({ + "org": org, "github_prefix": github_prefix, "template_repo": template_repo, + "repo_suffix": repo_suffix, "mode": mode, "access_username": access_username, + }) + if self.result is not None: + return self.result + repo_name = f"{github_prefix}-{repo_suffix}" + return ProvisionResult( + status=ProvisionStatus.OK, + repo_name=repo_name, + repo_url=f"https://github.com/{org}/{repo_name}", + ) + + +def _registry(github, provisioner=None): + return TeamRegistry(github, provisioner or FakeProvisioner()) + + +LAB = dict(course_id="c", lab_id="5", org="test-org", + github_prefix="os-task5", template_repo="test-org/os-task5-template") + + +class TestCreateTeam: + """Creating a team (§7.3).""" + + def test_first_team_gets_team_1(self): + github = FakeGitHub(repos=[]) + provisioner = FakeProvisioner() + result = _registry(github, provisioner).create_team( + **LAB, username="alice", title="Пингвины", description="учим планировщик", + ) + + assert result.status == TeamActionStatus.OK + assert result.team.slug == "team-1" + assert result.repo_url == "https://github.com/test-org/os-task5-team-1" + assert provisioner.calls[0]["repo_suffix"] == "team-1" + assert provisioner.calls[0]["access_username"] == "alice" + + def test_next_team_gets_the_following_number(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1", "Пингвины")]) + result = _registry(github).create_team(**LAB, username="dave", title="Тюлени") + assert result.team.slug == "team-2" + + def test_title_and_description_are_written_to_the_repository(self): + github = FakeGitHub(repos=[]) + _registry(github).create_team( + **LAB, username="alice", title="Пингвины", description="учим планировщик", + ) + assert github.updated == [ + ("os-task5-team-1", {"description": "Пингвины — учим планировщик"}) + ] + + def test_description_is_just_the_title_when_empty(self): + github = FakeGitHub(repos=[]) + _registry(github).create_team(**LAB, username="alice", title="Пингвины") + assert github.updated == [("os-task5-team-1", {"description": "Пингвины"})] + + def test_fork_mode_is_passed_through(self): + provisioner = FakeProvisioner() + _registry(FakeGitHub(repos=[]), provisioner).create_team( + **LAB, username="alice", title="Пингвины", mode="fork", + ) + assert provisioner.calls[0]["mode"] == "fork" + + def test_invalid_title_creates_nothing(self): + provisioner = FakeProvisioner() + result = _registry(FakeGitHub(repos=[]), provisioner).create_team( + **LAB, username="alice", title="ab", + ) + assert result.error_code == "INVALID_TITLE" + assert provisioner.calls == [] + + def test_duplicate_title_is_refused(self): + github = FakeGitHub(repos=[_repo("os-task5-team-1", "Пингвины")]) + result = _registry(github).create_team(**LAB, username="dave", title=" пингвины ") + assert result.error_code == "TITLE_TAKEN" + + def test_count_max_blocks_creation(self): + github = FakeGitHub(repos=[ + _repo("os-task5-team-1", "Пингвины"), _repo("os-task5-team-2", "Тюлени"), + ]) + result = _registry(github).create_team( + **LAB, username="dave", title="Моржи", team_config=TeamConfig(count_max=2), + ) + assert result.error_code == "TEAM_LIMIT_REACHED" + + def test_student_already_in_a_team_cannot_create_another(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины")], + collaborators={"os-task5-team-1": [_collaborator("alice")]}, + ) + result = _registry(github).create_team(**LAB, username="ALICE", title="Моржи") + + assert result.error_code == "ALREADY_IN_TEAM" + assert result.team.slug == "team-1" + assert result.repo_url == "https://github.com/test-org/os-task5-team-1" + + def test_pending_invitation_also_blocks_creating_another_team(self): + github = FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины")], + invitations={"os-task5-team-1": [_invitation("carol")]}, + ) + result = _registry(github).create_team(**LAB, username="carol", title="Моржи") + assert result.error_code == "ALREADY_IN_TEAM" + + def test_existing_repository_name_is_a_race(self): + """The organization listing lagged behind, or the name is taken.""" + github = FakeGitHub(repos=[]) + github.existing_repos.add("os-task5-team-1") + provisioner = FakeProvisioner() + + result = _registry(github, provisioner).create_team( + **LAB, username="alice", title="Пингвины", + ) + assert result.error_code == "SLUG_RACE" + assert provisioner.calls == [] + + def test_unavailable_org_repos(self): + result = _registry(FakeGitHub(repos=None)).create_team( + **LAB, username="alice", title="Пингвины", + ) + assert result.error_code == "TEAMS_UNAVAILABLE" + + def test_provisioning_failure_is_passed_through(self): + provisioner = FakeProvisioner(result=ProvisionResult( + status=ProvisionStatus.ERROR, + message="Репозиторий-шаблон не найден", + error_code="TEMPLATE_NOT_FOUND", + )) + result = _registry(FakeGitHub(repos=[]), provisioner).create_team( + **LAB, username="alice", title="Пингвины", + ) + assert result.error_code == "TEMPLATE_NOT_FOUND" + + def test_the_cache_is_dropped_after_creation(self): + github = FakeGitHub(repos=[]) + registry = _registry(github) + registry.list_teams("test-org", "os-task5") + registry.create_team(**LAB, username="alice", title="Пингвины") + + assert registry.cached_teams("test-org", "os-task5") is None + + def test_reads_the_team_list_fresh_under_the_lock(self): + """A stale cache must not decide the number or the title check.""" + github = FakeGitHub(repos=[]) + registry = _registry(github) + registry.list_teams("test-org", "os-task5") + calls_before = github.org_repo_calls + + registry.create_team(**LAB, username="alice", title="Пингвины") + assert github.org_repo_calls == calls_before + 1 + + +class TestJoinTeam: + """Joining a team, or repairing access to one's own (§7.4).""" + + def _github(self): + return FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины"), _repo("os-task5-team-2", "Тюлени")], + collaborators={"os-task5-team-1": [_collaborator("alice")]}, + invitations={"os-task5-team-1": [_invitation("carol")]}, + ) + + def test_joins_an_existing_team(self): + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).join_team( + **LAB, username="dave", slug="team-1", + ) + + assert result.status == TeamActionStatus.OK + assert result.repo_url == "https://github.com/test-org/os-task5-team-1" + assert provisioner.calls[0]["repo_suffix"] == "team-1" + assert provisioner.calls[0]["access_username"] == "dave" + + def test_full_team_is_refused(self): + result = _registry(self._github()).join_team( + **LAB, username="dave", slug="team-1", team_config=TeamConfig(size_max=2), + ) + assert result.error_code == "TEAM_FULL" + + def test_pending_invitation_occupies_a_place(self): + """alice is a member and carol is invited - two of two places.""" + github = self._github() + result = _registry(github).join_team( + **LAB, username="dave", slug="team-1", team_config=TeamConfig(size_max=2), + ) + assert result.error_code == "TEAM_FULL" + + def test_own_team_only_repairs_access_even_when_full(self): + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).join_team( + **LAB, username="ALICE", slug="team-1", team_config=TeamConfig(size_max=2), + ) + + assert result.status == TeamActionStatus.OK + assert provisioner.calls[0]["access_username"] == "ALICE" + + def test_member_of_another_team_is_refused(self): + result = _registry(self._github()).join_team(**LAB, username="alice", slug="team-2") + + assert result.error_code == "ALREADY_IN_TEAM" + assert result.team.slug == "team-1" + + def test_unknown_slug(self): + result = _registry(self._github()).join_team(**LAB, username="dave", slug="team-9") + assert result.error_code == "TEAM_NOT_FOUND" + + def test_slug_not_matching_the_pattern_is_rejected_before_any_call(self): + """The repository name is assembled by the server, never accepted.""" + github = self._github() + provisioner = FakeProvisioner() + for bad in ("../../secret", "team-1/../x", "student1", "TEAM-1", ""): + result = _registry(github, provisioner).join_team( + **LAB, username="dave", slug=bad, + ) + assert result.error_code == "TEAM_NOT_FOUND", bad + assert provisioner.calls == [] + assert github.org_repo_calls == 0 + + def test_unreadable_roster_blocks_joining(self): + github = self._github() + github.list_collaborators = lambda org, repo, affiliation="direct": None + result = _registry(github).join_team( + **LAB, username="dave", slug="team-1", team_config=TeamConfig(size_max=4), + ) + assert result.error_code == "TEAMS_UNAVAILABLE" + + def test_provisioning_failure_is_passed_through(self): + provisioner = FakeProvisioner(result=ProvisionResult( + status=ProvisionStatus.ERROR, + message="Не удалось предоставить доступ", + error_code="INVITE_FAILED", + )) + result = _registry(self._github(), provisioner).join_team( + **LAB, username="dave", slug="team-1", + ) + assert result.error_code == "INVITE_FAILED" + + def test_the_cache_is_dropped_after_joining(self): + github = self._github() + registry = _registry(github) + registry.list_teams("test-org", "os-task5") + registry.join_team(**LAB, username="dave", slug="team-1") + + assert registry.cached_teams("test-org", "os-task5") is None From 785f6f5742ef7a3cc6c1bdc5363df1206ed78a99 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 20:04:02 +0300 Subject: [PATCH 06/10] =?UTF-8?q?=D0=9F=D1=80=D0=BE=D0=B2=D0=B5=D1=80?= =?UTF-8?q?=D1=8F=D1=82=D1=8C=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=D0=BD?= =?UTF-8?q?=D1=8B=D0=B5=20=D0=BB=D0=B0=D0=B1=D0=BE=D1=80=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D1=80=D0=BD=D1=8B=D0=B5=20=D1=80=D0=B0=D0=B1=D0=BE=D1=82=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Репозиторий у команды один, поэтому проверяется он один раз, а результат разносится по строкам участников. evaluate_student вызывается ровно один раз на команду с синтетическим SheetContext (пустая ячейка, нет порядкового номера), а защита ячейки применяется потом к каждому участнику по его собственной ячейке. Тяжёлая часть - файлы, коммиты, check-runs, логи job'ов - не повторяется на каждого студента. Одиночная проверка находит репозиторий команды по логину студента и пишет оценку только в его строку: публичный эндпоинт не пишет в чужие строки. Студент без команды получает понятное сообщение, а не «репозиторий не найден». Режим by_file для командных лаб отклоняется: в репозитории один файл с ФИО на несколько человек. Студенты без команды попадают в отчёт со статусом no_team, в отчёт добавлена колонка команды. Co-Authored-By: Claude Opus 5 --- grading/bulk.py | 224 ++++++++++++++++++++--- main.py | 35 ++++ tests/test_admin_endpoints.py | 27 +++ tests/test_bulk_grading.py | 195 ++++++++++++++++++++ tests/test_grade_lab_characterization.py | 139 ++++++++++++++ 5 files changed, 591 insertions(+), 29 deletions(-) diff --git a/grading/bulk.py b/grading/bulk.py index 27df35a..137baa0 100644 --- a/grading/bulk.py +++ b/grading/bulk.py @@ -150,6 +150,7 @@ def evaluate_student( lab_config: dict[str, Any], course_info: dict[str, Any], sheet_context: Callable[[], SheetContext], + repo_name: str | None = None, ) -> StudentOutcome: """ Grade one student's repository. @@ -171,12 +172,15 @@ def evaluate_student( 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. + repo_name: Repository to grade. Defaults to the conventional + `{github-prefix}-{username}`; a team lab passes the team's shared + repository instead (docs/TEAM_ASSIGNMENTS_PLAN.md §10.1). 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) + repo_name = repo_name or repo_name_for(lab_config, username) logger.info(f"Evaluating repository: {org}/{repo_name}") # Step 1: Repository checks (required files, workflows, commits) @@ -524,13 +528,15 @@ def resolve_github_cell(existing: str, username: str) -> tuple[bool, str | None] @dataclass class BulkResult: """Outcome for a single student, as shown in the run report.""" - status: str # updated | rejected | pending | error | conflict | unmatched | ambiguous + # updated | rejected | pending | error | conflict | unmatched | ambiguous | no_team + status: str 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 + team: str | None = None # Team title, for team labs def to_dict(self) -> dict: return { @@ -541,6 +547,7 @@ def to_dict(self) -> dict: "grade": self.grade, "message": self.message, "registered": self.registered, + "team": self.team, } @@ -683,6 +690,7 @@ class _Target: student_name: str | None repo: str registered: bool = False # Username was queued for writing into the sheet + team: str | None = None # Team title, for team labs def _plan_by_file( @@ -800,6 +808,114 @@ def _plan_by_sheet( return targets +def _plan_teams( + job: BulkJob, + github_client: GitHubClient, + org: str, + course_info: dict[str, Any], + lab_config: dict[str, Any], + targets: list[_Target], +) -> list[list[_Target]]: + """ + Attach every student to their team and group the targets by repository. + + A team's repository is graded once, and the outcome is spread over its + members' rows (§10.3). Students who are in no team are finished work: they + land in `job.results` with the `no_team` status and are never graded. + + Returns: + Groups of targets, one group per team repository + + Raises: + BulkGradingError: the organization's repositories are unavailable, so + no team can be resolved at all + """ + from .teams import TeamRegistry + + registry = TeamRegistry(github_client) + teams = registry.list_teams( + org, + lab_config.get("github-prefix", ""), + course_info.get("github", {}).get("teachers") or [], + ) + if teams is None: + raise BulkGradingError("Не удалось получить список команд лабораторной работы") + + index = registry.member_index(teams) + logger.info(f"Bulk job {job.job_id}: {len(teams)} team(s), {len(index)} member(s)") + + groups: "OrderedDict[str, list[_Target]]" = OrderedDict() + for target in targets: + team = index.get(target.username.casefold()) + if team is None: + job.results.append(BulkResult( + status="no_team", + student_name=target.student_name, + github=target.username, + message="Студент не состоит ни в одной команде этой лабораторной работы", + )) + continue + + target.repo = team.repo_name + target.team = team.title or team.slug + groups.setdefault(team.repo_name, []).append(target) + + return list(groups.values()) + + +def _team_member_result( + target: _Target, + outcome: StudentOutcome, + values: list[list[str]], + lab_col: int, +) -> tuple[BulkResult, bool]: + """ + Turn one team-wide outcome into one member's row of the report. + + The team was graded with a synthetic, empty cell value, so the cell + protection was not applied there - it is applied here, per member, against + that member's own cell (§10.3). + + Returns: + (result, should_write) + """ + if outcome.status != "updated": + return BulkResult( + status=outcome.status, + student_name=target.student_name, + github=target.username, + repo=target.repo, + grade=outcome.cell_value if outcome.status == "rejected" else None, + message=outcome.message, + registered=target.registered, + team=target.team, + ), False + + current = cell_from_grid(values, target.row, lab_col) + if not can_overwrite_cell(current): + return BulkResult( + status="rejected", + student_name=target.student_name, + github=target.username, + repo=target.repo, + grade=current, + message=CELL_PROTECTED_MESSAGE, + registered=target.registered, + team=target.team, + ), False + + return BulkResult( + status="updated", + student_name=target.student_name, + github=target.username, + repo=target.repo, + grade=outcome.cell_value, + message=outcome.message, + registered=target.registered, + team=target.team, + ), True + + def run_bulk_grading( job: BulkJob, grader: LabGrader, @@ -875,6 +991,14 @@ def flush() -> None: ) task_id_col = taskid_column(course_info, lab_config) + team_lab = is_team_lab(lab_config) + if team_lab and job.mode == "by_file": + # One name file per team cannot identify several students; the + # endpoint refuses this combination, this is the safety net. + raise BulkGradingError( + "Для командной лабораторной работы режим сопоставления по файлу с ФИО неприменим" + ) + if job.mode == "by_file": targets, github_writes = _plan_by_file( job, github_client, org, lab_config, job.name_file, @@ -884,22 +1008,45 @@ def flush() -> None: else: targets = _plan_by_sheet(values, student_col, github_col, lab_config) + if team_lab: + # One group per team repository; students without a team are + # already reported and drop out of `targets`. + units = _plan_teams(job, github_client, org, course_info, lab_config, targets) + targets = [target for unit in units for target in unit] + else: + units = [[target] for target in targets] + # 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" + f"Bulk job {job.job_id}: {len(targets)} student(s) to grade in " + f"{len(units)} unit(s), {len(job.results)} rejected while planning" ) cancelled = False - for target in targets: + for unit in units: if job.cancel_requested: logger.info(f"Bulk job {job.job_id}: cancellation requested") cancelled = True break - def context_for(target=target) -> SheetContext: + # A team's repository is evaluated exactly once, for the whole + # unit: the heavy part (files, commits, check-runs, job logs) must + # not be repeated per member. + first = unit[0] + + def context_for(target=first) -> SheetContext: + if team_lab: + # Synthetic context: the cell protection cannot be decided + # for a team, so it is applied per member afterwards, and + # a team has no order number to derive a TASKID from. + return SheetContext( + current_cell_value="", + student_order=None, + deadline=deadline, + decimal_separator=decimal_separator, + ) return SheetContext( current_cell_value=cell_from_grid(values, target.row, lab_col), student_order=( @@ -912,32 +1059,51 @@ def context_for(target=target) -> SheetContext: 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, + grader, org, first.username, lab_config, course_info, context_for, + repo_name=first.repo if team_lab else None, ) - if outcome.status == "updated": - pending.append((target.row, lab_col, outcome.cell_value)) + unit_results = [] + for target in unit: + if team_lab: + result, should_write = _team_member_result( + target, outcome, values, lab_col + ) + else: + 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, + ) + should_write = outcome.status == "updated" + if should_write: + pending.append((target.row, lab_col, outcome.cell_value)) + unit_results.append(result) 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, + logger.exception( + f"Bulk job {job.job_id}: error grading {first.username} ({first.repo})" ) - - job.results.append(result) - job.processed += 1 + unit_results = [ + BulkResult( + status="error", + student_name=target.student_name, + github=target.username, + repo=target.repo, + message=f"Внутренняя ошибка при проверке: {e}", + registered=target.registered, + team=target.team, + ) + for target in unit + ] + + job.results.extend(unit_results) + job.processed += len(unit_results) if len(pending) >= WRITE_BATCH_SIZE: flush() diff --git a/main.py b/main.py index 59f94d8..ee4d438 100644 --- a/main.py +++ b/main.py @@ -830,8 +830,32 @@ def load_sheet_context() -> SheetContext: decimal_separator=decimal_separator, ) + # A team lab has one repository per team, not per student: find the + # student's team and grade that repository, writing the result only + # into this student's own row (docs/TEAM_ASSIGNMENTS_PLAN.md §10.2). + team_repo_name = None + if is_team_lab(lab_config_dict): + registry = TeamRegistry(github_client) + teams = registry.list_teams( + org, repo_prefix, _course_teachers(course_info) + ) + if teams is None: + raise HTTPException( + status_code=502, + detail="Не удалось получить список команд с GitHub. Попробуйте ещё раз позже", + ) + team = registry.find_member_team(teams, username) + if team is None: + raise HTTPException( + status_code=404, + detail="Вы ещё не состоите в команде для этой лабораторной работы", + ) + team_repo_name = team.repo_name + logger.info(f"Grading team repository {org}/{team_repo_name} for '{username}'") + outcome = evaluate_student( grader, org, username, lab_config_dict, course_info, load_sheet_context, + repo_name=team_repo_name, ) if outcome.status == "error": @@ -1724,6 +1748,17 @@ def start_bulk_grade( ) raise HTTPException(status_code=400, detail="Missing course configuration") + if mode == "by_file" and is_team_lab(lab_config_dict): + # One repository holds one name file for several students, so a name + # cannot resolve a row for the whole team (§10.3 of the team plan). + raise HTTPException( + status_code=400, + detail=( + "Для командной лабораторной работы сопоставление по файлу с ФИО неприменимо: " + "проверяются студенты с указанным в таблице логином GitHub" + ), + ) + 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) diff --git a/tests/test_admin_endpoints.py b/tests/test_admin_endpoints.py index 51239c9..ce7728c 100644 --- a/tests/test_admin_endpoints.py +++ b/tests/test_admin_endpoints.py @@ -452,6 +452,33 @@ def test_name_file_selects_by_file_mode(self, mock_request, bulk_course_config, assert job.mode == "by_file" assert job.name_file == "info.md" + def test_by_file_mode_is_refused_for_a_team_lab(self, mock_request, bulk_course_config, mock_worksheet): + """One name file per team cannot identify several students (§10.3).""" + bulk_course_config["labs"]["1"]["team"] = {"size-max": 4} + + 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(name_file="info.md"), admin="admin", + ) + + assert exc_info.value.status_code == 400 + assert "ФИО" in exc_info.value.detail + + def test_team_lab_still_runs_in_by_sheet_mode(self, mock_request, bulk_course_config, mock_worksheet): + import json + + bulk_course_config["labs"]["1"]["team"] = {"size-max": 4} + 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(), admin="admin", + ) + + job = main_module.get_bulk_job(json.loads(response.body)["job_id"]) + assert job.mode == "by_sheet" + def test_blank_name_file_falls_back_to_by_sheet_mode(self, mock_request, bulk_course_config, mock_worksheet): import json diff --git a/tests/test_bulk_grading.py b/tests/test_bulk_grading.py index 0e6d782..a017df1 100644 --- a/tests/test_bulk_grading.py +++ b/tests/test_bulk_grading.py @@ -815,3 +815,198 @@ def test_planning_failures_count_toward_progress(self, bulk_setup): assert job.total == 2 assert job.processed == 2 + + +class TestRunBulkGradingTeamLab: + """A team lab grades one repository per team (§10.3 of the team plan).""" + + @pytest.fixture(autouse=True) + def clean_teams_state(self): + from grading.teams import reset_teams_state + + reset_teams_state() + yield + reset_teams_state() + + def _team_setup(self, bulk_setup, size_max=None): + bulk_setup["lab_config"]["team"] = {"size-max": size_max} if size_max else {} + return bulk_setup + + def _github_client(self, teams): + """teams: {slug: (description, [members])}""" + client = MagicMock() + prefix = "os-task1" + client.list_org_repos.return_value = [ + {"name": f"{prefix}-{slug}", "description": description} + for slug, (description, _members) in teams.items() + ] + rosters = { + f"{prefix}-{slug}": [ + {"login": login, "permissions": {"push": True, "admin": False}} + for login in members + ] + for slug, (_description, members) in teams.items() + } + client.list_collaborators.side_effect = lambda org, repo, affiliation="direct": ( + rosters.get(repo, []) + ) + client.list_invitations.side_effect = lambda org, repo: [] + return client + + def test_repository_is_evaluated_once_per_team(self, bulk_setup): + """alice and bob share team-1, so the repo is graded once, not twice.""" + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + assert job.status == "done" + assert grader.check_repository.call_count == 1 + assert grader._evaluate_ci_internal.call_count == 1 + + def test_the_team_repository_is_the_one_graded(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + org, repo, _config = grader.check_repository.call_args.args + assert repo == "os-task1-team-1" + assert all(result.repo == "os-task1-team-1" for result in job.results) + + def test_the_grade_reaches_every_member(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + job = _job() + _run(job, setup, github_client=client) + + assert [r.status for r in job.results] == ["updated", "updated"] + assert _written_cells(setup["worksheet"]) == [("D3", "v"), ("D4", "v")] + assert [r.team for r in job.results] == ["Пингвины", "Пингвины"] + + def test_cell_protection_is_applied_per_member(self, bulk_setup): + """Alice already has a grade; bob still gets his.""" + setup = self._team_setup(bulk_setup) + setup["grid"][2][3] = "v@8" + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + job = _job() + _run(job, setup, github_client=client) + + assert job.results[0].status == "rejected" + assert job.results[0].grade == "v@8" + assert job.results[1].status == "updated" + assert _written_cells(setup["worksheet"]) == [("D4", "v")] + + def test_student_without_a_team_is_reported(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice"])}) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + by_github = {r.github: r for r in job.results} + assert by_github["bob"].status == "no_team" + assert by_github["alice"].status == "updated" + assert grader.check_repository.call_count == 1 + assert job.total == 2 and job.processed == 2 + + def test_two_teams_are_graded_separately(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({ + "team-1": ("Пингвины", ["alice"]), + "team-2": ("Тюлени", ["bob"]), + }) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + assert grader.check_repository.call_count == 2 + assert {r.team for r in job.results} == {"Пингвины", "Тюлени"} + + def test_ci_error_is_copied_to_every_member(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + grader = _passing_grader() + grader.check_repository.return_value = _grade_result( + GradeStatus.ERROR, message="Нет коммитов", error_code="NO_COMMITS" + ) + job = _job() + _run(job, setup, grader=grader, github_client=client) + + assert [r.status for r in job.results] == ["error", "error"] + assert all("Нет коммитов" in r.message for r in job.results) + setup["worksheet"].batch_update.assert_not_called() + + def test_pending_ci_is_copied_to_every_member(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + ci = CIEvaluation( + grade_result=_grade_result(GradeStatus.PENDING, message="CI ещё выполняется ⏳"), + ci_passed=False, + ) + job = _job() + _run(job, setup, grader=_grader_mock(ci), github_client=client) + + assert [r.status for r in job.results] == ["pending", "pending"] + + def test_taskid_is_never_checked_for_a_team(self, bulk_setup): + setup = self._team_setup(bulk_setup) + setup["course_info"]["google"]["task-id-column"] = 0 + setup["lab_config"]["taskid-max"] = 20 + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + grader.check_taskid.assert_not_called() + assert [r.status for r in job.results] == ["updated", "updated"] + + def test_unavailable_teams_fail_the_job(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = MagicMock() + client.list_org_repos.return_value = None + job = _job() + _run(job, setup, github_client=client) + + assert job.status == "failed" + assert "команд" in job.error + + def test_by_file_mode_is_refused(self, bulk_setup): + setup = self._team_setup(bulk_setup) + job = _job(mode="by_file", name_file="info.md") + _run(job, setup, github_client=MagicMock()) + + assert job.status == "failed" + assert "файлу с ФИО" in job.error + + def test_dry_run_writes_nothing(self, bulk_setup): + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice", "bob"])}) + job = _job(dry_run=True) + _run(job, setup, github_client=client) + + assert [r.status for r in job.results] == ["updated", "updated"] + setup["worksheet"].batch_update.assert_not_called() + + def test_pending_invitee_is_graded_too(self, bulk_setup): + """A place is occupied by an invitation, and so is the grade row.""" + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": ("Пингвины", ["alice"])}) + client.list_invitations.side_effect = lambda org, repo: ( + [{"id": 1, "invitee": {"login": "bob"}}] if repo == "os-task1-team-1" else [] + ) + job = _job() + _run(job, setup, github_client=client) + + assert [r.status for r in job.results] == ["updated", "updated"] + + def test_slugless_team_falls_back_to_the_slug_as_a_name(self, bulk_setup): + """A repository whose description was cleared still reports a team.""" + setup = self._team_setup(bulk_setup) + client = self._github_client({"team-1": (None, ["alice", "bob"])}) + job = _job() + _run(job, setup, github_client=client) + + assert [r.team for r in job.results] == ["team-1", "team-1"] diff --git a/tests/test_grade_lab_characterization.py b/tests/test_grade_lab_characterization.py index 91efe38..a683ee1 100644 --- a/tests/test_grade_lab_characterization.py +++ b/tests/test_grade_lab_characterization.py @@ -564,3 +564,142 @@ def test_parse_invalid_lab_id(self, mock_env_vars): if __name__ == "__main__": pytest.main([__file__, "-v"]) + + +class TestGradeLabTeamLab: + """ + Single grading of a team lab: the team's repository is graded, and the + result goes into the requesting student's own row only + (docs/TEAM_ASSIGNMENTS_PLAN.md §10.2). + """ + + @pytest.fixture(autouse=True) + def setup(self, mock_env_vars): + from grading.teams import reset_teams_state + + reset_teams_state() + yield + reset_teams_state() + + @pytest.fixture + def team_course_config(self, sample_course_config): + sample_course_config["labs"]["1"]["team"] = {"size-max": 4} + return sample_course_config + + def _team_responses(self, org, repo_name, members=("testuser", "mate")): + responses.add( + responses.GET, + f"https://api.github.com/orgs/{org}/repos", + json=[{"name": repo_name, "description": "Пингвины"}], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/collaborators", + json=[ + {"login": login, "permissions": {"push": True, "admin": False}} + for login in members + ], + status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/invitations", + json=[], status=200, + ) + + def _passing_repo_responses(self, org, repo_name): + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/contents/test_main.py", + json={"name": "test_main.py"}, status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/contents/.github/workflows", + json=[{"name": "test.yml"}], status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/commits", + json=[{"sha": "abc123"}], status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/commits/abc123", + json={"sha": "abc123", "files": []}, status=200, + ) + responses.add( + responses.GET, + f"https://api.github.com/repos/{org}/{repo_name}/commits/abc123/check-runs", + json={"check_runs": [ + {"name": "test", "conclusion": "success", "html_url": "http://test"} + ]}, + status=200, + ) + + @responses.activate + def test_grades_the_team_repository_into_the_students_row( + self, team_course_config, mock_gspread, mock_service_account_creds, mock_request + ): + org = team_course_config["github"]["organization"] + repo_name = "test-task1-team-1" + self._team_responses(org, repo_name) + self._passing_repo_responses(org, repo_name) + + mock_gspread['worksheet'].row_values.return_value = ["№", "ФИО", "GitHub", "ЛР1"] + mock_gspread['worksheet'].col_values.return_value = ["", "", "testuser"] + + from main import grade_lab, GradeRequest + with patch("main.get_course_by_id", return_value=team_course_config): + result = grade_lab( + mock_request, "test-course", "group1", "ЛР1", GradeRequest(github="testuser"), + ) + + assert result["status"] == "updated" + assert result["result"] == "v" + # Only the requesting student's row is written - the endpoint is + # public and never writes into a groupmate's row. + mock_gspread['worksheet'].update_cell.assert_called_once() + # The individual repository name was never requested + assert not any("test-task1-testuser" in call.request.url for call in responses.calls) + + @responses.activate + def test_student_without_a_team_gets_a_clear_message( + self, team_course_config, mock_gspread, mock_service_account_creds, mock_request + ): + org = team_course_config["github"]["organization"] + self._team_responses(org, "test-task1-team-1", members=("mate",)) + + from main import grade_lab, GradeRequest + from fastapi import HTTPException + + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + grade_lab( + mock_request, "test-course", "group1", "ЛР1", + GradeRequest(github="testuser"), + ) + + assert exc_info.value.status_code == 404 + assert "команде" in exc_info.value.detail + mock_gspread['worksheet'].update_cell.assert_not_called() + + @responses.activate + def test_unavailable_teams_return_502( + self, team_course_config, mock_gspread, mock_service_account_creds, mock_request + ): + org = team_course_config["github"]["organization"] + responses.add(responses.GET, f"https://api.github.com/orgs/{org}/repos", status=500) + + from main import grade_lab, GradeRequest + from fastapi import HTTPException + + with patch("main.get_course_by_id", return_value=team_course_config): + with pytest.raises(HTTPException) as exc_info: + grade_lab( + mock_request, "test-course", "group1", "ЛР1", + GradeRequest(github="testuser"), + ) + + assert exc_info.value.status_code == 502 From 7d9161c5f1dc4185a0a64ad88e369d925a1a3966 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 20:10:02 +0300 Subject: [PATCH 07/10] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D1=82=D1=8C=20=D1=8D=D0=BA=D1=80=D0=B0=D0=BD=20=D0=B2=D1=8B?= =?UTF-8?q?=D0=B1=D0=BE=D1=80=D0=B0=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4?= =?UTF-8?q?=D1=8B=20=D0=B8=20=D0=B8=D0=BD=D1=81=D1=82=D1=80=D1=83=D0=BA?= =?UTF-8?q?=D1=86=D0=B8=D1=8E=20=D0=BF=D1=80=D0=B5=D0=BF=D0=BE=D0=B4=D0=B0?= =?UTF-8?q?=D0=B2=D0=B0=D1=82=D0=B5=D0=BB=D1=8E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Страница /join для командной лабы проходит четыре состояния: лендинг с кнопкой входа, выбор команды со списком и формой создания, карточка своей команды с кнопкой «Восстановить доступ», экран ошибки. Признак «студент авторизован» - успешный ответ GET .../teams, а не query-параметр: cookie join_session помечена HttpOnly и странице не видна, зато переживает перезагрузку. Заполненность команды показывается как «3 из 4», непринявшие приглашение участники помечены отдельно - видно, почему место занято. Кнопка присоединения блокируется для полных команд и для студента, который уже в команде; форма создания скрывается при достижении count-max с пояснением. Коды ошибок командных эндпоинтов переведены на ru, en и zh. Инструкция преподавателю (как удалить студента из команды, переименовать её, удалить пустую и закрыть создание новых) - в PROJECT_DESCRIPTION.md. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 24 ++ docs/PROJECT_DESCRIPTION.md | 47 +++- frontend/courses-front/src/api/index.js | 77 ++++++ .../src/components/JoinLab/CreateTeamForm.jsx | 88 +++++++ .../src/components/JoinLab/MyTeamCard.jsx | 62 +++++ .../src/components/JoinLab/TeamList.jsx | 81 +++++++ .../src/components/JoinLab/index.jsx | 221 ++++++++++++++++-- .../src/components/JoinLab/state.js | 74 ++++++ .../src/components/JoinLab/state.test.js | 120 ++++++++++ .../src/components/JoinLab/styled.js | 125 ++++++++++ .../src/locales/en/translation.json | 49 +++- .../src/locales/ru/translation.json | 49 +++- .../src/locales/zh/translation.json | 49 +++- 13 files changed, 1037 insertions(+), 29 deletions(-) create mode 100644 frontend/courses-front/src/components/JoinLab/CreateTeamForm.jsx create mode 100644 frontend/courses-front/src/components/JoinLab/MyTeamCard.jsx create mode 100644 frontend/courses-front/src/components/JoinLab/TeamList.jsx diff --git a/CLAUDE.md b/CLAUDE.md index 291273a..a5312bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,6 +56,7 @@ FRONTEND_URL=http://localhost:8080 |------|----------| | Add API endpoint | `main.py` | | Change grading logic | `grading/bulk.py` (`evaluate_student`, shared by single and bulk grading) | +| Team lab operations | `grading/teams.py` (`TeamRegistry`) | | 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}/` | @@ -119,6 +120,29 @@ 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`. +## Team (group) Lab Assignments + +Labs with a `team` section in their config are done by teams: one repository per team, shared by +its members (see `docs/TEAM_ASSIGNMENTS_PLAN.md` for the full design and +`docs/PROJECT_DESCRIPTION.md` for the teacher-facing instructions). + +- **A team is a repository.** `{github-prefix}-team-{N}` in the course organization; the roster is + its direct collaborators plus pending invitations, and the title/description live in the repo's + `description` field as `Название — описание`. No new storage: `grading/teams.py:TeamRegistry` + reads GitHub, caches the result for 30 s and mutates under a per-lab lock (single-worker backend + required, like `propagate.py`/`bulk.py`). Mutations re-read with `fresh=True` inside the lock. +- **Username only from the cookie.** The team endpoints take the student's GitHub login from the + signed `join_session` cookie (`require_join_session` in `main.py`) and never from the body, query + or path - anything else hands out access to a private repo under someone else's login. The + callback issues that cookie for a team lab instead of creating a repository. +- **`provision(access_username=...)`** separates the repo suffix (a team slug) from the student who + gets access; omitting it keeps the individual-lab behaviour untouched. +- **Grading**: `evaluate_student(..., repo_name=...)` grades the team's repository. It is called + exactly ONCE per team with a synthetic `SheetContext` (`current_cell_value=""`, + `student_order=None`); `can_overwrite_cell` is then applied per member against their own cell. + Calling it per member would triple the GitHub work. TASKID is off for team labs + (`taskid_column` returns None), and bulk `by_file` mode is refused. + ## Bulk Grading (admin) Grades a whole group for one lab in a single run, started from the admin lab list page diff --git a/docs/PROJECT_DESCRIPTION.md b/docs/PROJECT_DESCRIPTION.md index aa9e9dd..91f22c7 100644 --- a/docs/PROJECT_DESCRIPTION.md +++ b/docs/PROJECT_DESCRIPTION.md @@ -177,9 +177,12 @@ lab_grader_web/ | GET | `/courses/{course_id}/groups/{group_id}/labs` | Список лабораторных работ | | POST | `/courses/{course_id}/groups/{group_id}/register` | Регистрация студента | | POST | `/courses/{course_id}/groups/{group_id}/labs/{lab_id}/grade` | Проверка лабораторной работы | -| GET | `/join/{course_id}/{lab_id}` | Публичная информация для страницы присоединения к лабе | +| GET | `/join/{course_id}/{lab_id}` | Публичная информация для страницы присоединения к лабе (в том числе признак командной работы и лимиты) | | GET | `/join/{course_id}/{lab_id}/start` | Начало GitHub OAuth Flow для создания репозитория студента | -| GET | `/join/callback` | Колбэк GitHub OAuth: создание репозитория и починка доступа (см. `docs/REPO_GENERATION_PLAN.md`) | +| GET | `/join/callback` | Колбэк GitHub OAuth: создание репозитория и починка доступа (см. `docs/REPO_GENERATION_PLAN.md`); для командной лабы - выдача сессии студента | +| GET | `/join/{course_id}/{lab_id}/teams` | Список команд лабы, своя команда и лимиты (нужна cookie `join_session`) | +| POST | `/join/{course_id}/{lab_id}/teams` | Создание команды: `{"title": ..., "description": ...}` | +| POST | `/join/{course_id}/{lab_id}/teams/{slug}/join` | Вступление в команду либо починка доступа к своей | ### Административные маршруты @@ -245,6 +248,46 @@ lab_grader_web/ - Ошибка на одном студенте (нет коммитов, неверный вариант, недоступный репозиторий) не останавливает работу - она попадает в его строку отчёта. Останавливают работу только общие сбои: нет столбца `GitHub` или столбца лабы, недоступен список репозиториев организации. - Отмена проверяется между студентами: накопленная порция дописывается в таблицу, работа завершается со статусом `cancelled`. +### Командные лабораторные работы + +Лаба, которую студенты выполняют командами: один репозиторий на команду, доступ у всех её участников. Замена group assignment в GitHub Classroom. Ссылка для студентов та же, что и для индивидуальных лаб - `https:///join/{course_id}/{lab_id}`. + +**Как перевести лабу в командный режим.** Добавить в конфиг лабы секцию `team` (см. `docs/COURSE_CONFIG.md`); в ней необязательные `size-max` (максимум участников в команде) и `count-max` (максимум команд). Присутствие секции, даже пустой, - и есть признак командной лабы. Остальное как у индивидуальной: обязателен `template-repo`, `repo-provisioning` работает в обоих режимах. + +**Что видит студент.** После входа через GitHub - список уже созданных команд лабы: название, описание, логины участников, заполненность («3 из 4») и кнопку «Присоединиться»; плюс форму создания своей команды. Непринявшие приглашение участники помечены отдельно - видно, почему место занято. Повторный переход по той же ссылке приводит участника команды на карточку его команды с ссылкой на репозиторий и кнопкой «Восстановить доступ», которая пересоздаёт протухшее приглашение. + +**Как устроены репозитории команд.** `{github-prefix}-team-{N}` в организации курса, приватные; `N` - наименьший свободный номер среди существующих команд лабы. Участники - прямые коллабораторы с правом `push`; владельцы организации и логины из `github.teachers` в состав команды не попадают. Название и описание команды хранятся в поле `description` репозитория в виде `Название — описание` и правятся преподавателем прямо на GitHub. Отдельного хранилища состав команд не имеет: источник истины - сам репозиторий. + +**Как удалить студента из команды** (типовая ситуация: студент ошибся при выборе): + +1. Открыть репозиторий команды: `https://github.com/{организация}/{github-prefix}-team-{N}`. +2. Settings → Collaborators and teams. +3. Если студент принял приглашение - напротив его логина нажать `Remove`. +4. Если приглашение ещё не принято - оно показано в разделе `Pending invitations`, нажать `Cancel invitation`. Этот шаг обязателен: непринятое приглашение продолжает занимать место в команде и удерживает студента привязанным к ней. +5. Сообщить студенту, чтобы он снова открыл ссылку `/join/...` - он увидит список команд и сможет выбрать другую. + +Коммиты, которые студент успел сделать в старом репозитории, остаются в его истории; при необходимости преподаватель удаляет их обычными средствами Git. + +**Как переименовать команду.** Отредактировать `description` репозитория: `Название — описание` (разделитель - пробел, длинное тире, пробел). Если разделителя нет, вся строка показывается студентам как название. + +**Как удалить пустую команду.** Удалить репозиторий на GitHub. Номер `N` освободится и будет переиспользован следующей созданной командой. + +**Как закрыть создание новых команд.** Выставить `team.count-max` равным текущему числу команд. Вступление в уже созданные неполные команды при этом остаётся доступным: отдельного признака «формирование команд закрыто» нет. Убирать секцию `team` для этого нельзя - лаба перестанет быть командной, и проверка работ перестанет находить репозитории команд. + +**Проверка командных работ.** Оценка ставится каждому участнику в его собственную строку таблицы, но репозиторий проверяется один раз на команду: тяжёлая часть (файлы, коммиты, результаты CI, логи job'ов) не повторяется на каждого студента. Защита ячейки (`can_overwrite_cell`) при этом применяется индивидуально - у участника с уже выставленной оценкой она не перезаписывается, остальные оценку получают. Одиночная проверка (студент нажал «Проверить») находит репозиторий его команды по логину и пишет результат только в его строку; студент без команды получает понятное сообщение. В массовой проверке студенты без команды попадают в отчёт со статусом `no_team`, а режим сопоставления по файлу с ФИО для командных лаб недоступен - в репозитории один такой файл на несколько человек. + +**Проверка варианта (TASKID) для командных лаб не выполняется:** номер варианта выводится из порядкового номера студента в таблице, у команды такого номера нет. Заданный вместе с `team` ключ `taskid-max` игнорируется, при старте бэкенда это пишется в лог. + +**Технические детали и ограничения:** + +- Список команд лабы собирается из репозиториев организации (`1 + 2 × число_команд` запросов к GitHub) и кэшируется на 30 секунд: группа из 30 человек, одновременно открывшая страницу, тратит около 21 запроса вместо 630. +- Изменяющие операции (создание команды, вступление) идут под блокировкой своей лабы и внутри неё перечитывают список команд, минуя кэш, - два студента не могут занять один номер, одно название или последнее свободное место. Как и состояние фоновых работ, это корректно только при **одном uvicorn-воркере**. +- Действия преподавателя напрямую на GitHub в момент операции студента блокировкой не охватываются; расхождение исправляется на следующем чтении списка. Лимиты - проверка на момент операции, а не жёсткий инвариант. +- Непринятое приглашение занимает место в команде осознанно: иначе один студент мог бы занять места во всех командах. +- Модерация названий команд не выполняется: текст очищается от управляющих символов и обрезается по длине, за содержание отвечает преподаватель (может отредактировать `description` или удалить репозиторий). Логин создателя команды пишется в лог. +- Личность студента на всех командных эндпоинтах берётся только из подписанной cookie `join_session` (HttpOnly, `SameSite=Lax`, `path=/join`, 30 минут), которую выставляет колбэк OAuth. Из тела запроса, query-параметров и пути логин не принимается никогда. +- Переиспользования состава команд между лабами (аналог «set of teams» в GHC) нет: у каждой лабы свой репозиторий, состав задаётся заново. + ## Конфигурация курса ### Индексный файл курсов (`courses/index.yaml`) diff --git a/frontend/courses-front/src/api/index.js b/frontend/courses-front/src/api/index.js index 97d7a28..a6697a2 100644 --- a/frontend/courses-front/src/api/index.js +++ b/frontend/courses-front/src/api/index.js @@ -42,6 +42,83 @@ export const fetchJoinLab = async (courseId, labId) => { export const getJoinStartUrl = (courseId, labId) => `${API_BASE_URL}/join/${encodeURIComponent(courseId)}/${encodeURIComponent(labId)}/start`; + +// --- Командные лабораторные работы (docs/TEAM_ASSIGNMENTS_PLAN.md §8.2) --- +// +// Все три запроса идут с credentials: "include" - личность студента backend +// берёт из подписанной cookie join_session, и только из неё. В dev-режиме +// фронтенд на :8080 и backend на :8000 - это разные источники, поэтому без +// этой опции cookie не уедет (так же сделано в админке, LabList/index.jsx). + +const joinTeamsUrl = (courseId, labId) => + `${API_BASE_URL}/join/${encodeURIComponent(courseId)}/${encodeURIComponent(labId)}/teams`; + +// Backend отдаёт в `detail` стабильные коды (SESSION_REQUIRED, TEAM_FULL, ...), +// которые компонент переводит сам. Сюда попадают только те случаи, когда кода +// нет: сеть, таймаут, ответ прокси. +const teamErrorCode = (status, detail) => { + if (typeof detail === "string" && /^[A-Z][A-Z_]*$/.test(detail)) return detail; + if (status === 401) return "SESSION_REQUIRED"; + if (status === 404) return "join_not_found"; + if (status === 429) return "rate_limit"; + return "unknown"; +}; + +const requestJoinTeams = async (url, options = {}) => { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), JOIN_REQUEST_TIMEOUT_MS); + let response; + try { + response = await fetch(url, { + credentials: "include", + signal: controller.signal, + ...options, + }); + } catch (cause) { + const error = new Error("Team request failed", { cause }); + error.code = cause?.name === "AbortError" ? "request_timeout" : "unknown"; + throw error; + } finally { + clearTimeout(timeoutId); + } + + let data = null; + try { + data = await response.json(); + } catch { + // тело может быть пустым - код ошибки тогда выводится из статуса + } + + if (!response.ok) { + const error = new Error("Team request failed"); + error.code = teamErrorCode(response.status, data && data.detail); + error.status = response.status; + // ALREADY_IN_TEAM несёт с собой slug и ссылку на команду студента + error.payload = data || {}; + throw error; + } + + return data; +}; + +export const fetchJoinTeams = (courseId, labId) => + requestJoinTeams(joinTeamsUrl(courseId, labId)); + +export const createJoinTeam = (courseId, labId, { title, description }) => + requestJoinTeams(joinTeamsUrl(courseId, labId), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ title, description }), + }); + +// Имя репозитория собирает backend из префикса лабы и slug'а - отсюда +// уезжает только slug. +export const joinJoinTeam = (courseId, labId, slug) => + requestJoinTeams( + `${joinTeamsUrl(courseId, labId)}/${encodeURIComponent(slug)}/join`, + { method: "POST" } + ); + // Маппинг полей на русские названия для сообщений об ошибках const fieldLabels = { name: "Имя", diff --git a/frontend/courses-front/src/components/JoinLab/CreateTeamForm.jsx b/frontend/courses-front/src/components/JoinLab/CreateTeamForm.jsx new file mode 100644 index 0000000..4c1b050 --- /dev/null +++ b/frontend/courses-front/src/components/JoinLab/CreateTeamForm.jsx @@ -0,0 +1,88 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; + +import { + ActionButton, + FieldHint, + InlineError, + Label, + SectionTitle, + TeamForm, + TextInput, +} from "./styled"; +import { + DESCRIPTION_MAX_LENGTH, + ERROR_TRANSLATION_KEYS, + TITLE_MAX_LENGTH, + validateTeamForm, +} from "./state"; + + +export function CreateTeamForm({ canCreate, countMax, isBusy, onCreate }) { + const { t } = useTranslation(); + const [title, setTitle] = useState(""); + const [description, setDescription] = useState(""); + const [localError, setLocalError] = useState(null); + + if (!canCreate) { + return ( + + {countMax + ? t("join.team.creationClosedLimit", { count: countMax }) + : t("join.team.creationClosed")} + + ); + } + + const submit = (event) => { + event.preventDefault(); + // Клиентская проверка только избавляет от лишнего запроса: решение всё + // равно принимает backend (grading/teams.py), и его код ошибки победит. + const code = validateTeamForm(title, description); + setLocalError(code); + if (code) return; + onCreate({ title, description }); + }; + + return ( + + {t("join.team.createTitle")} + +
+ + setTitle(event.target.value)} + placeholder={t("join.team.namePlaceholder")} + /> +
+ +
+ + setDescription(event.target.value)} + placeholder={t("join.team.descriptionPlaceholder")} + /> +
+ + {localError && ( + + {t(ERROR_TRANSLATION_KEYS[localError] || "join.errors.unknown")} + + )} + + + {isBusy ? t("join.team.creating") : t("join.team.create")} + + + ); +} diff --git a/frontend/courses-front/src/components/JoinLab/MyTeamCard.jsx b/frontend/courses-front/src/components/JoinLab/MyTeamCard.jsx new file mode 100644 index 0000000..98f7dfb --- /dev/null +++ b/frontend/courses-front/src/components/JoinLab/MyTeamCard.jsx @@ -0,0 +1,62 @@ +import { useTranslation } from "react-i18next"; + +import { + FieldHint, + MemberChip, + RepositoryLink, + SecondaryButton, + SuccessPanel, + TeamCount, + TeamHeader, + TeamMembers, + TeamName, +} from "./styled"; +import { getSafeRepositoryUrl } from "./state"; + + +export function MyTeamCard({ team, sizeMax, isBusy, onRepairAccess }) { + const { t } = useTranslation(); + const repositoryUrl = getSafeRepositoryUrl(team.repo_url); + + return ( + + + {team.title || team.slug} + + {sizeMax + ? t("join.team.sizeOf", { size: team.size, max: sizeMax }) + : t("join.team.size", { count: team.size })} + + + + {team.description && {team.description}} + + + {team.members.map((login) => ( + {login} + ))} + {team.pending.map((login) => ( + + {login} · {t("join.team.pending")} + + ))} + + + {repositoryUrl ? ( + + {t("join.team.openRepository")} + + ) : ( + {t("join.errors.invalidRepositoryLink")} + )} + + {t("join.team.memberHint")} + + {/* Замена github-reinvite: пересоздаёт протухшее приглашение, + повторно проходя §4 плана #46 для репозитория команды. */} + + {isBusy ? t("join.team.repairing") : t("join.team.repairAccess")} + + + ); +} diff --git a/frontend/courses-front/src/components/JoinLab/TeamList.jsx b/frontend/courses-front/src/components/JoinLab/TeamList.jsx new file mode 100644 index 0000000..3d52ffb --- /dev/null +++ b/frontend/courses-front/src/components/JoinLab/TeamList.jsx @@ -0,0 +1,81 @@ +import { useTranslation } from "react-i18next"; + +import { + MemberChip, + SecondaryButton, + TeamCard, + TeamCards, + TeamCount, + TeamHeader, + TeamMembers, + TeamName, + Description, + FieldHint, +} from "./styled"; + + +// Логины участников показываются намеренно: это публичные идентификаторы +// GitHub, и именно по ним студент узнаёт команду своих однокурсников. ФИО не +// показываются - сценарий /join не знает группу студента и не открывает +// Google Таблицу. +export function TeamList({ teams, sizeMax, myTeam, busySlug, onJoin }) { + const { t } = useTranslation(); + + if (!teams.length) { + return {t("join.team.empty")}; + } + + return ( + + {teams.map((team) => { + const isMine = team.slug === myTeam; + const disabled = + Boolean(myTeam) || team.is_full || team.members_unknown || Boolean(busySlug); + + return ( + + + {team.title || team.slug} + + {sizeMax + ? t("join.team.sizeOf", { size: team.size, max: sizeMax }) + : t("join.team.size", { count: team.size })} + + + + {team.description && {team.description}} + + {team.members_unknown ? ( + {t("join.team.membersUnknown")} + ) : ( + + {team.members.map((login) => ( + {login} + ))} + {team.pending.map((login) => ( + + {login} · {t("join.team.pending")} + + ))} + + )} + + {!isMine && ( + onJoin(team.slug)} + > + {busySlug === team.slug + ? t("join.team.joining") + : team.is_full + ? t("join.team.full") + : t("join.team.join")} + + )} + + ); + })} + + ); +} diff --git a/frontend/courses-front/src/components/JoinLab/index.jsx b/frontend/courses-front/src/components/JoinLab/index.jsx index 6f23bf0..0526cea 100644 --- a/frontend/courses-front/src/components/JoinLab/index.jsx +++ b/frontend/courses-front/src/components/JoinLab/index.jsx @@ -1,10 +1,19 @@ -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { useParams, useSearchParams, useNavigate } from "react-router-dom"; -import { fetchJoinLab, getJoinStartUrl } from "../../api"; +import { + createJoinTeam, + fetchJoinLab, + fetchJoinTeams, + getJoinStartUrl, + joinJoinTeam, +} from "../../api"; import { SUPPORTED_LANGUAGES } from "../../language"; import { ButtonBack } from "../course-list/styled"; +import { CreateTeamForm } from "./CreateTeamForm"; +import { MyTeamCard } from "./MyTeamCard"; +import { TeamList } from "./TeamList"; import { ActionButton, Description, @@ -16,14 +25,18 @@ import { LanguageControl, LanguageSelect, RepositoryLink, + SectionTitle, Spinner, SuccessPanel, + TeamBadge, Title, Value, } from "./styled"; import { ERROR_TRANSLATION_KEYS, + findMyTeam, getSafeRepositoryUrl, + resolveJoinView, shouldShowJoinAction, } from "./state"; @@ -37,6 +50,11 @@ export function JoinLab() { const [loadError, setLoadError] = useState(null); const [isLoading, setIsLoading] = useState(true); const [isRedirecting, setIsRedirecting] = useState(false); + const [teamsData, setTeamsData] = useState(null); + const [teamsError, setTeamsError] = useState(null); + const [actionError, setActionError] = useState(null); + const [busySlug, setBusySlug] = useState(null); + const [isCreating, setIsCreating] = useState(false); const callbackStatus = searchParams.get("status"); // main.py передаёт код ошибки в query-параметре `reason` (не `error` - тот @@ -82,6 +100,46 @@ export function JoinLab() { }; }, [courseId, labId, hasLabContext, isStandaloneError]); + const teamEnabled = Boolean(lab && lab.team && lab.team.enabled); + + const reloadTeams = useCallback(() => { + // Признак «студент авторизован» - успешный ответ этого запроса: cookie + // join_session помечена HttpOnly и странице не видна, зато переживает + // перезагрузку, в отличие от query-параметра status=authenticated. + return fetchJoinTeams(courseId, labId) + .then((data) => { + setTeamsData(data); + setTeamsError(null); + return data; + }) + .catch((error) => { + setTeamsData(null); + setTeamsError(error.code || "unknown"); + return null; + }); + }, [courseId, labId]); + + useEffect(() => { + if (!teamEnabled) return; + let isCurrentRequest = true; + fetchJoinTeams(courseId, labId) + .then((data) => { + if (isCurrentRequest) { + setTeamsData(data); + setTeamsError(null); + } + }) + .catch((error) => { + if (isCurrentRequest) { + setTeamsData(null); + setTeamsError(error.code || "unknown"); + } + }); + return () => { + isCurrentRequest = false; + }; + }, [teamEnabled, courseId, labId]); + const beginOAuth = () => { setIsRedirecting(true); window.location.assign(getJoinStartUrl(courseId, labId)); @@ -90,6 +148,34 @@ export function JoinLab() { const translatedError = (code) => t(ERROR_TRANSLATION_KEYS[code] || "join.errors.unknown"); + const view = resolveJoinView({ teamEnabled, teamsData, teamsError }); + const myTeam = findMyTeam(teamsData); + + const handleCreate = ({ title, description }) => { + setActionError(null); + setIsCreating(true); + createJoinTeam(courseId, labId, { title, description }) + .then(() => reloadTeams()) + .catch((error) => { + setActionError(error.code || "unknown"); + // ALREADY_IN_TEAM и TITLE_TAKEN означают, что список устарел + reloadTeams(); + }) + .finally(() => setIsCreating(false)); + }; + + const handleJoin = (slug) => { + setActionError(null); + setBusySlug(slug); + joinJoinTeam(courseId, labId, slug) + .then(() => reloadTeams()) + .catch((error) => { + setActionError(error.code || "unknown"); + reloadTeams(); + }) + .finally(() => setBusySlug(null)); + }; + return ( navigate("/")}>{t("join.back")} @@ -147,37 +233,124 @@ export function JoinLab() { - {callbackStatus === "success" && repositoryUrl ? ( - - {t("join.successTitle")} - {t("join.successDescription")} - {username && ( - - {t("join.usernameLabel")}: {username} - + {teamEnabled && ( + <> + {t("join.team.badge")} + {(lab.team.size_max || lab.team.count_max) && ( + + {t("join.team.limits", { + size: lab.team.size_max || t("join.team.noLimit"), + count: lab.team.count_max || t("join.team.noLimit"), + })} + )} - - {t("join.openRepository")} - - - ) : callbackStatus === "success" ? ( + + )} + + {callbackStatus === "error" && ( {t("join.errorTitle")} - {t("join.errors.invalidRepositoryLink")} + {translatedError(callbackReason)} - ) : callbackStatus === "error" ? ( + )} + + {actionError && ( {t("join.errorTitle")} - {translatedError(callbackReason)} + {translatedError(actionError)} - ) : ( - {t("join.description")} )} - {shouldShowJoinAction(callbackStatus, repositoryUrl) && ( - - {isRedirecting ? t("join.redirecting") : t("join.signIn")} - + {view === "loading" && ( + + + )} + + {view === "landing" && ( + <> + {t("join.team.landing")} + + {isRedirecting ? t("join.redirecting") : t("join.signIn")} + + + )} + + {view === "error" && ( + <> + + {t("join.errorTitle")} + {translatedError(teamsError)} + + reloadTeams()}> + {t("join.team.retry")} + + + )} + + {view === "picker" && ( + <> + {t("join.team.pickTitle")} + {t("join.team.pickDescription")} + + + + )} + + {view === "member" && myTeam && ( + <> + {t("join.team.myTeamTitle")} + handleJoin(myTeam.slug)} + /> + + )} + + {view === "individual" && ( + <> + {callbackStatus === "success" && repositoryUrl ? ( + + {t("join.successTitle")} + {t("join.successDescription")} + {username && ( + + {t("join.usernameLabel")}: {username} + + )} + + {t("join.openRepository")} + + + ) : callbackStatus === "success" ? ( + + {t("join.errorTitle")} + {t("join.errors.invalidRepositoryLink")} + + ) : callbackStatus === "error" ? null : ( + {t("join.description")} + )} + + {shouldShowJoinAction(callbackStatus, repositoryUrl) && ( + + {isRedirecting ? t("join.redirecting") : t("join.signIn")} + + )} + )} )} diff --git a/frontend/courses-front/src/components/JoinLab/state.js b/frontend/courses-front/src/components/JoinLab/state.js index e61c1de..5ac5402 100644 --- a/frontend/courses-front/src/components/JoinLab/state.js +++ b/frontend/courses-front/src/components/JoinLab/state.js @@ -69,3 +69,77 @@ export function getSafeRepositoryUrl(rawUrl) { return null; } } + + +// Коды ошибок командных эндпоинтов (main.py §8.3 плана командных лаб). +// Отдаются backend'ом в `detail` как стабильные строки - фронтенд переводит +// их сам, как и коды RepoProvisioner выше. +Object.assign(ERROR_TRANSLATION_KEYS, { + NOT_A_TEAM_LAB: "join.errors.notATeamLab", + LAB_NOT_CONFIGURED: "join.errors.notConfigured", + SESSION_REQUIRED: "join.errors.sessionRequired", + TEAMS_UNAVAILABLE: "join.errors.teamsUnavailable", + TEAM_NOT_FOUND: "join.errors.teamNotFound", + ALREADY_IN_TEAM: "join.errors.alreadyInTeam", + TEAM_FULL: "join.errors.teamFull", + TEAM_LIMIT_REACHED: "join.errors.teamLimitReached", + TITLE_TAKEN: "join.errors.titleTaken", + INVALID_TITLE: "join.errors.invalidTitle", + SLUG_RACE: "join.errors.slugRace", + PROVISION_FAILED: "join.errors.unknown", + + // Клиентская валидация формы создания команды (§3.4 плана) + title_too_short: "join.errors.titleTooShort", + title_too_long: "join.errors.titleTooLong", + title_has_separator: "join.errors.titleHasSeparator", + description_too_long: "join.errors.descriptionTooLong", +}); + + +// Ограничения названия и описания команды. Должны совпадать с +// grading/teams.py: клиентская проверка только избавляет от лишнего запроса, +// решение всё равно принимает backend. +export const TITLE_MIN_LENGTH = 3; +export const TITLE_MAX_LENGTH = 60; +export const DESCRIPTION_MAX_LENGTH = 200; +export const DESCRIPTION_SEPARATOR = " — "; + + +export function cleanTeamText(value) { + return (value || "").replace(/\s+/g, " ").trim(); +} + + +export function validateTeamForm(title, description) { + const cleanTitle = cleanTeamText(title); + const cleanDescription = cleanTeamText(description); + + if (cleanTitle.length < TITLE_MIN_LENGTH) return "title_too_short"; + if (cleanTitle.length > TITLE_MAX_LENGTH) return "title_too_long"; + if (cleanTitle.includes(DESCRIPTION_SEPARATOR)) return "title_has_separator"; + if (cleanDescription.length > DESCRIPTION_MAX_LENGTH) return "description_too_long"; + return null; +} + + +/** + * Экран, который видит студент (§5 плана командных лаб). + * + * Признак «студент авторизован» - не query-параметр, а успешно полученный + * список команд: cookie join_session помечена HttpOnly и странице не видна, + * зато переживает перезагрузку, поэтому источником истины должен быть ответ + * backend, а не адресная строка. + */ +export function resolveJoinView({ teamEnabled, teamsData, teamsError }) { + if (!teamEnabled) return "individual"; + if (teamsData) return teamsData.my_team ? "member" : "picker"; + if (teamsError === "SESSION_REQUIRED") return "landing"; + if (teamsError) return "error"; + return "loading"; +} + + +export function findMyTeam(teamsData) { + if (!teamsData || !teamsData.my_team) return null; + return teamsData.teams.find((team) => team.slug === teamsData.my_team) || null; +} diff --git a/frontend/courses-front/src/components/JoinLab/state.test.js b/frontend/courses-front/src/components/JoinLab/state.test.js index 2d1e96c..9134327 100644 --- a/frontend/courses-front/src/components/JoinLab/state.test.js +++ b/frontend/courses-front/src/components/JoinLab/state.test.js @@ -3,8 +3,12 @@ import test from "node:test"; import { ERROR_TRANSLATION_KEYS, + cleanTeamText, + findMyTeam, getSafeRepositoryUrl, + resolveJoinView, shouldShowJoinAction, + validateTeamForm, } from "./state.js"; @@ -79,3 +83,119 @@ test("повреждённая ссылка успеха оставляет кн ); assert.equal(shouldShowJoinAction("error", null), true); }); + + +test("каждый код ошибки командных эндпоинтов имеет ключ локализации", () => { + const codes = [ + "NOT_A_TEAM_LAB", + "LAB_NOT_CONFIGURED", + "SESSION_REQUIRED", + "TEAMS_UNAVAILABLE", + "TEAM_NOT_FOUND", + "ALREADY_IN_TEAM", + "TEAM_FULL", + "TEAM_LIMIT_REACHED", + "TITLE_TAKEN", + "INVALID_TITLE", + "SLUG_RACE", + "PROVISION_FAILED", + "title_too_short", + "title_too_long", + "title_has_separator", + "description_too_long", + ]; + + for (const code of codes) { + assert.equal( + typeof ERROR_TRANSLATION_KEYS[code], + "string", + `код ${code} должен иметь ключ локализации` + ); + } +}); + + +test("состояние экрана выбирается по данным backend, а не по адресной строке", () => { + // Индивидуальная лаба - прежний экран + assert.equal( + resolveJoinView({ teamEnabled: false, teamsData: null, teamsError: null }), + "individual" + ); + + // Пока список команд не пришёл - загрузка + assert.equal( + resolveJoinView({ teamEnabled: true, teamsData: null, teamsError: null }), + "loading" + ); + + // Нет сессии - лендинг с кнопкой входа, а не ошибка + assert.equal( + resolveJoinView({ teamEnabled: true, teamsData: null, teamsError: "SESSION_REQUIRED" }), + "landing" + ); + + // Любая другая ошибка - экран ошибки + assert.equal( + resolveJoinView({ teamEnabled: true, teamsData: null, teamsError: "TEAMS_UNAVAILABLE" }), + "error" + ); + + // Авторизован, команды нет - выбор команды + assert.equal( + resolveJoinView({ + teamEnabled: true, + teamsData: { my_team: null, teams: [] }, + teamsError: null, + }), + "picker" + ); + + // Авторизован и состоит в команде - карточка своей команды + assert.equal( + resolveJoinView({ + teamEnabled: true, + teamsData: { my_team: "team-2", teams: [] }, + teamsError: null, + }), + "member" + ); +}); + + +test("своя команда находится по slug из ответа backend", () => { + const teamsData = { + my_team: "team-2", + teams: [ + { slug: "team-1", title: "Пингвины" }, + { slug: "team-2", title: "Тюлени" }, + ], + }; + + assert.equal(findMyTeam(teamsData).title, "Тюлени"); + assert.equal(findMyTeam({ my_team: null, teams: teamsData.teams }), null); + assert.equal(findMyTeam(null), null); + // Ссылка на несуществующую команду не должна ронять страницу + assert.equal(findMyTeam({ my_team: "team-9", teams: teamsData.teams }), null); +}); + + +test("клиентская валидация формы создания команды повторяет правила backend", () => { + assert.equal(validateTeamForm("Пингвины", ""), null); + assert.equal(validateTeamForm(" Пингвины ", " учим планировщик "), null); + + assert.equal(validateTeamForm("", ""), "title_too_short"); + assert.equal(validateTeamForm("ab", ""), "title_too_short"); + assert.equal(validateTeamForm("я".repeat(61), ""), "title_too_long"); + assert.equal(validateTeamForm("я".repeat(60), ""), null); + assert.equal(validateTeamForm("Пингвины — лучшие", ""), "title_has_separator"); + assert.equal(validateTeamForm("Пингвины", "я".repeat(201)), "description_too_long"); + // Разделитель в описании допустим - разбор идёт по первому вхождению + assert.equal(validateTeamForm("Пингвины", "первый — второй"), null); +}); + + +test("схлопывание пробелов совпадает с очисткой на backend", () => { + assert.equal(cleanTeamText(" Весёлые пингвины "), "Весёлые пингвины"); + assert.equal(cleanTeamText("Пингвины\nи тюлени"), "Пингвины и тюлени"); + assert.equal(cleanTeamText(null), ""); +}); diff --git a/frontend/courses-front/src/components/JoinLab/styled.js b/frontend/courses-front/src/components/JoinLab/styled.js index 74a7671..dc562f6 100644 --- a/frontend/courses-front/src/components/JoinLab/styled.js +++ b/frontend/courses-front/src/components/JoinLab/styled.js @@ -141,3 +141,128 @@ export const Spinner = styled.span` border-radius: 50%; animation: ${rotate} 0.8s linear infinite; `; + + +// --- Командные лабораторные работы --- + +export const TeamBadge = styled.span` + align-self: flex-start; + padding: 4px 10px; + border-radius: 100px; + border: 1px solid ${colors.buttonHover}; + background: rgba(60, 60, 67, 0.06); + color: ${colors.textSecondary}; + font-size: 12px; +`; + +export const SectionTitle = styled.h2` + margin: 0; + color: ${colors.textPrimary}; + font-size: 16px; + line-height: 1.4; +`; + +export const TeamCards = styled.ul` + display: flex; + flex-direction: column; + gap: 12px; + margin: 0; + padding: 0; + list-style: none; +`; + +export const TeamCard = styled.li` + display: flex; + flex-direction: column; + gap: 8px; + padding: 16px; + border: 1px solid ${colors.buttonHover}; + border-radius: 12px; + background: ${({ $mine }) => ($mine ? "rgba(34, 195, 142, 0.06)" : "#fff")}; + border-color: ${({ $mine }) => ($mine ? colors.save : colors.buttonHover)}; +`; + +export const TeamHeader = styled.div` + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 12px; +`; + +export const TeamName = styled.div` + color: ${colors.textPrimary}; + font-size: 15px; + font-weight: 600; +`; + +export const TeamCount = styled.span` + flex: 0 0 auto; + color: ${colors.textSecondary}; + font-size: 12px; +`; + +export const TeamMembers = styled.ul` + display: flex; + flex-wrap: wrap; + gap: 6px; + margin: 0; + padding: 0; + list-style: none; +`; + +export const MemberChip = styled.li` + padding: 3px 9px; + border-radius: 100px; + border: 1px dashed ${({ $pending }) => ($pending ? colors.cancel : "transparent")}; + background: ${({ $pending }) => ($pending ? "transparent" : colors.buttonHover)}; + color: ${({ $pending }) => ($pending ? colors.textSecondary : colors.textPrimary)}; + font-size: 12px; +`; + +export const SecondaryButton = styled(ActionButton)` + width: auto; + align-self: flex-start; + background: transparent; + color: ${colors.textPrimary}; + border: 1px solid ${colors.buttonBorder}; + + &:disabled { + cursor: not-allowed; + opacity: 0.45; + } +`; + +export const TeamForm = styled.form` + display: flex; + flex-direction: column; + gap: 12px; + padding: 16px; + border: 1px solid ${colors.buttonHover}; + border-radius: 12px; +`; + +export const TextInput = styled.input` + ${textStyles} + width: 100%; + box-sizing: border-box; + padding: 10px 12px; + border: 1px solid ${colors.buttonHover}; + border-radius: 8px; + color: ${colors.textPrimary}; + font-size: 14px; + + &:focus-visible { + outline: 2px solid ${colors.buttonBackground}; + outline-offset: 1px; + } +`; + +export const FieldHint = styled.span` + color: ${colors.textSecondary}; + font-size: 12px; +`; + +export const InlineError = styled.span` + color: ${colors.error}; + font-size: 12px; +`; diff --git a/frontend/courses-front/src/locales/en/translation.json b/frontend/courses-front/src/locales/en/translation.json index 71e3a99..51448ab 100644 --- a/frontend/courses-front/src/locales/en/translation.json +++ b/frontend/courses-front/src/locales/en/translation.json @@ -154,7 +154,54 @@ "actionsEnableFailed": "Could not enable checks in the repository. Contact the teacher.", "nameTaken": "A repository with this name is already taken. Contact the teacher.", "forkCheckFailed": "Could not verify the repository. Please try again in a few minutes.", - "unknown": "An unexpected error occurred. Try again or contact the teacher." + "unknown": "An unexpected error occurred. Try again or contact the teacher.", + "notATeamLab": "This lab is done individually, not as a team.", + "sessionRequired": "Your sign-in session has expired. Sign in with GitHub again.", + "teamsUnavailable": "Could not get the list of teams from GitHub. Try again in a minute.", + "teamNotFound": "Team not found. Refresh the list of teams.", + "alreadyInTeam": "You are already in a team for this lab. Contact your teacher to change teams.", + "teamFull": "This team has no free places left. Pick another one.", + "teamLimitReached": "The maximum number of teams has been reached. Join one of the existing teams.", + "titleTaken": "A team with this name already exists. Pick another name.", + "invalidTitle": "The team name is not acceptable. Check its length and remove unusual characters.", + "slugRace": "Someone created a team at the same moment. Please try again.", + "titleTooShort": "The team name must be at least 3 characters long.", + "titleTooLong": "The team name must be at most 60 characters long.", + "titleHasSeparator": "The team name must not contain “ — ”.", + "descriptionTooLong": "The team description must be at most 200 characters long." + }, + "team": { + "badge": "Team assignment", + "limits": "Maximum team members: {{size}}. Maximum teams: {{count}}.", + "noLimit": "unlimited", + "landing": "Sign in with GitHub to see this lab's teams and join one of them, or create your own.", + "loading": "Loading the list of teams…", + "retry": "Try again", + "pickTitle": "Teams of this lab", + "pickDescription": "Join your groupmates' team or create your own. One repository per team, shared by all its members.", + "empty": "There are no teams yet. Create the first one.", + "join": "Join", + "joining": "Joining…", + "full": "Team is full", + "size": "Members: {{count}}", + "sizeOf": "{{size}} of {{max}}", + "pending": "invitation sent", + "pendingHint": "The student has not accepted the invitation yet, but the place is already taken.", + "membersUnknown": "The team roster is unavailable right now. Refresh the page in a minute.", + "createTitle": "Create a team", + "nameLabel": "Team name", + "namePlaceholder": "For example: Penguins", + "descriptionLabel": "Description (optional)", + "descriptionPlaceholder": "What the team is working on", + "create": "Create team", + "creating": "Creating…", + "creationClosed": "Creating new teams is closed. Join one of the existing teams.", + "creationClosedLimit": "The maximum number of teams ({{count}}) has been reached. Join one of the existing teams.", + "myTeamTitle": "Your team", + "openRepository": "Open the team repository", + "memberHint": "If the repository invitation never arrived or has expired, press “Restore access” and it will be sent again.", + "repairAccess": "Restore access", + "repairing": "Restoring…" } } } diff --git a/frontend/courses-front/src/locales/ru/translation.json b/frontend/courses-front/src/locales/ru/translation.json index 8e9ca9a..c67f02a 100644 --- a/frontend/courses-front/src/locales/ru/translation.json +++ b/frontend/courses-front/src/locales/ru/translation.json @@ -154,7 +154,54 @@ "actionsEnableFailed": "Не удалось включить проверку в репозитории. Обратитесь к преподавателю.", "nameTaken": "Репозиторий с таким именем уже занят. Обратитесь к преподавателю.", "forkCheckFailed": "Не удалось проверить репозиторий. Попробуйте ещё раз через несколько минут.", - "unknown": "Произошла непредвиденная ошибка. Попробуйте ещё раз или сообщите преподавателю." + "unknown": "Произошла непредвиденная ошибка. Попробуйте ещё раз или сообщите преподавателю.", + "notATeamLab": "Эта лабораторная работа выполняется индивидуально, а не командой.", + "sessionRequired": "Сессия входа истекла. Войдите через GitHub ещё раз.", + "teamsUnavailable": "Не удалось получить список команд с GitHub. Попробуйте ещё раз через минуту.", + "teamNotFound": "Команда не найдена. Обновите список команд.", + "alreadyInTeam": "Вы уже состоите в команде этой лабораторной работы. Чтобы сменить команду, обратитесь к преподавателю.", + "teamFull": "В команде не осталось свободных мест. Выберите другую команду.", + "teamLimitReached": "Достигнуто максимальное число команд. Присоединитесь к одной из существующих.", + "titleTaken": "Команда с таким названием уже есть. Придумайте другое.", + "invalidTitle": "Название команды не подходит. Проверьте длину и уберите лишние символы.", + "slugRace": "Кто-то создал команду одновременно с вами. Повторите попытку.", + "titleTooShort": "Название команды должно содержать не меньше 3 символов.", + "titleTooLong": "Название команды не должно быть длиннее 60 символов.", + "titleHasSeparator": "Название команды не должно содержать « — ».", + "descriptionTooLong": "Описание команды не должно быть длиннее 200 символов." + }, + "team": { + "badge": "Командная работа", + "limits": "Максимум участников в команде: {{size}}. Максимум команд: {{count}}.", + "noLimit": "без ограничения", + "landing": "Войдите через GitHub, чтобы увидеть команды этой лабораторной работы и присоединиться к одной из них или создать свою.", + "loading": "Загрузка списка команд…", + "retry": "Повторить", + "pickTitle": "Команды лабораторной работы", + "pickDescription": "Присоединитесь к команде однокурсников или создайте свою. Один репозиторий на команду, доступ получают все участники.", + "empty": "Команд пока нет. Создайте первую.", + "join": "Присоединиться", + "joining": "Присоединяем…", + "full": "Мест нет", + "size": "Участников: {{count}}", + "sizeOf": "{{size}} из {{max}}", + "pending": "приглашение отправлено", + "pendingHint": "Студент ещё не принял приглашение, но место в команде уже занято.", + "membersUnknown": "Состав команды сейчас недоступен. Обновите страницу через минуту.", + "createTitle": "Создать команду", + "nameLabel": "Название команды", + "namePlaceholder": "Например: Пингвины", + "descriptionLabel": "Описание (необязательно)", + "descriptionPlaceholder": "Чем занимается команда", + "create": "Создать команду", + "creating": "Создаём…", + "creationClosed": "Создание новых команд закрыто. Присоединитесь к одной из существующих.", + "creationClosedLimit": "Достигнут максимум команд ({{count}}). Присоединитесь к одной из существующих.", + "myTeamTitle": "Ваша команда", + "openRepository": "Открыть репозиторий команды", + "memberHint": "Если приглашение в репозиторий не пришло или уже истекло, нажмите «Восстановить доступ» - оно будет отправлено заново.", + "repairAccess": "Восстановить доступ", + "repairing": "Восстанавливаем…" } } } diff --git a/frontend/courses-front/src/locales/zh/translation.json b/frontend/courses-front/src/locales/zh/translation.json index 0105ac5..e39a5d8 100644 --- a/frontend/courses-front/src/locales/zh/translation.json +++ b/frontend/courses-front/src/locales/zh/translation.json @@ -157,7 +157,54 @@ "actionsEnableFailed": "无法在仓库中启用检查，请联系教师。", "nameTaken": "该名称的仓库已被占用，请联系教师。", "forkCheckFailed": "无法验证仓库，请几分钟后重试。", - "unknown": "发生未知错误，请重试或联系教师。" + "unknown": "发生未知错误，请重试或联系教师。", + "notATeamLab": "本实验为个人完成，不是小组实验。", + "sessionRequired": "登录会话已过期，请重新使用 GitHub 登录。", + "teamsUnavailable": "无法从 GitHub 获取小组列表，请一分钟后重试。", + "teamNotFound": "未找到该小组，请刷新小组列表。", + "alreadyInTeam": "您已加入本实验的某个小组。如需更换小组，请联系教师。", + "teamFull": "该小组已满员，请选择其他小组。", + "teamLimitReached": "小组数量已达上限，请加入已有小组。", + "titleTaken": "该小组名称已被占用，请换一个名称。", + "invalidTitle": "小组名称不合适，请检查长度并删除特殊字符。", + "slugRace": "有人同时创建了小组，请重试。", + "titleTooShort": "小组名称至少需要 3 个字符。", + "titleTooLong": "小组名称不能超过 60 个字符。", + "titleHasSeparator": "小组名称不能包含“ — ”。", + "descriptionTooLong": "小组描述不能超过 200 个字符。" + }, + "team": { + "badge": "小组作业", + "limits": "每组最多 {{size}} 人，最多 {{count}} 个小组。", + "noLimit": "不限", + "landing": "使用 GitHub 登录后即可查看本实验的小组，加入其中一个或创建自己的小组。", + "loading": "正在加载小组列表…", + "retry": "重试", + "pickTitle": "本实验的小组", + "pickDescription": "加入同学的小组或创建自己的小组。每个小组共用一个仓库，所有成员都有访问权限。", + "empty": "目前还没有小组，创建第一个吧。", + "join": "加入", + "joining": "正在加入…", + "full": "小组已满", + "size": "成员：{{count}}", + "sizeOf": "{{size}} / {{max}}", + "pending": "邀请已发送", + "pendingHint": "该学生尚未接受邀请，但名额已被占用。", + "membersUnknown": "暂时无法获取小组成员，请一分钟后刷新页面。", + "createTitle": "创建小组", + "nameLabel": "小组名称", + "namePlaceholder": "例如：企鹅队", + "descriptionLabel": "描述（可选）", + "descriptionPlaceholder": "小组的研究方向", + "create": "创建小组", + "creating": "正在创建…", + "creationClosed": "已停止创建新小组，请加入已有小组。", + "creationClosedLimit": "小组数量已达上限（{{count}}），请加入已有小组。", + "myTeamTitle": "您的小组", + "openRepository": "打开小组仓库", + "memberHint": "如果没有收到仓库邀请或邀请已过期，请点击“恢复访问”重新发送。", + "repairAccess": "恢复访问", + "repairing": "正在恢复…" } } } From f4e2c5d8d676cafaa692e1d62f115c5c20e7af69 Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 20:12:50 +0300 Subject: [PATCH 08/10] =?UTF-8?q?=D0=A3=D0=B1=D1=80=D0=B0=D1=82=D1=8C=20.D?= =?UTF-8?q?S=5FStore=20=D0=B8=D0=B7=20=D0=B8=D0=BD=D0=B4=D0=B5=D0=BA=D1=81?= =?UTF-8?q?=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Файл попал в коммит этапа 3 по недосмотру: он служебный, macOS создаёт его сам и в репозитории ему не место. На диске файл остаётся, но снова становится неотслеживаемым. Co-Authored-By: Claude Opus 5 --- .DS_Store | Bin 6148 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 68aae905f9dd62de78f5fb2e2a9c162007003972..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeH~JqiLr422WjLa^D=avBfd4F=H@cmdJHO4vf|=jgutAh=qK$O|OjBr{>zSL|#= zM7Q^0Bhrh=0&bMGg^4NhP6ip}EVs*WJD From 693e2e423e94e9eac676a4d570c1fc4e75c521ea Mon Sep 17 00:00:00 2001 From: Mark Polyak Date: Tue, 8 Sep 2026 20:14:06 +0300 Subject: [PATCH 09/10] =?UTF-8?q?=D0=9F=D0=BE=D0=BA=D0=B0=D0=B7=D0=B0?= =?UTF-8?q?=D1=82=D1=8C=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=D1=83=20?= =?UTF-8?q?=D0=B2=20=D0=BE=D1=82=D1=87=D1=91=D1=82=D0=B5=20=D0=BC=D0=B0?= =?UTF-8?q?=D1=81=D1=81=D0=BE=D0=B2=D0=BE=D0=B9=20=D0=BF=D1=80=D0=BE=D0=B2?= =?UTF-8?q?=D0=B5=D1=80=D0=BA=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Колонка «Команда» и статус no_team в таблице отчёта. Колонка появляется только у командной лабы: у индивидуальной поле пустое у всех строк, и лишний столбец только мешает. Co-Authored-By: Claude Opus 5 --- .../src/components/admin/LabList/BulkGradeDialog.jsx | 6 ++++++ frontend/courses-front/src/locales/en/translation.json | 6 ++++-- frontend/courses-front/src/locales/ru/translation.json | 6 ++++-- frontend/courses-front/src/locales/zh/translation.json | 6 ++++-- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx b/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx index c0cb424..b5e043c 100644 --- a/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx +++ b/frontend/courses-front/src/components/admin/LabList/BulkGradeDialog.jsx @@ -32,6 +32,7 @@ const RESULT_STATUS_COLOR = { conflict: "error", unmatched: "warning", ambiguous: "warning", + no_team: "warning", }; async function fetchJson(url, options) { @@ -140,6 +141,9 @@ export const BulkGradeDialog = ({ courseId, lab, onClose, onError }) => { const running = job && job.status === "running"; const results = (job && job.results) || []; + // Колонка команды появляется только у командной лабы: у индивидуальной + // поле team пустое у всех строк, и лишний столбец только мешает. + const hasTeams = results.some((result) => result.team); return ( @@ -228,6 +232,7 @@ export const BulkGradeDialog = ({ courseId, lab, onClose, onError }) => { {t("adminLabs.bulk.columns.student")} {t("adminLabs.bulk.columns.github")} + {hasTeams && {t("adminLabs.bulk.columns.team")}} {t("adminLabs.bulk.columns.status")} {t("adminLabs.bulk.columns.grade")} {t("adminLabs.bulk.columns.message")} @@ -241,6 +246,7 @@ export const BulkGradeDialog = ({ courseId, lab, onClose, onError }) => { {r.github || "—"} {r.registered && ` (${t("adminLabs.bulk.registered")})`} + {hasTeams && {r.team || "—"}} Date: Tue, 8 Sep 2026 21:17:45 +0300 Subject: [PATCH 10/10] =?UTF-8?q?=D0=98=D1=81=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=B8=D1=82=D1=8C=20=D0=B7=D0=B0=D0=BC=D0=B5=D1=87=D0=B0=D0=BD?= =?UTF-8?q?=D0=B8=D1=8F=20=D0=BA=D0=BE=D0=B4-=D1=80=D0=B5=D0=B2=D1=8C?= =?UTF-8?q?=D1=8E=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=D0=BD=D1=8B=D1=85?= =?UTF-8?q?=20=D0=BB=D0=B0=D0=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Три дефекта, найденных ревью реализации issue #54. Лок лабы брался по сырому lab_id из URL, а find_lab_config приводит "5", "05", "ЛР5" и "lab5" к одной лабе - два студента получали два разных лока на одну лабу и одновременно проходили count-max, size-max, TITLE_TAKEN и ALREADY_IN_TEAM. _load_lab_for_join теперь отдаёт канонический ключ, и командные мутации лочатся по нему; заодно перестаёт неограниченно расти _lab_locks. Флаг members_unknown игнорировался: сбой GitHub на чтении состава читался как "студент ни в какой команде не состоит". В create_team проверки не было вовсе (студент заводил вторую команду со вторым репозиторием), в join_team она смотрела только на целевую команду, а групповая проверка сообщала преподавателю ложный no_team для целой команды. Определений членства было два: _ensure_access пропускал выдачу приглашения любому, у кого есть доступ на любом уровне, а состав команды считался по прямым коллабораторам с push. Студент с правом чтения (или с записью, унаследованной от базовых прав организации) получал "доступ выдан" на каждом заходе и навсегда оставался для проверки работ студентом без команды. Единственным определением членства объявлен состав команды; join_team передаёт force_invite для отсутствующих в нём, add_collaborator запрашивает push явно. Co-Authored-By: Claude Opus 5 --- docs/TEAM_ASSIGNMENTS_PLAN.md | 69 ++++++++++++++++++------- grading/bulk.py | 17 ++++++- grading/github_client.py | 28 ++++++++--- grading/repo_provisioning.py | 30 +++++++++-- grading/teams.py | 58 ++++++++++++++++----- main.py | 40 +++++++++------ tests/test_bulk_grading.py | 24 +++++++++ tests/test_join_endpoints.py | 64 ++++++++++++++++++++++++ tests/test_lab_resolution.py | 4 +- tests/test_teams.py | 94 ++++++++++++++++++++++++++++++++++- 10 files changed, 366 insertions(+), 62 deletions(-) diff --git a/docs/TEAM_ASSIGNMENTS_PLAN.md b/docs/TEAM_ASSIGNMENTS_PLAN.md index 16a861a..c187428 100644 --- a/docs/TEAM_ASSIGNMENTS_PLAN.md +++ b/docs/TEAM_ASSIGNMENTS_PLAN.md @@ -251,7 +251,13 @@ class TeamRegistry: операциями чтения: списком команд (`GET .../teams`) и определением команды студента при проверке работ (§10). Группа в 30 человек, одновременно открывшая страницу, тратит один набор запросов вместо тридцати. -- `_lab_locks: dict[(course_id, lab_id), threading.Lock]`, создаётся под общим мьютексом. +- `_lab_locks: dict[(course_id, lab_key), threading.Lock]`, создаётся под общим мьютексом. + `lab_key` — **канонический ключ лабы в YAML**, а не сырой сегмент URL: `find_lab_config` + приводит `5`, `05`, `ЛР5` и `lab5` к одной лабе, поэтому ключ по сырому `lab_id` выдал бы двум + студентам два разных лока на одну лабу, и `count-max`, `size-max`, `TITLE_TAKEN`, + `ALREADY_IN_TEAM` обходились бы одновременным запросом. Канонический ключ заодно ограничивает + размер `_lab_locks`: множество написаний бесконечно, множество ключей конфига — нет. + Ключ отдаёт `_load_lab_for_join`, который и так его вычисляет. Любая изменяющая операция (создание, присоединение) выполняется под блокировкой своей лабы и **внутри неё** перечитывает состояние с `fresh=True`, игнорируя кэш. Кэш инвалидируется после успешной мутации. @@ -267,23 +273,26 @@ class TeamRegistry: Под блокировкой лабы: 1. `teams = list_teams(fresh=True)`; при `None` — ошибка `TEAMS_UNAVAILABLE`. -2. Студент уже в команде → `ALREADY_IN_TEAM` (в ответе — slug и ссылка на его команду). -3. `count-max` задан и `len(teams) >= count-max` → `TEAM_LIMIT_REACHED`. -4. Название занято → `TITLE_TAKEN`. -5. `slug = f"team-{next_team_number(teams)}"`; выполняется `repo_exists(org, f"{prefix}-{slug}")`. +2. Хотя бы у одной команды `members_unknown` → `TEAMS_UNAVAILABLE`. Без полного состава нельзя + утверждать, что студент ещё не в команде, а ответ «не в команде» выдаёт ему вторую команду со + вторым репозиторием, между которыми потом придётся выбирать проверке. +3. Студент уже в команде → `ALREADY_IN_TEAM` (в ответе — slug и ссылка на его команду). +4. `count-max` задан и `len(teams) >= count-max` → `TEAM_LIMIT_REACHED`. +5. Название занято → `TITLE_TAKEN`. +6. `slug = f"team-{next_team_number(teams)}"`; выполняется `repo_exists(org, f"{prefix}-{slug}")`. Репозиторий с таким именем уже существует (список организации отстал от реального состояния или имя занято посторонним репозиторием) → `SLUG_RACE`, студенту предлагается повторить. -6. `RepoProvisioner.provision(org, github_prefix, template_repo, repo_suffix=slug, mode=..., +7. `RepoProvisioner.provision(org, github_prefix, template_repo, repo_suffix=slug, mode=..., access_username=username)` — создание репозитория и выдача доступа создателю. -7. При успехе — `update_repo(org, repo_name, {"description": composed_description})`. Этот вызов +8. При успехе — `update_repo(org, repo_name, {"description": composed_description})`. Этот вызов нужен в обоих режимах: `generate` не проставляет описание, а форк наследует описание шаблона, которое здесь заменяется названием команды. Ошибка на этом шаге логируется, но не отменяет создание: репозиторий рабочий, название можно проставить позже вручную. -8. Инвалидация кэша, возврат `TeamInfo` и `repo_url`. +9. Инвалидация кэша, возврат `TeamInfo` и `repo_url`. Гонка по имени репозитория (два студента одновременно получили один `N`) разрешается блокировкой лабы: оба запроса выполняются последовательно, второй видит уже созданную команду в свежем списке -и получает следующий номер. Проверка `repo_exists` на шаге 5 — страховка на случай, когда +и получает следующий номер. Проверка `repo_exists` на шаге 6 — страховка на случай, когда `list_org_repos` отдаёт неполный список сразу после создания репозитория. Если гонка всё же дойдёт до обработки `422` внутри `RepoProvisioner` («репозиторий появился параллельно»), `provision` вернёт успех и студент окажется участником созданной параллельно команды с чужим @@ -298,11 +307,15 @@ class TeamRegistry: `f"{github-prefix}-{slug}"`; из запроса имя репозитория не принимается никогда. 2. `teams = list_teams(fresh=True)`; команда не найдена → `TEAM_NOT_FOUND`. 3. Студент состоит в другой команде → `ALREADY_IN_TEAM`. -4. Студент уже в этой команде → выполняется только починка доступа (шаг 6) и возвращается `OK`. -5. `size-max` задан и `team.size >= size-max` → `TEAM_FULL`. -6. `RepoProvisioner.provision(..., repo_suffix=slug, access_username=username)` — репозиторий уже - существует, поэтому фактически выполняется `_ensure_access` (плюс `_repair_fork` в fork-режиме). -7. Инвалидация кэша, возврат `repo_url`. +4. Студент уже в этой команде → выполняется только починка доступа (шаг 7) и возвращается `OK`. +5. Хотя бы у одной команды `members_unknown` → `TEAMS_UNAVAILABLE`. «Не найден в прочитанных + составах» не означает «не состоит ни в одной команде»: нечитаемой может оказаться именно его + команда, а нечитаемый состав целевой команды вдобавок скрывает число занятых мест. +6. `size-max` задан и `team.size >= size-max` → `TEAM_FULL`. +7. `RepoProvisioner.provision(..., repo_suffix=slug, access_username=username, + force_invite=<студент отсутствует в составе>)` — репозиторий уже существует, поэтому фактически + выполняется `_ensure_access` (плюс `_repair_fork` в fork-режиме). Смысл `force_invite` — в §9.1. +8. Инвалидация кэша, возврат `repo_url`. ## 8. Backend: эндпоинты @@ -357,7 +370,7 @@ Google Таблицу. |---|---|---| | `NOT_A_TEAM_LAB` | 400 | Командный эндпоинт вызван для индивидуальной лабы | | `SESSION_REQUIRED` | 401 | Нет cookie, подпись невалидна, срок истёк, лаба в cookie не та | -| `TEAMS_UNAVAILABLE` | 502 | Не удалось получить список репозиториев организации | +| `TEAMS_UNAVAILABLE` | 502 | Не удалось получить список репозиториев организации либо состав хотя бы одной команды (§7.3, §7.4) | | `TEAM_NOT_FOUND` | 404 | Нет команды с таким slug | | `ALREADY_IN_TEAM` | 409 | Студент уже состоит в команде этой лабы | | `TEAM_FULL` | 409 | Достигнут `size-max` | @@ -377,12 +390,28 @@ Google Таблицу. ```python def provision(self, org, github_prefix, template_repo, repo_suffix, - mode="template", access_username=None) -> ProvisionResult: + mode="template", access_username=None, force_invite=False) -> ProvisionResult: ... - access_error = self._ensure_access(org, repo_name, access_username or repo_suffix) + access_error = self._ensure_access( + org, repo_name, access_username or repo_suffix, force_invite=force_invite + ) ``` -Обратная совместимость сохраняется: для индивидуальных лаб параметр не передаётся. +Обратная совместимость сохраняется: для индивидуальных лаб оба параметра не передаются. + +Второе изменение — параметр `force_invite`, снимающий расхождение двух определений членства. +`_ensure_access` начинается с быстрой проверки `is_direct_collaborator`, которая отвечает на вопрос +«может ли пользователь вообще открыть репозиторий», а не «является ли он прямым коллаборатором с +правом push». Доступ только на чтение и право записи, унаследованное от базовых прав организации, +дают там `204`, но в состав команды (§7.1) такой студент не попадает. Без `force_invite` он на +каждом заходе получал бы «доступ выдан» и при этом навсегда оставался бы для проверки работ +студентом без команды. Поэтому единственным определением членства объявляется состав из §7.1, а +`join_team` передаёт `force_invite=True` для студента, которого в этом составе нет: приглашение +выдаётся напрямую, и следующее чтение состава его увидит. + +Соответственно `GitHubClient.add_collaborator` передаёт `permission` явно (`"push"` по умолчанию), +а не полагается на значение по умолчанию на стороне GitHub: это же и повышает права уже +существующего коллаборатора с чтения до записи. ### 9.2. `grading/github_client.py` @@ -444,6 +473,10 @@ def list_collaborators(self, org, repo, affiliation="direct") -> list[dict] | No отвечает `400`, если его всё же запросили. 2. Целям (`_Target`) проставляется `repo` из `member_index`. Студенты без команды попадают в отчёт со статусом `no_team` (новый статус, аналог существующего `unmatched`) и не проверяются. + Если хотя бы у одной команды `members_unknown`, прогон прекращается с `BulkGradingError` — так + же, как при недоступном списке репозиториев организации. Участник команды с нечитаемым составом + неотличим от студента, который вообще не вступал в команду, и статус `no_team` сообщил бы + преподавателю неверный факт: что целая команда не зарегистрировалась. 3. Цели группируются по репозиторию команды. Для каждой команды `evaluate_student` вызывается **один раз**, с синтетическим `SheetContext`: `current_cell_value=""` (защита ячейки на этом шаге не применяется), `student_order=None`, реальные `deadline` и `decimal_separator`. diff --git a/grading/bulk.py b/grading/bulk.py index 137baa0..e81c7dd 100644 --- a/grading/bulk.py +++ b/grading/bulk.py @@ -827,8 +827,9 @@ def _plan_teams( Groups of targets, one group per team repository Raises: - BulkGradingError: the organization's repositories are unavailable, so - no team can be resolved at all + BulkGradingError: the organization's repositories are unavailable, or + some team's roster could not be read - in both cases a student without + a team cannot be told apart from one whose team is simply unreadable """ from .teams import TeamRegistry @@ -841,6 +842,18 @@ def _plan_teams( if teams is None: raise BulkGradingError("Не удалось получить список команд лабораторной работы") + unreadable = [team.slug for team in teams if team.members_unknown] + if unreadable: + # Members of a team whose roster could not be read are indistinguishable + # from students who never joined one. Reporting them as "no_team" would + # tell the teacher a whole team never registered, so the run stops + # instead - the same treatment the unavailable repository list gets. + raise BulkGradingError( + "Не удалось прочитать состав команд: " + + ", ".join(unreadable) + + ". Повторите проверку позже" + ) + index = registry.member_index(teams) logger.info(f"Bulk job {job.job_id}: {len(teams)} team(s), {len(index)} member(s)") diff --git a/grading/github_client.py b/grading/github_client.py index f6b41ad..e203ee8 100644 --- a/grading/github_client.py +++ b/grading/github_client.py @@ -572,10 +572,13 @@ def is_direct_collaborator(self, org: str, repo: str, username: str) -> bool: Note: GitHub's docs don't document an `affiliation` param for this single-user "check collaborator" endpoint (only for the list-collaborators one) - it's used here anyway per docs/REPO_GENERATION_PLAN.md §4, which - specifies this exact call. Team labs deliberately do NOT count a roster - with it: list_collaborators below takes the documented `affiliation` - param, and this one stays what it always was - a quick "does this user - already have access" check before issuing an invitation. + specifies this exact call. A 204 therefore means "can reach the + repository", not "is a direct collaborator with push": read-only + access and write inherited from the organization's base permission + both answer 204. Team labs must not decide membership from it - they + read the roster through list_collaborators below (documented + `affiliation`, plus the `permissions` object) and pass force_invite to + RepoProvisioner when a student is missing from it. Args: org: Organization or user name @@ -650,7 +653,13 @@ def delete_invitation(self, org: str, repo: str, invitation_id: int) -> bool: resp = requests.delete(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT) return resp.status_code == 204 - def add_collaborator(self, org: str, repo: str, username: str) -> requests.Response: + def add_collaborator( + self, + org: str, + repo: str, + username: str, + permission: str = "push", + ) -> requests.Response: """ Invite (or directly add) a user as a repository collaborator. @@ -662,13 +671,20 @@ def add_collaborator(self, org: str, repo: str, username: str) -> requests.Respo org: Organization or user name repo: Repository name username: GitHub username to invite + permission: Access level to grant. Sent explicitly rather than + relying on GitHub's default so that an existing collaborator + who only has read access is upgraded to push - a team member + who cannot push stays invisible to the roster (see + RepoProvisioner._ensure_access) Returns: The raw requests.Response (201 = invitation created, 204 = user already had access and was added directly) """ url = f"{self.BASE_URL}/repos/{org}/{repo}/collaborators/{username}" - return requests.put(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT) + return requests.put( + url, headers=self.headers, json={"permission": permission}, timeout=self.DEFAULT_TIMEOUT + ) def get_job_logs(self, org: str, repo: str, job_id: int) -> str | None: """ diff --git a/grading/repo_provisioning.py b/grading/repo_provisioning.py index 6931a57..6fd1aff 100644 --- a/grading/repo_provisioning.py +++ b/grading/repo_provisioning.py @@ -61,6 +61,7 @@ def provision( repo_suffix: str, mode: str = "template", access_username: str | None = None, + force_invite: bool = False, ) -> ProvisionResult: """ Ensure `{github_prefix}-{repo_suffix}` exists in `org` (created from @@ -85,6 +86,10 @@ def provision( API) or "fork" (a real fork of the template, see issue #51) access_username: Student to grant access to. Defaults to `repo_suffix`, preserving the individual-lab behavior. + force_invite: Skip the "already has access" shortcut and always + (re-)issue a direct push invitation. Callers that keep their + own definition of membership pass True when the student does + not match it - see _ensure_access. Returns: ProvisionResult describing success or the specific failure @@ -104,7 +109,9 @@ def provision( if create_error: return create_error - access_error = self._ensure_access(org, repo_name, access_username or repo_suffix) + access_error = self._ensure_access( + org, repo_name, access_username or repo_suffix, force_invite=force_invite + ) if access_error: return access_error @@ -406,17 +413,34 @@ def _repair_fork(self, org: str, repo_name: str) -> ProvisionResult | None: return None - def _ensure_access(self, org: str, repo_name: str, username: str) -> ProvisionResult | None: + def _ensure_access( + self, + org: str, + repo_name: str, + username: str, + force_invite: bool = False, + ) -> ProvisionResult | None: """ Make sure `username` has direct collaborator access to the repo, re-issuing a pending invitation if one already exists (a plain PUT without deleting the stale invitation first does not resend the notification - see docs/REPO_GENERATION_PLAN.md §4). + The shortcut below answers "can this user reach the repository at + all", which is not the question a team lab asks. Read-only access, or + write access inherited from the organization's base permission, both + answer 204 here while leaving the student out of the roster + TeamRegistry builds from direct collaborators with push + (docs/TEAM_ASSIGNMENTS_PLAN.md §7.1) - so the student would be told + "access granted" on every attempt and still be graded as teamless. + Callers that have already consulted that roster pass force_invite=True + to skip the shortcut, which leaves the roster as the single definition + of membership. + Returns: ProvisionResult with an error, or None if access is now in place """ - if self.github.is_direct_collaborator(org, repo_name, username): + if not force_invite and self.github.is_direct_collaborator(org, repo_name, username): logger.info(f"{username} already has direct access to {org}/{repo_name}") return None diff --git a/grading/teams.py b/grading/teams.py index 8ebb8f7..79c5bbf 100644 --- a/grading/teams.py +++ b/grading/teams.py @@ -259,9 +259,19 @@ def _error(code: str, message: str, team: TeamInfo | None = None) -> TeamActionR _lab_locks_mutex = threading.Lock() -def lab_lock(course_id: str, lab_id: str) -> threading.Lock: - """The mutation lock of one lab, created on first use.""" - key = (course_id, lab_id) +def lab_lock(course_id: str, lab_key: str) -> threading.Lock: + """ + The mutation lock of one lab, created on first use. + + `lab_key` must be the lab's canonical key in the course YAML, not the raw + lab_id from the URL: find_lab_config resolves "5", "05", "ЛР5" and "lab5" + to the same lab, so keying the lock by the raw path segment hands two + students two different locks for one lab and lets them pass count-max, + size-max, TITLE_TAKEN and ALREADY_IN_TEAM concurrently. Keying it + canonically also bounds the size of _lab_locks, which the raw value - + an unbounded set of spellings - does not. + """ + key = (course_id, lab_key) with _lab_locks_mutex: lock = _lab_locks.get(key) if lock is None: @@ -457,7 +467,7 @@ def next_team_number(teams: list[TeamInfo]) -> int: def create_team( self, course_id: str, - lab_id: str, + lab_key: str, org: str, github_prefix: str, template_repo: str, @@ -473,7 +483,8 @@ def create_team( The whole sequence runs under the lab's lock and re-reads the team list with fresh=True inside it, so two students cannot take the same - number or the same title. + number or the same title. `lab_key` must be the lab's canonical + config key - see lab_lock. """ config = team_config or TeamConfig() @@ -483,11 +494,21 @@ def create_team( except TeamTitleError as e: return _error("INVALID_TITLE", str(e)) - with lab_lock(course_id, lab_id): + with lab_lock(course_id, lab_key): teams = self.list_teams(org, github_prefix, teachers, fresh=True) if teams is None: return _error("TEAMS_UNAVAILABLE", "Не удалось получить список команд") + if any(team.members_unknown for team in teams): + # Without every roster there is no way to tell whether this + # student is already in a team, and guessing "no" hands them a + # second team with a second repository that grading then has + # to pick between. + return _error( + "TEAMS_UNAVAILABLE", + "Не удалось прочитать состав команд. Попробуйте ещё раз позже", + ) + existing = self.find_member_team(teams, username) if existing is not None: return _error( @@ -515,7 +536,7 @@ def create_team( return _error("SLUG_RACE", "Не удалось занять имя репозитория, повторите попытку") logger.info( - f"Student {username} creates team {slug} ({clean_title!r}) in {course_id}/{lab_id}" + f"Student {username} creates team {slug} ({clean_title!r}) in {course_id}/{lab_key}" ) provision = self.provisioner.provision( org, github_prefix, template_repo, slug, @@ -563,7 +584,7 @@ def create_team( def join_team( self, course_id: str, - lab_id: str, + lab_key: str, org: str, github_prefix: str, template_repo: str, @@ -579,14 +600,15 @@ def join_team( The repository name is always assembled by the server from the lab's prefix and a slug matching TEAM_SLUG_RE - a repository name is never - accepted from the request. + accepted from the request. `lab_key` must be the lab's canonical + config key - see lab_lock. """ config = team_config or TeamConfig() if not TEAM_SLUG_RE.match(slug or ""): return _error("TEAM_NOT_FOUND", "Команда не найдена") - with lab_lock(course_id, lab_id): + with lab_lock(course_id, lab_key): teams = self.list_teams(org, github_prefix, teachers, fresh=True) if teams is None: return _error("TEAMS_UNAVAILABLE", "Не удалось получить список команд") @@ -605,17 +627,22 @@ def join_team( already_in_this_team = current is not None if not already_in_this_team: - if team.members_unknown: + # `current is None` only means "not found in the rosters we + # could read". Any unreadable roster may be the student's own, + # and treating that as "in no team" lets them into a second + # one; an unreadable target roster additionally hides how many + # seats are taken. + if any(candidate.members_unknown for candidate in teams): return _error( "TEAMS_UNAVAILABLE", - "Не удалось прочитать состав команды. Попробуйте ещё раз позже", + "Не удалось прочитать состав команд. Попробуйте ещё раз позже", team=team, ) if config.size_max is not None and team.size >= config.size_max: return _error("TEAM_FULL", "В команде нет свободных мест", team=team) logger.info( - f"Student {username} joins team {slug} in {course_id}/{lab_id} " + f"Student {username} joins team {slug} in {course_id}/{lab_key} " f"(access repair: {already_in_this_team})" ) # The repository already exists, so this is effectively @@ -623,6 +650,11 @@ def join_team( provision = self.provisioner.provision( org, github_prefix, template_repo, slug, mode=mode, access_username=username, + # The roster read above is the definition of membership here, + # so a student missing from it needs a direct push invitation + # even when GitHub says they can already reach the repository + # - see RepoProvisioner._ensure_access. + force_invite=not already_in_this_team, ) if provision.status != ProvisionStatus.OK: return _error( diff --git a/main.py b/main.py index ee4d438..e14ba0f 100644 --- a/main.py +++ b/main.py @@ -923,14 +923,17 @@ def load_sheet_context() -> SheetContext: REPO_PROVISIONING_MODES = {"template", "fork"} -def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str, TeamConfig | None]: +def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, str, dict, str, TeamConfig | None]: """ Load course/lab config needed by the /join flow. Returns: - (course_info, lab_config, github_organization, team_config). The last - element is None for an individual lab and a TeamConfig for a team one - (docs/TEAM_ASSIGNMENTS_PLAN.md §4). + (course_info, lab_key, lab_config, github_organization, team_config). + `lab_key` is the lab's canonical key in the course YAML: one lab is + reachable through several spellings of lab_id ("5", "05", "ЛР5"), and + anything that keys shared state by lab must use this value rather than + the raw path segment. team_config is None for an individual lab and a + TeamConfig for a team one (docs/TEAM_ASSIGNMENTS_PLAN.md §4). Raises: HTTPException: 404 for unknown course/lab, 400 if the lab has no @@ -944,7 +947,7 @@ def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str, Te resolved = find_lab_config(labs, lab_id) if not resolved: raise HTTPException(status_code=404, detail="Лабораторная работа не найдена") - _lab_key, lab_config = resolved + lab_key, lab_config = resolved template_repo = lab_config.get("template-repo") if not template_repo: @@ -974,7 +977,7 @@ def _load_lab_for_join(course_id: str, lab_id: str) -> tuple[dict, dict, str, Te if not org: raise HTTPException(status_code=400, detail="Для курса не настроена GitHub организация") - return course_info, lab_config, org, team_config + return course_info, lab_key, lab_config, org, team_config def _oauth_redirect_uri(request: Request) -> str: @@ -1176,7 +1179,7 @@ def _exchange_code_for_username(code: str, redirect_uri: str) -> str | None: @limiter.limit("30/minute") def join_lab_info(request: Request, course_id: str, lab_id: str): """Публичная информация для лендинга страницы присоединения к лабе (без аутентификации).""" - course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) + course_info, _lab_key, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) teams_count = None if team_config is not None: @@ -1253,7 +1256,7 @@ def join_callback( return RedirectResponse(url=_join_result_redirect(course_id, lab_id, "error", reason="missing_code")) try: - course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) + course_info, _lab_key, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) except HTTPException: return RedirectResponse(url=_join_result_redirect(course_id, lab_id, "error", reason="config")) @@ -1319,21 +1322,26 @@ def _course_teachers(course_info: dict) -> list[str]: return [str(entry) for entry in teachers if entry] -def _load_team_lab(course_id: str, lab_id: str) -> tuple[dict, dict, str, TeamConfig]: +def _load_team_lab(course_id: str, lab_id: str) -> tuple[dict, str, dict, str, TeamConfig]: """ Like _load_lab_for_join, but only for a lab that really is a team lab. + Returns: + (course_info, lab_key, lab_config, github_organization, team_config). + `lab_key` is what the team mutations must lock on - see + _load_lab_for_join. + Raises: HTTPException(400): NOT_A_TEAM_LAB for an individual lab, or LAB_NOT_CONFIGURED when the lab has no github-prefix to build team repository names from """ - course_info, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) + course_info, lab_key, lab_config, org, team_config = _load_lab_for_join(course_id, lab_id) if team_config is None: raise HTTPException(status_code=400, detail="NOT_A_TEAM_LAB") if not lab_config.get("github-prefix"): raise HTTPException(status_code=400, detail="LAB_NOT_CONFIGURED") - return course_info, lab_config, org, team_config + return course_info, lab_key, lab_config, org, team_config # Provisioning failures that a student can retry (GitHub-side or transient) @@ -1400,7 +1408,7 @@ def _team_payload(team: TeamInfo, is_mine: bool, size_max: int | None) -> dict: @limiter.limit("30/minute") def join_lab_teams(request: Request, course_id: str, lab_id: str): """Список команд лабы, команда студента и лимиты (см. §8.2 плана).""" - course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + course_info, _lab_key, lab_config, org, team_config = _load_team_lab(course_id, lab_id) username = require_join_session(request, course_id, lab_id) registry = _team_registry() @@ -1473,12 +1481,12 @@ def _team_action_response(result, status_code: int = 200) -> JSONResponse | dict @limiter.limit("10/minute") def create_join_team(request: Request, course_id: str, lab_id: str, body: CreateTeamRequest): """Создаёт команду и выдаёт доступ к её репозиторию создателю (§7.3 плана).""" - course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + course_info, lab_key, lab_config, org, team_config = _load_team_lab(course_id, lab_id) username = require_join_session(request, course_id, lab_id) result = _team_registry().create_team( course_id=course_id, - lab_id=lab_id, + lab_key=lab_key, org=org, github_prefix=lab_config["github-prefix"], template_repo=lab_config["template-repo"], @@ -1501,12 +1509,12 @@ def join_join_team(request: Request, course_id: str, lab_id: str, slug: str): Имя репозитория собирается сервером из префикса лабы и slug'а, прошедшего TEAM_SLUG_RE; из запроса имя репозитория не принимается никогда (§7.4). """ - course_info, lab_config, org, team_config = _load_team_lab(course_id, lab_id) + course_info, lab_key, lab_config, org, team_config = _load_team_lab(course_id, lab_id) username = require_join_session(request, course_id, lab_id) result = _team_registry().join_team( course_id=course_id, - lab_id=lab_id, + lab_key=lab_key, org=org, github_prefix=lab_config["github-prefix"], template_repo=lab_config["template-repo"], diff --git a/tests/test_bulk_grading.py b/tests/test_bulk_grading.py index a017df1..23c41c3 100644 --- a/tests/test_bulk_grading.py +++ b/tests/test_bulk_grading.py @@ -912,6 +912,30 @@ def test_student_without_a_team_is_reported(self, bulk_setup): assert grader.check_repository.call_count == 1 assert job.total == 2 and job.processed == 2 + def test_unreadable_roster_fails_the_run_instead_of_reporting_no_team(self, bulk_setup): + """ + Regression: a team whose roster GitHub would not return used to be + skipped silently, and its members were reported as never having joined + a team - a false statement the teacher had no way to spot. + """ + setup = self._team_setup(bulk_setup) + client = self._github_client({ + "team-1": ("Пингвины", ["alice"]), + "team-2": ("Тюлени", ["bob"]), + }) + client.list_collaborators.side_effect = lambda org, repo, affiliation="direct": ( + None if repo == "os-task1-team-2" else + [{"login": "alice", "permissions": {"push": True, "admin": False}}] + ) + grader = _passing_grader() + job = _job() + _run(job, setup, grader=grader, github_client=client) + + assert job.status == "failed" + assert "team-2" in job.error + assert [r.status for r in job.results] == [] + assert grader.check_repository.call_count == 0 + def test_two_teams_are_graded_separately(self, bulk_setup): setup = self._team_setup(bulk_setup) client = self._github_client({ diff --git a/tests/test_join_endpoints.py b/tests/test_join_endpoints.py index 7a7161b..de8e4ca 100644 --- a/tests/test_join_endpoints.py +++ b/tests/test_join_endpoints.py @@ -949,3 +949,67 @@ def test_without_a_session_returns_401(self, team_course_config, mock_request): with pytest.raises(HTTPException) as exc_info: main_module.join_join_team(mock_request, "test-course", "1", "team-1") assert exc_info.value.status_code == 401 + + +class TestLabKeyCanonicalization: + """ + One lab is reachable through several spellings of lab_id, and the mutation + lock must not depend on which one the student's URL used. + """ + + def test_load_team_lab_returns_the_canonical_key(self, team_course_config): + with patch("main.get_course_by_id", return_value=team_course_config): + _course, by_key, _config, _org, _team = main_module._load_team_lab("test-course", "1") + _course, by_name, _config, _org, _team = main_module._load_team_lab("test-course", "ЛР1") + + assert by_key == "1" + assert by_name == "1" + + @responses.activate + def test_both_spellings_take_the_same_lock(self, team_course_config): + """ + Regression: keying the lock by the raw path segment handed "1" and + "ЛР1" two different locks, so two students could pass count-max, + size-max and ALREADY_IN_TEAM at the same time. + """ + import grading.teams as teams_module + + _team_repo_responses() + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/test-task1-team-1", + json={"name": "test-task1-team-1"}, + status=200, + ) + responses.add( + responses.GET, + "https://api.github.com/repos/test-org/test-task1-team-1/collaborators/dave", + status=404, + ) + responses.add( + responses.PUT, + "https://api.github.com/repos/test-org/test-task1-team-1/collaborators/dave", + status=201, + ) + responses.add( + responses.DELETE, + "https://api.github.com/repos/test-org/test-task1-team-1/invitations/1", + status=204, + ) + + keys = [] + real_lab_lock = teams_module.lab_lock + + def recording_lab_lock(course_id, lab_key): + keys.append((course_id, lab_key)) + return real_lab_lock(course_id, lab_key) + + with patch("grading.teams.lab_lock", recording_lab_lock), \ + patch("main.get_course_by_id", return_value=team_course_config): + for lab_id in ("1", "ЛР1"): + main_module.join_join_team( + _session_request("dave", lab_id=lab_id), "test-course", lab_id, "team-1", + ) + + assert len(keys) == 2 + assert keys[0] == keys[1] == ("test-course", "1") diff --git a/tests/test_lab_resolution.py b/tests/test_lab_resolution.py index 7057a44..a4f5595 100644 --- a/tests/test_lab_resolution.py +++ b/tests/test_lab_resolution.py @@ -155,11 +155,11 @@ def test_join_info_returns_the_lab_addressed_by_key(self, monkeypatch): } monkeypatch.setattr(main_module, "get_course_by_id", lambda _cid: course) - _course, lab_config, _org, _team = main_module._load_lab_for_join("os", "01") + _course, _key, lab_config, _org, _team = main_module._load_lab_for_join("os", "01") assert lab_config["short-name"] == "ЛР0.1" assert lab_config["template-repo"] == "org/t01" - _course, lab_config, _org, _team = main_module._load_lab_for_join("os", "1") + _course, _key, lab_config, _org, _team = main_module._load_lab_for_join("os", "1") assert lab_config["short-name"] == "ЛР1" def test_unknown_lab_still_404(self, monkeypatch): diff --git a/tests/test_teams.py b/tests/test_teams.py index d31e1dc..f6f5db4 100644 --- a/tests/test_teams.py +++ b/tests/test_teams.py @@ -478,10 +478,11 @@ def __init__(self, result=None): self.result = result def provision(self, org, github_prefix, template_repo, repo_suffix, - mode="template", access_username=None): + mode="template", access_username=None, force_invite=False): self.calls.append({ "org": org, "github_prefix": github_prefix, "template_repo": template_repo, "repo_suffix": repo_suffix, "mode": mode, "access_username": access_username, + "force_invite": force_invite, }) if self.result is not None: return self.result @@ -497,7 +498,7 @@ def _registry(github, provisioner=None): return TeamRegistry(github, provisioner or FakeProvisioner()) -LAB = dict(course_id="c", lab_id="5", org="test-org", +LAB = dict(course_id="c", lab_key="5", org="test-org", github_prefix="os-task5", template_repo="test-org/os-task5-template") @@ -724,3 +725,92 @@ def test_the_cache_is_dropped_after_joining(self): registry.join_team(**LAB, username="dave", slug="team-1") assert registry.cached_teams("test-org", "os-task5") is None + + +class TestUnreadableRoster: + """ + A roster GitHub refused to hand over is not evidence that the student is + in no team - §7.3/§7.4. Regression: it used to read as "teamless" and + handed the student a second team with a second repository. + """ + + def _github(self): + return FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины"), _repo("os-task5-team-2", "Тюлени")], + collaborators={ + "os-task5-team-1": [_collaborator("alice")], + # The student may well be in this one - there is no way to tell. + "os-task5-team-2": None, + }, + ) + + def test_creating_a_team_is_refused(self): + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).create_team( + **LAB, username="dave", title="Моржи", + ) + + assert result.error_code == "TEAMS_UNAVAILABLE" + assert provisioner.calls == [], "репозиторий не должен быть создан" + + def test_joining_another_team_is_refused(self): + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).join_team( + **LAB, username="dave", slug="team-1", + ) + + assert result.error_code == "TEAMS_UNAVAILABLE" + assert provisioner.calls == [] + + def test_repairing_access_in_your_own_team_still_works(self): + """alice is readable in team-1, so team-2 being unreadable changes nothing for her.""" + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).join_team( + **LAB, username="alice", slug="team-1", + ) + + assert result.status == TeamActionStatus.OK + assert provisioner.calls[0]["force_invite"] is False + + +class TestMembershipDefinition: + """ + The roster is the single definition of membership: a student missing from + it gets a direct push invitation even if GitHub says they can already + reach the repository (read-only access, or write inherited from the + organization's base permission). + """ + + def _github(self): + return FakeGitHub( + repos=[_repo("os-task5-team-1", "Пингвины")], + collaborators={"os-task5-team-1": [_collaborator("alice")]}, + invitations={"os-task5-team-1": [_invitation("carol")]}, + ) + + def test_a_student_outside_the_roster_is_force_invited(self): + provisioner = FakeProvisioner() + _registry(self._github(), provisioner).join_team( + **LAB, username="dave", slug="team-1", + ) + + assert provisioner.calls[0]["force_invite"] is True + assert provisioner.calls[0]["access_username"] == "dave" + + def test_an_existing_member_is_not_force_invited(self): + provisioner = FakeProvisioner() + _registry(self._github(), provisioner).join_team( + **LAB, username="alice", slug="team-1", + ) + + assert provisioner.calls[0]["force_invite"] is False + + def test_a_pending_invitee_repairs_access_without_forcing(self): + """carol was invited and has not accepted - the re-invite path handles her.""" + provisioner = FakeProvisioner() + result = _registry(self._github(), provisioner).join_team( + **LAB, username="carol", slug="team-1", + ) + + assert result.status == TeamActionStatus.OK + assert provisioner.calls[0]["force_invite"] is False