diff --git a/CONTEXT.md b/CONTEXT.md index 4eb1087..e42202f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -112,6 +112,18 @@ _Avoid_: Appointment, event, session A Project's one fixed weekly meeting: a weekday, a start and end time in Belgian time, and a place. Single dates can be skipped (holidays); clearing it stops it (e.g. during exams). Its meetings of the coming seven days show alongside the Deadlines and Meetings; an Archived Project has none. _Avoid_: Recurring meeting, standup, timetable +**Chat**: +A Project's one group chat (spec #91): short messages of its Chat people, shown oldest first, with new ones appearing within seconds. Every Teacher and the Superuser can also read and write in every Chat without being one of its Chat people. An Archived Project's Chat is read-only. +_Avoid_: Forum, channel, conversation, group + +**Chat people**: +Whom a Project's Chat is for: its Members and its Product Owner. Only they count for the Unread badge; a Member who is removed or moved loses access to the Chat at once, and a new Member sees its whole history. +_Avoid_: Participants, chat members + +**Unread**: +The messages of others in a Chat after the last one a User has seen; opening the Chat reads them. The Chat item in the navigation shows how many there are in the User's own Chats (for a Teacher: those where they are one of the Chat people). +_Avoid_: New messages, notifications + **Makers**: The permanent record of the Students (name, Programme, Year) who were Members of a Project when it was archived. Nobody is ever removed from it: archiving a restored Project again only adds the new Members (a Student already listed is not added twice). Shown as "Made by". _Avoid_: History, former members, credits diff --git a/backend/migrations/versions/0014_chat.py b/backend/migrations/versions/0014_chat.py new file mode 100644 index 0000000..b99e51c --- /dev/null +++ b/backend/migrations/versions/0014_chat.py @@ -0,0 +1,59 @@ +"""chat + +Revision ID: 0014 +Revises: 0013 +Create Date: 2026-10-22 14:20:00 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0014" +down_revision: str | Sequence[str] | None = "0013" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Upgrade schema.""" + # The Project Chat (spec #91): its messages, and how far each User read it. + op.create_table( + "chat_messages", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("author_id", sa.Integer(), nullable=True), + sa.Column("body", sa.Text(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.CheckConstraint("char_length(body) <= 4000", name="ck_chat_messages_length"), + sa.ForeignKeyConstraint(["author_id"], ["users.id"], ondelete="SET NULL"), + sa.ForeignKeyConstraint(["project_id"], ["projects.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_chat_messages_project_id"), "chat_messages", ["project_id"] + ) + op.create_table( + "chat_reads", + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("last_read_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["project_id"], ["projects.id"], ondelete="CASCADE"), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("project_id", "user_id"), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_table("chat_reads") + op.drop_index(op.f("ix_chat_messages_project_id"), table_name="chat_messages") + op.drop_table("chat_messages") diff --git a/backend/src/pmp_backend/api/chat.py b/backend/src/pmp_backend/api/chat.py new file mode 100644 index 0000000..d92709c --- /dev/null +++ b/backend/src/pmp_backend/api/chat.py @@ -0,0 +1,124 @@ +from datetime import datetime +from typing import Annotated + +from fastapi import APIRouter, Response, status +from pydantic import BaseModel, StringConstraints + +from pmp_backend.api.deps import CurrentUserDep, SessionDep +from pmp_backend.api.schemas import PersonOut +from pmp_backend.domain import ProjectStatus +from pmp_backend.models import ChatMessage, User +from pmp_backend.services import chat + +router = APIRouter(prefix="/api", tags=["chat"]) + + +class ChatOut(BaseModel): + project_id: int + title: str + # Archived Projects' Chats are read-only. + read_only: bool + + +class MessageIn(BaseModel): + body: Annotated[ + str, + StringConstraints( + strip_whitespace=True, min_length=1, max_length=chat.MAX_LENGTH + ), + ] + + +class MessageOut(BaseModel): + id: int + # None: a Deleted User. + author: PersonOut | None + body: str + created_at: datetime + mine: bool + + @classmethod + def of(cls, message: ChatMessage, user: User) -> "MessageOut": + author = message.author + return cls( + id=message.id, + author=PersonOut.of(author) if author else None, + body=message.body, + created_at=message.created_at, + mine=message.author_id == user.id, + ) + + +class ReadIn(BaseModel): + message_id: int + + +class ChatSummaryOut(BaseModel): + project_id: int + title: str + unread: int + mine: bool + + +class UnreadOut(BaseModel): + count: int + # Whether the User has any Chat (Students without a Project have none). + has_chat: bool + + +@router.get("/projects/{project_id}/chat") +def get_chat(project_id: int, session: SessionDep, user: CurrentUserDep) -> ChatOut: + project = chat.readable_project(session, user, project_id) + return ChatOut( + project_id=project.id, + title=project.title, + read_only=project.status == ProjectStatus.ARCHIVED.value, + ) + + +@router.get("/projects/{project_id}/chat/messages") +def list_messages( + project_id: int, + session: SessionDep, + user: CurrentUserDep, + after: int | None = None, +) -> list[MessageOut]: + """Oldest first: every message after `after`, or the newest ones.""" + found = chat.messages(session, user, project_id, after) + return [MessageOut.of(message, user) for message in found] + + +@router.post( + "/projects/{project_id}/chat/messages", status_code=status.HTTP_201_CREATED +) +def send_message( + project_id: int, body: MessageIn, session: SessionDep, user: CurrentUserDep +) -> MessageOut: + return MessageOut.of(chat.send(session, user, project_id, body.body), user) + + +@router.post("/projects/{project_id}/chat/read", status_code=status.HTTP_204_NO_CONTENT) +def mark_read( + project_id: int, body: ReadIn, session: SessionDep, user: CurrentUserDep +) -> Response: + chat.mark_read(session, user, project_id, body.message_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.get("/chats") +def list_chats(session: SessionDep, user: CurrentUserDep) -> list[ChatSummaryOut]: + return [ + ChatSummaryOut( + project_id=summary.project_id, + title=summary.title, + unread=summary.unread, + mine=summary.mine, + ) + for summary in chat.chats(session, user) + ] + + +@router.get("/chats/unread") +def unread(session: SessionDep, user: CurrentUserDep) -> UnreadOut: + count, has_chat = chat.unread_total(session, user) + return UnreadOut(count=count, has_chat=has_chat) diff --git a/backend/src/pmp_backend/app.py b/backend/src/pmp_backend/app.py index cc5c81c..621df19 100644 --- a/backend/src/pmp_backend/app.py +++ b/backend/src/pmp_backend/app.py @@ -8,6 +8,7 @@ from pmp_backend.api import ( auth, + chat, dev_login, github, planning, @@ -69,6 +70,7 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: app.include_router(github.router) app.include_router(trello.router) app.include_router(planning.router) + app.include_router(chat.router) if settings.dev_login: app.include_router(dev_login.router) diff --git a/backend/src/pmp_backend/models.py b/backend/src/pmp_backend/models.py index d43c0aa..5e653bb 100644 --- a/backend/src/pmp_backend/models.py +++ b/backend/src/pmp_backend/models.py @@ -313,3 +313,42 @@ class DailyEmailSent(Base): ForeignKey("users.id", ondelete="CASCADE"), primary_key=True ) day: Mapped[date] = mapped_column(Date, primary_key=True) + + +class ChatMessage(Base): + """A message in a Project's Chat (spec #91). A Deleted User leaves + `author_id` null: shown as "Deleted user".""" + + __tablename__ = "chat_messages" + __table_args__ = ( + CheckConstraint("char_length(body) <= 4000", name="ck_chat_messages_length"), + ) + + id: Mapped[int] = mapped_column(primary_key=True) + project_id: Mapped[int] = mapped_column( + ForeignKey("projects.id", ondelete="CASCADE"), index=True + ) + author_id: Mapped[int | None] = mapped_column( + ForeignKey("users.id", ondelete="SET NULL") + ) + body: Mapped[str] = mapped_column(Text) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) + + author: Mapped[User | None] = relationship(lazy="joined") + + +class ChatRead(Base): + """How far a User has read a Project's Chat: the last message they saw. + Messages after it are Unread for them.""" + + __tablename__ = "chat_reads" + + project_id: Mapped[int] = mapped_column( + ForeignKey("projects.id", ondelete="CASCADE"), primary_key=True + ) + user_id: Mapped[int] = mapped_column( + ForeignKey("users.id", ondelete="CASCADE"), primary_key=True + ) + last_read_id: Mapped[int] = mapped_column(default=0) diff --git a/backend/src/pmp_backend/services/chat.py b/backend/src/pmp_backend/services/chat.py new file mode 100644 index 0000000..6bfcdfb --- /dev/null +++ b/backend/src/pmp_backend/services/chat.py @@ -0,0 +1,178 @@ +"""The Project Chat (spec #91): one group chat per Project. + +Its Chat people are the Members and the Product Owner; every Teacher and the +Superuser can also read and write in every Chat. Unread counts come from how far +each User read a Chat (`ChatRead`). +""" + +from dataclasses import dataclass + +from sqlalchemy import and_, func, or_, select +from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.orm import Session + +from pmp_backend.domain import ProjectStatus, Role +from pmp_backend.errors import Conflict, Forbidden, Refused +from pmp_backend.models import ChatMessage, ChatRead, Project, User +from pmp_backend.services.projects import get_project + +MAX_LENGTH = 4000 +# The newest messages a Chat opens with; older ones come later (search, #97). +PAGE = 200 + + +class ChatError(Refused): + """A Chat action was refused; the message is fit to show.""" + + +class ChatForbiddenError(ChatError, Forbidden): + pass + + +class ChatConflictError(ChatError, Conflict): + pass + + +def is_staff(user: User) -> bool: + return user.role in (Role.TEACHER, Role.SUPERUSER) + + +def is_chat_person(user: User, project: Project) -> bool: + """A Member or the Product Owner: whom the Chat is for.""" + return user.project_id == project.id or user.id == project.product_owner_id + + +def can_read(user: User, project: Project) -> bool: + return is_staff(user) or is_chat_person(user, project) + + +def readable_project(session: Session, user: User, project_id: int) -> Project: + """The Project whose Chat `user` may read; refused otherwise.""" + project = get_project(session, project_id) + if not can_read(user, project): + raise ChatForbiddenError("You don't have access to this chat.") + return project + + +def messages( + session: Session, user: User, project_id: int, after: int | None = None +) -> list[ChatMessage]: + """The Chat's messages, oldest first: all after message `after`, or else + the newest PAGE.""" + readable_project(session, user, project_id) + query = select(ChatMessage).where(ChatMessage.project_id == project_id) + if after is not None: + return list( + session.scalars( + query.where(ChatMessage.id > after).order_by(ChatMessage.id) + ) + ) + newest = session.scalars(query.order_by(ChatMessage.id.desc()).limit(PAGE)) + return list(reversed(list(newest))) + + +def send(session: Session, user: User, project_id: int, body: str) -> ChatMessage: + project = readable_project(session, user, project_id) + if project.status == ProjectStatus.ARCHIVED.value: + raise ChatConflictError(f"{project.title} is archived: its chat is read-only.") + body = body.strip() + if not body: + raise ChatError("Write a message first.") + if len(body) > MAX_LENGTH: + raise ChatError(f"A message can be at most {MAX_LENGTH} characters.") + message = ChatMessage(project_id=project.id, author_id=user.id, body=body) + session.add(message) + session.flush() + # Your own message is never Unread for you. + _read_up_to(session, user, project.id, message.id) + session.commit() + return message + + +def mark_read(session: Session, user: User, project_id: int, message_id: int) -> None: + readable_project(session, user, project_id) + _read_up_to(session, user, project_id, message_id) + session.commit() + + +def _read_up_to(session: Session, user: User, project_id: int, message_id: int) -> None: + """Move the User's read mark forward (never back) to `message_id`.""" + statement = insert(ChatRead).values( + project_id=project_id, user_id=user.id, last_read_id=message_id + ) + session.execute( + statement.on_conflict_do_update( + index_elements=[ChatRead.project_id, ChatRead.user_id], + set_={ + "last_read_id": func.greatest( + ChatRead.last_read_id, statement.excluded.last_read_id + ) + }, + ) + ) + + +@dataclass(frozen=True) +class ChatSummary: + project_id: int + title: str + unread: int + # Whether it counts for the User's badge: they are one of its Chat people. + mine: bool + + +def _visible_projects(session: Session, user: User) -> list[Project]: + """Staff see every Active Project's Chat; Students their own Project's.""" + query = select(Project).where(Project.status == ProjectStatus.ACTIVE.value) + if not is_staff(user): + if user.project_id is None: + return [] + query = query.where(Project.id == user.project_id) + return list(session.scalars(query.order_by(func.lower(Project.title)))) + + +def _unread_counts(session: Session, user: User, project_ids: list[int]) -> dict: + """Per Project: messages by others after the User's read mark.""" + if not project_ids: + return {} + read_mark = func.coalesce(ChatRead.last_read_id, 0) + rows = session.execute( + select(ChatMessage.project_id, func.count()) + .outerjoin( + ChatRead, + and_( + ChatRead.project_id == ChatMessage.project_id, + ChatRead.user_id == user.id, + ), + ) + .where( + ChatMessage.project_id.in_(project_ids), + ChatMessage.id > read_mark, + or_(ChatMessage.author_id.is_(None), ChatMessage.author_id != user.id), + ) + .group_by(ChatMessage.project_id) + ) + return dict(rows.all()) + + +def chats(session: Session, user: User) -> list[ChatSummary]: + """The Chats the User sees, by title, with their Unread counts.""" + projects = _visible_projects(session, user) + counts = _unread_counts(session, user, [project.id for project in projects]) + return [ + ChatSummary( + project.id, + project.title, + counts.get(project.id, 0), + is_chat_person(user, project), + ) + for project in projects + ] + + +def unread_total(session: Session, user: User) -> tuple[int, bool]: + """The badge: Unread messages in the User's own Chats (for Teachers, those of + their Projects as Product Owner), and whether they have any Chat at all.""" + summaries = chats(session, user) + mine = [summary for summary in summaries if summary.mine] + return sum(summary.unread for summary in mine), bool(summaries) diff --git a/backend/tests/test_chat.py b/backend/tests/test_chat.py new file mode 100644 index 0000000..6047399 --- /dev/null +++ b/backend/tests/test_chat.py @@ -0,0 +1,186 @@ +from fastapi.testclient import TestClient + +from tests.conftest import ( + add_member, + auth, + create_project, + create_student, + ready_to_work, +) + + +def student_id(client: TestClient, staff) -> int: + students = client.get("/api/students", headers=staff).json() + return next(s["id"] for s in students if s["name"] == "Student Account") + + +def logged_in_student(client: TestClient, superuser, first_name: str) -> dict: + """A new Student's auth headers, past the password change.""" + user_id = create_student(client, superuser, first_name) + password = client.post(f"/api/users/{user_id}/reset-password", headers=superuser) + temporary = password.json()["temporary_password"] + email = f"{first_name.lower()}@student.vives.be" + token = client.post( + "/api/auth/login", json={"email": email, "password": temporary} + ).json()["access_token"] + client.post( + "/api/auth/change-password", + json={"current_password": temporary, "new_password": "own-password-1"}, + headers=auth(token), + ) + return auth(token) + + +def send(client: TestClient, headers, project_id: int, body: str): + return client.post( + f"/api/projects/{project_id}/chat/messages", + json={"body": body}, + headers=headers, + ) + + +def bodies(client: TestClient, headers, project_id: int, after=None) -> list[str]: + params = {} if after is None else {"after": after} + response = client.get( + f"/api/projects/{project_id}/chat/messages", params=params, headers=headers + ) + assert response.status_code == 200, response.text + return [message["body"] for message in response.json()] + + +def unread(client: TestClient, headers) -> dict: + return client.get("/api/chats/unread", headers=headers).json() + + +def test_the_team_and_staff_talk_in_the_project_chat(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + add_member(client, teacher, drone, student_id(client, teacher)) + student = ready_to_work(client, "student") + + first = send(client, student, drone, " Who brings the soldering iron? ") + send(client, teacher, drone, "I do. See https://vives.be") + send(client, superuser, drone, "Good luck!") + + assert first.status_code == 201 + assert first.json()["body"] == "Who brings the soldering iron?" + assert first.json()["author"]["name"] == "Student Account" + assert first.json()["mine"] is True + assert bodies(client, student, drone) == [ + "Who brings the soldering iron?", + "I do. See https://vives.be", + "Good luck!", + ] + assert bodies(client, teacher, drone, after=first.json()["id"]) == [ + "I do. See https://vives.be", + "Good luck!", + ] + chat = client.get(f"/api/projects/{drone}/chat", headers=student).json() + assert chat == {"project_id": drone, "title": "Drone", "read_only": False} + + +def test_other_students_have_no_access(client: TestClient): + teacher = ready_to_work(client, "teacher") + drone = create_project(client, teacher, "Drone") + outsider = logged_in_student(client, ready_to_work(client, "superuser"), "Lotte") + + assert send(client, outsider, drone, "Hi").status_code == 403 + response = client.get(f"/api/projects/{drone}/chat/messages", headers=outsider) + assert response.status_code == 403 + assert response.json()["detail"] == "You don't have access to this chat." + assert unread(client, outsider) == {"count": 0, "has_chat": False} + + +def test_a_removed_member_loses_access_and_a_new_one_sees_the_history( + client: TestClient, +): + teacher = ready_to_work(client, "teacher") + drone = create_project(client, teacher, "Drone") + student = student_id(client, teacher) + add_member(client, teacher, drone, student) + headers = ready_to_work(client, "student") + send(client, teacher, drone, "Welcome") + + client.delete(f"/api/projects/{drone}/members/{student}", headers=teacher) + removed = client.get(f"/api/projects/{drone}/chat/messages", headers=headers) + add_member(client, teacher, drone, student) + + assert removed.status_code == 403 + assert bodies(client, headers, drone) == ["Welcome"] + + +def test_unread_counts_follow_reading(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + add_member(client, teacher, drone, student_id(client, teacher)) + student = ready_to_work(client, "student") + + send(client, student, drone, "One") + last = send(client, student, drone, "Two").json()["id"] + + # The sender has nothing Unread; the Product Owner has both messages. + assert unread(client, student) == {"count": 0, "has_chat": True} + assert unread(client, teacher) == {"count": 2, "has_chat": True} + # The Superuser isn't one of the Chat people: listed, not in the badge. + assert unread(client, superuser)["count"] == 0 + listed = client.get("/api/chats", headers=superuser).json() + assert listed == [ + {"project_id": drone, "title": "Drone", "unread": 2, "mine": False} + ] + + read = client.post( + f"/api/projects/{drone}/chat/read", json={"message_id": last}, headers=teacher + ) + # Reading never moves back. + client.post( + f"/api/projects/{drone}/chat/read", json={"message_id": 0}, headers=teacher + ) + + assert read.status_code == 204 + assert unread(client, teacher)["count"] == 0 + send(client, superuser, drone, "Three") + assert unread(client, teacher)["count"] == 1 + assert unread(client, student)["count"] == 1 + + +def test_students_see_only_their_own_chat(client: TestClient): + teacher = ready_to_work(client, "teacher") + drone = create_project(client, teacher, "Drone") + create_project(client, teacher, "Robot") + add_member(client, teacher, drone, student_id(client, teacher)) + student = ready_to_work(client, "student") + + assert [c["title"] for c in client.get("/api/chats", headers=student).json()] == [ + "Drone" + ] + assert [c["title"] for c in client.get("/api/chats", headers=teacher).json()] == [ + "Drone", + "Robot", + ] + + +def test_message_length_is_checked(client: TestClient): + teacher = ready_to_work(client, "teacher") + drone = create_project(client, teacher, "Drone") + + assert send(client, teacher, drone, "x" * 4000).status_code == 201 + assert send(client, teacher, drone, "x" * 4001).status_code == 422 + assert send(client, teacher, drone, " ").status_code == 422 + + +def test_an_archived_projects_chat_is_read_only(client: TestClient): + teacher = ready_to_work(client, "teacher") + drone = create_project(client, teacher, "Drone") + send(client, teacher, drone, "Before") + + client.post(f"/api/projects/{drone}/archive", headers=teacher) + response = send(client, teacher, drone, "After") + + assert response.status_code == 409 + assert response.json()["detail"] == "Drone is archived: its chat is read-only." + assert bodies(client, teacher, drone) == ["Before"] + assert client.get(f"/api/projects/{drone}/chat", headers=teacher).json()[ + "read_only" + ] diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a9da199..153ea96 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -235,6 +235,14 @@ After the MVP went live, the team tried it and listed what bothered them. The li Newest first. "Live" means deployed to the VIVES VM by the Deploy workflow. "Migration" is the Alembic database version after that change. +### 2026-10-22: Project chat (spec #91) + +Grilling Q1–Q22 with the team, spec #91 and tickets #92–#98. Built one ticket at a time. + +| | What | Issue | PR | +|---|---|---|---| +| New | **Chat basics:** one chat per Project (Members, Product Owner; every Teacher and the Superuser can also read and write), text messages with clickable links, new messages within ~2 s (ADR 0008), Unread badge on the Chat / Chats item, Chats screen, Chat button on a Project's page; migration `0014` | #92 | #99 | + ### 2026-10-22: team change requests | | What | Issue | PR | diff --git a/docs/adr/0008-chat-updates-by-polling.md b/docs/adr/0008-chat-updates-by-polling.md new file mode 100644 index 0000000..bb66a40 --- /dev/null +++ b/docs/adr/0008-chat-updates-by-polling.md @@ -0,0 +1,16 @@ +# Chat updates by polling + +An open Project Chat (spec #91) asks the backend every 2 seconds for the messages after the last one it shows, and the navigation asks every 10 seconds for the Unread count. The backend doesn't push anything: the frontend keeps calling its REST API, as for every other screen (ADR 0003). + +The browser already has a live WebSocket to the NiceGUI frontend, so whatever the frontend learns appears at once. Only the step from the frontend to the backend is polled. With a few dozen people in open chats that is a few dozen small, indexed queries per second, which the VM handles easily. + +## Considered options + +- **Push from the backend** (WebSocket or server-sent events between backend and frontend): new messages without the 2-second delay, but a second kind of connection to keep alive across Deploys, and the backend would have to know which pages are open. +- **Sharing messages inside the frontend process** (one NiceGUI app, so pages could notify each other): fast, but it only sees messages sent through this frontend, and it puts Chat logic in the frontend instead of behind the API. + +## Consequences + +- A message from someone else appears within about 2 seconds; the sender sees their own at once. +- "Is typing…" (#97) uses the same polling, and the backend keeps it in memory, which works because the backend runs as one process. If it ever runs as several workers, typing has to move to the database. +- Polling stops when the page closes, so idle tabs cost nothing more than the 10-second badge check. diff --git a/frontend/src/pmp_frontend/api.py b/frontend/src/pmp_frontend/api.py index 5a2296f..02f0276 100644 --- a/frontend/src/pmp_frontend/api.py +++ b/frontend/src/pmp_frontend/api.py @@ -661,3 +661,48 @@ async def update_project( return await _request( "PUT", f"/api/projects/{project_id}", token=token, json=fields ) + + +async def chat(token: str, project_id: int) -> dict[str, Any]: + """A Project's Chat: project_id, title, read_only.""" + return await _request("GET", f"/api/projects/{project_id}/chat", token=token) + + +async def chat_messages( + token: str, project_id: int, *, after: int | None = None +) -> list[dict[str, Any]]: + """Oldest first: every message after `after`, or the newest ones (id, author, + body, created_at, mine).""" + query = f"?after={after}" if after is not None else "" + return await _request( + "GET", f"/api/projects/{project_id}/chat/messages{query}", token=token + ) + + +async def send_chat_message(token: str, project_id: int, body: str) -> dict[str, Any]: + return await _request( + "POST", + f"/api/projects/{project_id}/chat/messages", + token=token, + json={"body": body}, + ) + + +async def mark_chat_read(token: str, project_id: int, message_id: int) -> None: + await _request( + "POST", + f"/api/projects/{project_id}/chat/read", + token=token, + json={"message_id": message_id}, + ) + + +async def chats(token: str) -> list[dict[str, Any]]: + """The Chats the User sees: project_id, title, unread, mine.""" + return await _request("GET", "/api/chats", token=token) + + +async def chat_unread(token: str) -> dict[str, Any]: + """The badge: `count` of Unread messages in the User's own Chats, and + `has_chat`.""" + return await _request("GET", "/api/chats/unread", token=token) diff --git a/frontend/src/pmp_frontend/chat_page.py b/frontend/src/pmp_frontend/chat_page.py new file mode 100644 index 0000000..cbe9ab9 --- /dev/null +++ b/frontend/src/pmp_frontend/chat_page.py @@ -0,0 +1,184 @@ +"""The Project Chat (spec #91): a Student's Chat, the Chats screen for Teachers +and the Superuser, and one Chat's page, kept up to date by polling (ADR 0008).""" + +import html +import re +from contextlib import suppress +from typing import Any + +from nicegui import ui +from starlette.responses import Response + +from pmp_frontend import api +from pmp_frontend.shell import ErrorMessage, guard, page_frame, role_page, token +from pmp_frontend.times import shown + +STAFF = {"superuser", "teacher"} +# How often an open Chat asks for new messages (ADR 0008). +POLL_SECONDS = 2.0 +MAX_LENGTH = 4000 +# A web link, without trailing punctuation such as the full stop ending a sentence. +URL = re.compile(r"https?://[^\s<>\"']*[^\s<>\"'.,;:!?)\]]") + +CHATS_COLUMNS = [ + {"name": "title", "label": "Project", "field": "title", "align": "left"}, + {"name": "unread", "label": "Unread", "field": "unread", "align": "left"}, +] +UNREAD_CELL = """ + + + +""" + + +def linked(text: str) -> str: + """A message as HTML: escaped, with its web links clickable (new tab).""" + parts = [] + last = 0 + for match in URL.finditer(text): + parts.append(html.escape(text[last : match.start()])) + url = html.escape(match.group(0)) + parts.append( + f'{url}' + ) + last = match.end() + parts.append(html.escape(text[last:])) + return "".join(parts) + + +def _message(message: dict[str, Any]) -> None: + """One message: your own on the right on sand, others' on the left.""" + mine = message["mine"] + author = message["author"]["name"] if message["author"] else "Deleted user" + with ui.row().classes("w-full" + (" justify-end" if mine else "")): + bubble = "bg-[#efeee9]" if mine else "bg-white border" + with ( + ui.column() + .classes(f"gap-0 max-w-[80%] rounded-lg px-3 py-2 {bubble}") + .mark(f"message-{message['id']}") + ): + ui.label(f"{author} · {shown(message['created_at'])}").classes( + "text-caption" + ).style("color: #666666") + # Escaped by `linked`, so no sanitising needed. + ui.html(linked(message["body"]), sanitize=False).classes( + "whitespace-pre-wrap break-words" + ).mark("message-body") + + +async def chat_view(project_id: int) -> None: + """A Chat: its messages, new ones appearing within seconds, and the send box.""" + try: + chat = await api.chat(token(), project_id) + first = await api.chat_messages(token(), project_id) + except api.ApiError as error: + ui.label(error.message) + return + seen: set[int] = set() + state: dict[str, Any] = {"last": 0, "empty": None} + + def show(messages: list[dict[str, Any]]) -> bool: + """Add the messages not shown yet; whether there were any.""" + new = [message for message in messages if message["id"] not in seen] + if new and state["empty"] is not None: + state["empty"].delete() + state["empty"] = None + with column: + for message in new: + _message(message) + seen.add(message["id"]) + state["last"] = max(state["last"], message["id"]) + if new: + ui.timer(0.1, lambda: scroll.scroll_to(percent=1.0), once=True) + return bool(new) + + async def read() -> None: + if state["last"]: + # Only the badge depends on it; the next poll tries again. + with suppress(api.ApiError): + await api.mark_chat_read(token(), project_id, state["last"]) + + async def poll() -> None: + try: + new = await api.chat_messages(token(), project_id, after=state["last"]) + except api.ApiError: + # Shown again on the next try; a Deploy takes a minute. + return + if show(new): + await read() + + async def send() -> None: + body = (box.value or "").strip() + if not body: + return + try: + sent = await api.send_chat_message(token(), project_id, body) + except api.ApiError as error: + send_error.show(error.message) + return + send_error.hide() + box.value = "" + show([sent]) + + ui.label(f"Chat: {chat['title']}").classes("text-h4").mark("chat-title") + with ui.card().classes("w-full p-0"): + scroll = ui.scroll_area().classes("w-full h-[60vh]").mark("chat-messages") + with scroll: + column = ui.column().classes("w-full gap-2 p-2") + if not first: + with column: + state["empty"] = ui.label("No messages yet.").classes("text-caption") + show(first) + await read() + if chat["read_only"]: + ui.label("This Project is archived: its chat is read-only.") + else: + with ui.row().classes("w-full items-end no-wrap"): + box = ( + ui.textarea(placeholder="Write a message…") + .props(f"autogrow outlined dense maxlength={MAX_LENGTH}") + .classes("grow") + .mark("chat-input") + ) + ui.button(icon="send", on_click=send).props("round").mark("chat-send") + # Enter sends; Shift+Enter starts a new line. + box.on("keydown.enter.exact.prevent", send) + send_error = ErrorMessage() + ui.timer(POLL_SECONDS, poll) + + +def register() -> None: + @role_page("/chat", "Chat", {"student"}) + async def student_chat(user: api.CurrentUser) -> None: + found = await api.chats(token()) + if not found: + ui.label("Chat").classes("text-h4") + ui.label("You'll have a chat once you're a Member of a Project.") + return + await chat_view(found[0]["project_id"]) + + @role_page("/chats", "Chats", STAFF) + async def chats_page(user: api.CurrentUser) -> None: + ui.label("Chats").classes("text-h4") + found = await api.chats(token()) + if not found: + ui.label("No Active Projects, so no chats yet.") + return + table = ui.table(columns=CHATS_COLUMNS, rows=found, row_key="project_id") + table.classes("w-full").mark("chats") + table.add_slot("body-cell-unread", UNREAD_CELL) + table.on( + "rowClick", + lambda event: ui.navigate.to(f"/chats/{event.args[1]['project_id']}"), + ) + + @ui.page("/chats/{project_id}") + async def chat_page(project_id: int) -> Response | None: + user = await guard() + if isinstance(user, Response): + return user + with page_frame("Chat", user): + if user.role in STAFF: + ui.link("← Chats", "/chats").classes("text-dark") + await chat_view(project_id) + return None diff --git a/frontend/src/pmp_frontend/pages.py b/frontend/src/pmp_frontend/pages.py index 3886b5a..a4545d2 100644 --- a/frontend/src/pmp_frontend/pages.py +++ b/frontend/src/pmp_frontend/pages.py @@ -4,6 +4,7 @@ from pmp_frontend import ( api, + chat_page, members_page, my_project_page, photos, @@ -195,3 +196,4 @@ async def submit() -> None: photos.register() top3_page.register() planning.register() + chat_page.register() diff --git a/frontend/src/pmp_frontend/projects_page.py b/frontend/src/pmp_frontend/projects_page.py index b8e6980..ec04cb8 100644 --- a/frontend/src/pmp_frontend/projects_page.py +++ b/frontend/src/pmp_frontend/projects_page.py @@ -241,7 +241,14 @@ async def toggle_choice(event) -> None: archived = project["status"] == "archived" ui.link("← Projects", "/projects").classes("text-dark") - heading = ui.label(project["title"]).classes("text-h4") + with ui.row().classes("w-full items-center"): + heading = ui.label(project["title"]).classes("text-h4") + ui.space() + ui.button( + "Chat", + icon="forum", + on_click=lambda: ui.navigate.to(f"/chats/{project_id}"), + ).props("outline color=dark").mark("open-chat") # Status, Open for choice, Archive/Restore and Delete together in one # card instead of loose under the title (#81). with ui.card().classes("w-full max-w-lg").mark("status-card"): diff --git a/frontend/src/pmp_frontend/shell.py b/frontend/src/pmp_frontend/shell.py index f705036..10e1a55 100644 --- a/frontend/src/pmp_frontend/shell.py +++ b/frontend/src/pmp_frontend/shell.py @@ -22,15 +22,21 @@ ("Programmes", "/programmes", "school"), ("Top 3", "/top3", "format_list_numbered"), ("Planning", "/planning", "event"), + ("Chats", "/chats", "forum"), ], "teacher": [ ("Projects", "/projects", "folder"), ("Members", "/members", "badge"), ("Top 3", "/top3", "format_list_numbered"), ("Planning", "/planning", "event"), + ("Chats", "/chats", "forum"), ], - "student": [("My project", "/my-project", "work")], + "student": [("My project", "/my-project", "work"), ("Chat", "/chat", "chat")], } +# The navigation items that show the Unread badge (spec #91), and how often it +# is refreshed. +CHAT_PATHS = {"/chat", "/chats"} +BADGE_SECONDS = 10.0 def home(user: api.CurrentUser) -> str: @@ -74,7 +80,10 @@ def header(user: api.CurrentUser) -> None: # Until the Temporary password is changed, there is nowhere else to go. if not user.must_change_password: for label, path, icon in NAVIGATION[user.role]: - _navigation_link(label, path, icon).mark("navigation") + link = _navigation_link(label, path, icon).mark("navigation") + if path in CHAT_PATHS: + # A Student without any Chat doesn't get the item at all. + _unread_badge(link, hide_without_chat=user.role == "student") _navigation_link("My account", "/account", "account_circle") ui.label(user.full_name).classes("font-medium") ui.button("Log out", icon="logout", on_click=_logout).props("flat color=dark") @@ -90,6 +99,29 @@ def _navigation_link(label: str, path: str, icon: str) -> ui.link: return link +def _unread_badge(link: ui.link, *, hide_without_chat: bool) -> None: + """The number of Unread messages on the Chat item, refreshed every few + seconds; hidden while there are none.""" + with link: + badge = ui.badge(color="primary").props("rounded").mark("chat-badge") + badge.set_visibility(False) + if hide_without_chat: + link.set_visibility(False) + + async def refresh() -> None: + try: + unread = await api.chat_unread(token()) + except api.ApiError: + return + badge.text = str(unread["count"]) + badge.set_visibility(unread["count"] > 0) + if hide_without_chat: + link.set_visibility(unread["has_chat"]) + + ui.timer(0.1, refresh, once=True) + ui.timer(BADGE_SECONDS, refresh) + + def page_frame(title: str, user: api.CurrentUser): """The frame of a logged-in page: navigation, and the logo leading home.""" # Until the Temporary password is changed, there is nowhere else to go. diff --git a/frontend/tests/conftest.py b/frontend/tests/conftest.py index c1cf263..d6f369f 100644 --- a/frontend/tests/conftest.py +++ b/frontend/tests/conftest.py @@ -41,8 +41,28 @@ def trello() -> testing.FakeTrello: return testing.FakeTrello() +class FinishingTransport(httpx.ASGITransport): + """Lets every backend request finish, even when the page that sent it goes + away at the end of a test, as a real server does. A request cancelled halfway + would leave its database transaction open, and the next test's reset would + wait for it forever (pages poll the backend since the Chat, spec #91).""" + + def __init__(self, app) -> None: + super().__init__(app=app) + self.running: set[asyncio.Task] = set() + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + task = asyncio.ensure_future(super().handle_async_request(request)) + self.running.add(task) + task.add_done_callback(self.running.discard) + return await asyncio.shield(task) + + async def finish(self) -> None: + await asyncio.gather(*self.running, return_exceptions=True) + + @pytest.fixture(autouse=True) -def backend_in_process( +async def backend_in_process( tmp_path: Path, request: pytest.FixtureRequest, github: testing.FakeGitHub, @@ -76,9 +96,12 @@ def backend_in_process( # ASGITransport does not run the backend's startup, so seed here. with backend.state.sessionmaker() as session: seed_starting_accounts(session, logins_file) - api.use_transport(httpx.ASGITransport(app=backend)) + transport = FinishingTransport(backend) + api.use_transport(transport) yield api.use_transport(None) + await transport.finish() + backend.state.sessionmaker.kw["bind"].dispose() async def eventually(check: Callable[[], bool], retries: int = 60) -> None: diff --git a/frontend/tests/test_archive.py b/frontend/tests/test_archive.py index a2712dd..bef1cfe 100644 --- a/frontend/tests/test_archive.py +++ b/frontend/tests/test_archive.py @@ -12,7 +12,10 @@ def status_badge(user: User) -> str | None: """The text of the Status card's badge, once the page shows it.""" - found = user.find(marker="status-badge").elements + try: + found = user.find(marker="status-badge").elements + except AssertionError: + return None return next((badge.text for badge in found if isinstance(badge, ui.badge)), None) @@ -45,6 +48,7 @@ async def test_teacher_archives_and_restores_a_project(user: User): await eventually(lambda: project_titles(user) == ["Drone"]) await user.open("/projects/1") + await user.should_see(marker="change-status", retries=30) user.find(marker="change-status").click() await user.should_see("Restore Drone?", retries=10) user.find(marker="confirm").click() diff --git a/frontend/tests/test_chat.py b/frontend/tests/test_chat.py new file mode 100644 index 0000000..4dd1b87 --- /dev/null +++ b/frontend/tests/test_chat.py @@ -0,0 +1,136 @@ +import asyncio + +from nicegui import ui +from nicegui.testing import User + +from pmp_frontend import api +from pmp_frontend.chat_page import linked +from tests.conftest import STARTING_ACCOUNTS, eventually, ready_to_work + + +async def api_token(role: str) -> str: + """A starting account's token, through the API, past the password change.""" + email, password = STARTING_ACCOUNTS[role] + token, _ = await api.login(email, password) + await api.change_password(token, password, f"{role}-api-password") + return token + + +async def drone_with_the_student(token: str) -> int: + """A Project "Drone" owned by the Teacher, with the starting Student in it.""" + teachers = await api.teachers(token) + owner = next(t["id"] for t in teachers if t["name"] == "Teacher Account") + project = await api.create_project( + token, + { + "title": "Drone", + "description": None, + "product_owner_id": owner, + "team_size_min": 1, + "team_size_max": 3, + }, + ) + students = await api.students(token) + student = next(s["id"] for s in students if s["name"] == "Student Account") + await api.add_member(token, project["id"], student) + return project["id"] + + +def found(user: User, marker: str) -> list: + """The visible elements with `marker`, or none yet.""" + try: + return list(user.find(marker=marker).elements) + except AssertionError: + return [] + + +def navigation_targets(user: User) -> set[str]: + """Where the visible navigation items lead; hidden items aren't found.""" + return {link.props.get("href") for link in found(user, "navigation")} + + +def bodies(user: User) -> list[str]: + shown = sorted(found(user, "message-body"), key=lambda element: element.id) + return [element.content for element in shown] + + +async def test_a_student_chats_with_the_product_owner(user: User): + superuser = await api_token("superuser") + drone = await drone_with_the_student(superuser) + await ready_to_work(user, "student") + + await user.open("/chat") + await user.should_see("Chat: Drone", retries=30) + await eventually(lambda: "/chat" in navigation_targets(user)) + await user.should_see("No messages yet.") + user.find(marker="chat-input").type("Who brings the soldering iron?") + user.find(marker="chat-send").click() + await eventually(lambda: bodies(user) == ["Who brings the soldering iron?"]) + await user.should_not_see("No messages yet.") + + # Another person's message appears without reloading (polling, ADR 0008). + teacher = await api_token("teacher") + await api.send_chat_message(teacher, drone, "I do: https://vives.be.") + await eventually(lambda: len(bodies(user)) == 2, retries=100) + assert bodies(user)[1] == ( + 'I do: https://vives.be.' + ) + + +async def test_a_teacher_sees_unread_messages_and_opens_the_chat(user: User): + superuser = await api_token("superuser") + drone = await drone_with_the_student(superuser) + student = await api_token("student") + await api.send_chat_message(student, drone, "Can we meet on Friday?") + await ready_to_work(user, "teacher") + + await user.open("/chats") + await user.should_see(marker="chats", retries=30) + badge = user.find(marker="chat-badge").elements.pop() + assert isinstance(badge, ui.badge) + await eventually(lambda: badge.text == "1" and badge.visible, retries=100) + table = user.find(marker="chats").elements.pop() + assert isinstance(table, ui.table) + assert [(row["title"], row["unread"]) for row in table.rows] == [("Drone", 1)] + + await user.open(f"/chats/{drone}") + await user.should_see("Chat: Drone", retries=30) + await eventually(lambda: bodies(user) == ["Can we meet on Friday?"]) + # Opening the chat reads it (before the page is shown). + email, _ = STARTING_ACCOUNTS["teacher"] + teacher, _ = await api.login(email, "teacher-own-password") + assert (await api.chats(teacher))[0]["unread"] == 0 + + +async def test_a_project_page_links_to_its_chat(user: User): + superuser = await api_token("superuser") + drone = await drone_with_the_student(superuser) + await ready_to_work(user, "teacher") + + await user.open(f"/projects/{drone}") + await user.should_see(marker="open-chat", retries=30) + user.find(marker="open-chat").click() + + await user.should_see("Chat: Drone", retries=30) + + +async def test_a_student_without_a_project_has_no_chat_item(user: User): + await ready_to_work(user, "student") + await user.open("/my-project") + await user.should_see(marker="navigation", retries=30) + + # Shown only once the backend says there is a Chat (checked 0.1 s after the + # page loads): never, here. + await user.should_see("You haven't been assigned to a project yet.", retries=30) + await asyncio.sleep(1) + assert "/my-project" in navigation_targets(user) + assert "/chat" not in navigation_targets(user) + + +def test_links_are_escaped_and_clickable(): + assert linked("hi see http://x.be/a?b=1&c=2, ok") == ( + "<b>hi</b> see " + 'http://x.be/a?b=1&c=2, ok' + ) diff --git a/frontend/tests/test_projects.py b/frontend/tests/test_projects.py index 16a9911..cba309c 100644 --- a/frontend/tests/test_projects.py +++ b/frontend/tests/test_projects.py @@ -67,6 +67,14 @@ async def test_teacher_deletes_a_project_after_confirming(user: User): await eventually(lambda: project_titles(user) == []) +def photo_source(user: User) -> str | None: + """The shown photo's address; None while the photo card is redrawn.""" + try: + return user.find(marker="project-photo").elements.pop().source + except AssertionError: + return None + + async def test_teacher_uploads_replaces_and_removes_a_photo(user: User): await ready_to_work(user, "teacher") await create_project(user, "Drone", 1, 3) @@ -76,11 +84,7 @@ async def test_teacher_uploads_replaces_and_removes_a_photo(user: User): await upload_photo(user) await user.should_see(marker="project-photo", retries=30) await upload_photo(user) - await eventually( - lambda: ( - user.find(marker="project-photo").elements.pop().source == "/photos/1?v=2" - ) - ) + await eventually(lambda: photo_source(user) == "/photos/1?v=2") user.find(marker="remove-photo").click() await user.should_see("Remove the photo of Drone?", retries=10) diff --git a/frontend/tests/test_users.py b/frontend/tests/test_users.py index 6ba93af..22e5abb 100644 --- a/frontend/tests/test_users.py +++ b/frontend/tests/test_users.py @@ -33,6 +33,7 @@ async def test_superuser_creates_a_student(user: User): async def test_duplicate_email_is_shown_in_the_dialog(user: User): await ready_to_work(user, "superuser") await user.open("/users") + await user.should_see(marker="users", retries=30) user.find(marker="new-user").click() user.find(marker="first-name").type("Copy") @@ -46,6 +47,7 @@ async def test_duplicate_email_is_shown_in_the_dialog(user: User): async def test_invalid_input_gets_a_readable_message(user: User): await ready_to_work(user, "superuser") await user.open("/users") + await user.should_see(marker="users", retries=30) user.find(marker="new-user").click() user.find(marker="first-name").type("Ann")