diff --git a/CONTEXT.md b/CONTEXT.md index e42202f..a833ab8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -117,9 +117,13 @@ A Project's one group chat (spec #91): short messages of its Chat people, shown _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. +Whom a Project's Chat is for: its Members, its Product Owner and its Guests. 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 +**Guest**: +Any active User added to a Project's Chat on top of its team, by the Product Owner, a Teacher or the Superuser (never by Members). A Guest reads the whole history and writes like the team, can leave, and can be removed by the same staff; archiving the Project ends their access. +_Avoid_: Visitor, external member, invitee + **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 diff --git a/backend/migrations/versions/0015_chat_guests.py b/backend/migrations/versions/0015_chat_guests.py new file mode 100644 index 0000000..bfa8ad2 --- /dev/null +++ b/backend/migrations/versions/0015_chat_guests.py @@ -0,0 +1,42 @@ +"""chat guests + +Revision ID: 0015 +Revises: 0014 +Create Date: 2026-10-22 15:25:00 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0015" +down_revision: str | Sequence[str] | None = "0014" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Upgrade schema.""" + # Guests in a Project's Chat (spec #91), on top of its Chat people. + op.create_table( + "chat_guests", + sa.Column("project_id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column( + "added_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + 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_guests") diff --git a/backend/src/pmp_backend/api/chat.py b/backend/src/pmp_backend/api/chat.py index d92709c..5114422 100644 --- a/backend/src/pmp_backend/api/chat.py +++ b/backend/src/pmp_backend/api/chat.py @@ -18,6 +18,19 @@ class ChatOut(BaseModel): title: str # Archived Projects' Chats are read-only. read_only: bool + # Teachers and the Superuser add and remove Guests. + can_manage_guests: bool + # The User is a Guest here, so they may leave. + is_guest: bool + + +class ChatPersonOut(PersonOut): + # "Member", "Product Owner" or "Guest". + kind: str + + +class GuestIn(BaseModel): + user_id: int class MessageIn(BaseModel): @@ -69,13 +82,57 @@ class UnreadOut(BaseModel): @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) + read_only = project.status == ProjectStatus.ARCHIVED.value return ChatOut( project_id=project.id, title=project.title, - read_only=project.status == ProjectStatus.ARCHIVED.value, + read_only=read_only, + can_manage_guests=chat.is_staff(user) and not read_only, + is_guest=chat.is_guest(session, user, project) and not read_only, ) +@router.get("/projects/{project_id}/chat/people") +def chat_people( + project_id: int, session: SessionDep, user: CurrentUserDep +) -> list[ChatPersonOut]: + """The Product Owner, the Members, then the Guests.""" + return [ + ChatPersonOut(**PersonOut.of(person.user).model_dump(), kind=person.kind) + for person in chat.people(session, user, project_id) + ] + + +@router.get("/projects/{project_id}/chat/guests/candidates") +def guest_candidates( + project_id: int, session: SessionDep, user: CurrentUserDep +) -> list[PersonOut]: + """Active Users who could join as a Guest (Teachers and the Superuser).""" + return [ + PersonOut.of(candidate) + for candidate in chat.guest_candidates(session, user, project_id) + ] + + +@router.post("/projects/{project_id}/chat/guests", status_code=status.HTTP_201_CREATED) +def add_guest( + project_id: int, body: GuestIn, session: SessionDep, user: CurrentUserDep +) -> PersonOut: + return PersonOut.of(chat.add_guest(session, user, project_id, body.user_id)) + + +@router.delete( + "/projects/{project_id}/chat/guests/{user_id}", + status_code=status.HTTP_204_NO_CONTENT, +) +def remove_guest( + project_id: int, user_id: int, session: SessionDep, user: CurrentUserDep +) -> Response: + """Teachers and the Superuser remove a Guest; a Guest leaves with their own id.""" + chat.remove_guest(session, user, project_id, user_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + @router.get("/projects/{project_id}/chat/messages") def list_messages( project_id: int, @@ -105,8 +162,20 @@ def mark_read( return Response(status_code=status.HTTP_204_NO_CONTENT) +@router.post( + "/projects/{project_id}/chat/leave", status_code=status.HTTP_204_NO_CONTENT +) +def leave_chat(project_id: int, session: SessionDep, user: CurrentUserDep) -> Response: + """A Guest leaves the Chat.""" + chat.remove_guest(session, user, project_id, user.id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + @router.get("/chats") -def list_chats(session: SessionDep, user: CurrentUserDep) -> list[ChatSummaryOut]: +def list_chats( + session: SessionDep, user: CurrentUserDep, mine: bool = False +) -> list[ChatSummaryOut]: + """With `mine`: only the Chats where the User is one of the Chat people.""" return [ ChatSummaryOut( project_id=summary.project_id, @@ -114,7 +183,7 @@ def list_chats(session: SessionDep, user: CurrentUserDep) -> list[ChatSummaryOut unread=summary.unread, mine=summary.mine, ) - for summary in chat.chats(session, user) + for summary in chat.chats(session, user, mine=mine) ] diff --git a/backend/src/pmp_backend/models.py b/backend/src/pmp_backend/models.py index 5e653bb..4b24259 100644 --- a/backend/src/pmp_backend/models.py +++ b/backend/src/pmp_backend/models.py @@ -352,3 +352,22 @@ class ChatRead(Base): ForeignKey("users.id", ondelete="CASCADE"), primary_key=True ) last_read_id: Mapped[int] = mapped_column(default=0) + + +class ChatGuest(Base): + """A Guest in a Project's Chat: any User added by the Product Owner, a + Teacher or the Superuser, on top of the Members and the Product Owner.""" + + __tablename__ = "chat_guests" + + 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 + ) + added_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) + + user: Mapped[User] = relationship(lazy="joined") diff --git a/backend/src/pmp_backend/services/chat.py b/backend/src/pmp_backend/services/chat.py index 6bfcdfb..350db4d 100644 --- a/backend/src/pmp_backend/services/chat.py +++ b/backend/src/pmp_backend/services/chat.py @@ -1,8 +1,8 @@ """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`). +Its Chat people are the Members, the Product Owner and the Guests; 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 @@ -12,9 +12,10 @@ 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.errors import Conflict, Forbidden, NotFound, Refused +from pmp_backend.models import ChatGuest, ChatMessage, ChatRead, Project, User from pmp_backend.services.projects import get_project +from pmp_backend.services.users import get_user MAX_LENGTH = 4000 # The newest messages a Chat opens with; older ones come later (search, #97). @@ -33,27 +34,57 @@ class ChatConflictError(ChatError, Conflict): pass +class ChatNotFoundError(ChatError, NotFound): + 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.""" +def is_team(user: User, project: Project) -> bool: + """A Member or the Product Owner.""" 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 guest_project_ids(session: Session, user: User) -> set[int]: + """The Projects in whose Chat the User is a Guest.""" + return set( + session.scalars( + select(ChatGuest.project_id).where(ChatGuest.user_id == user.id) + ) + ) + + +def is_guest(session: Session, user: User, project: Project) -> bool: + return session.get(ChatGuest, (project.id, user.id)) is not None + + +def is_chat_person(session: Session, user: User, project: Project) -> bool: + """A Member, the Product Owner or a Guest: whom the Chat is for. Guests lose + access when the Project is archived.""" + if is_team(user, project): + return True + active = project.status == ProjectStatus.ACTIVE.value + return active and is_guest(session, 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): + if not (is_staff(user) or is_chat_person(session, user, project)): raise ChatForbiddenError("You don't have access to this chat.") return project +def writable_project(session: Session, user: User, project_id: int) -> Project: + """The Project whose Chat `user` may write in: Archived ones are read-only.""" + 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.") + return project + + def messages( session: Session, user: User, project_id: int, after: int | None = None ) -> list[ChatMessage]: @@ -72,9 +103,7 @@ def messages( 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.") + project = writable_project(session, user, project_id) body = body.strip() if not body: raise ChatError("Write a message first.") @@ -121,13 +150,17 @@ class ChatSummary: mine: bool -def _visible_projects(session: Session, user: User) -> list[Project]: - """Staff see every Active Project's Chat; Students their own Project's.""" +def _visible_projects( + session: Session, user: User, guest_ids: set[int] +) -> list[Project]: + """Staff see every Active Project's Chat; Students their own Project's and + those where they are a Guest.""" query = select(Project).where(Project.status == ProjectStatus.ACTIVE.value) if not is_staff(user): - if user.project_id is None: + own = {user.project_id} if user.project_id is not None else set() + if not own | guest_ids: return [] - query = query.where(Project.id == user.project_id) + query = query.where(Project.id.in_(own | guest_ids)) return list(session.scalars(query.order_by(func.lower(Project.title)))) @@ -155,24 +188,107 @@ def _unread_counts(session: Session, user: User, project_ids: list[int]) -> dict 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) +def chats(session: Session, user: User, *, mine: bool = False) -> list[ChatSummary]: + """The Chats the User sees (with `mine`: only those where they are one of the + Chat people), by title, with their Unread counts.""" + guest_ids = guest_project_ids(session, user) + projects = _visible_projects(session, user, guest_ids) counts = _unread_counts(session, user, [project.id for project in projects]) - return [ + summaries = [ ChatSummary( project.id, project.title, counts.get(project.id, 0), - is_chat_person(user, project), + is_team(user, project) or project.id in guest_ids, ) for project in projects ] + return [summary for summary in summaries if summary.mine or not mine] 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.""" + """The badge: Unread messages in the User's own Chats (for Teachers, those + where they are the Product Owner or a Guest), 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) + + +# Who is in a Chat. + +MEMBER, PRODUCT_OWNER, GUEST = "Member", "Product Owner", "Guest" + + +@dataclass(frozen=True) +class ChatPerson: + user: User + # MEMBER, PRODUCT_OWNER or GUEST. + kind: str + + +def people(session: Session, user: User, project_id: int) -> list[ChatPerson]: + """The Chat people: the Product Owner, the Members, then the Guests.""" + project = readable_project(session, user, project_id) + found = [ChatPerson(project.product_owner, PRODUCT_OWNER)] + found += [ChatPerson(member, MEMBER) for member in project.members] + if project.status == ProjectStatus.ACTIVE.value: + found += [ChatPerson(guest.user, GUEST) for guest in _guests(session, project)] + return found + + +def _guests(session: Session, project: Project) -> list[ChatGuest]: + return list( + session.scalars( + select(ChatGuest) + .join(ChatGuest.user) + .where(ChatGuest.project_id == project.id) + .order_by(User.last_name, User.first_name) + ) + ) + + +def _managed_project(session: Session, actor: User, project_id: int) -> Project: + """The Project whose Guests `actor` may change: staff only, Active only.""" + if not is_staff(actor): + raise ChatForbiddenError( + "Only the Product Owner, Teachers and the Superuser add or remove Guests." + ) + return writable_project(session, actor, project_id) + + +def guest_candidates(session: Session, actor: User, project_id: int) -> list[User]: + """Active Users who could be added as a Guest: not yet one of the Chat people.""" + project = _managed_project(session, actor, project_id) + inside = {person.user.id for person in people(session, actor, project.id)} + users = session.scalars( + select(User) + .where(User.is_active.is_(True)) + .order_by(User.last_name, User.first_name) + ) + return [user for user in users if user.id not in inside] + + +def add_guest(session: Session, actor: User, project_id: int, user_id: int) -> User: + project = _managed_project(session, actor, project_id) + guest = get_user(session, user_id) + if not guest.is_active: + raise ChatError(f"{guest.full_name} is deactivated, so can't join a chat.") + if is_team(guest, project) or is_guest(session, guest, project): + raise ChatConflictError(f"{guest.full_name} is already in this chat.") + session.add(ChatGuest(project_id=project.id, user_id=guest.id)) + session.commit() + return guest + + +def remove_guest(session: Session, actor: User, project_id: int, user_id: int) -> None: + """Staff remove a Guest; a Guest may also leave the Chat themselves.""" + if actor.id == user_id: + project = writable_project(session, actor, project_id) + else: + project = _managed_project(session, actor, project_id) + guest = session.get(ChatGuest, (project.id, user_id)) + if guest is None: + raise ChatNotFoundError("This person is not a Guest in this chat.") + session.delete(guest) + session.commit() diff --git a/backend/tests/test_chat.py b/backend/tests/test_chat.py index 6047399..fddbcb7 100644 --- a/backend/tests/test_chat.py +++ b/backend/tests/test_chat.py @@ -77,7 +77,13 @@ def test_the_team_and_staff_talk_in_the_project_chat(client: TestClient): "Good luck!", ] chat = client.get(f"/api/projects/{drone}/chat", headers=student).json() - assert chat == {"project_id": drone, "title": "Drone", "read_only": False} + assert chat == { + "project_id": drone, + "title": "Drone", + "read_only": False, + "can_manage_guests": False, + "is_guest": False, + } def test_other_students_have_no_access(client: TestClient): diff --git a/backend/tests/test_chat_guests.py b/backend/tests/test_chat_guests.py new file mode 100644 index 0000000..a2fd354 --- /dev/null +++ b/backend/tests/test_chat_guests.py @@ -0,0 +1,166 @@ +from fastapi.testclient import TestClient + +from tests.conftest import add_member, create_project, ready_to_work +from tests.test_chat import logged_in_student, send, student_id, unread + + +def guests_url(project_id: int) -> str: + return f"/api/projects/{project_id}/chat/guests" + + +def people(client: TestClient, headers, project_id: int) -> list[tuple[str, str]]: + response = client.get(f"/api/projects/{project_id}/chat/people", headers=headers) + assert response.status_code == 200, response.text + return [(person["name"], person["kind"]) for person in response.json()] + + +def user_id(client: TestClient, superuser, name: str) -> int: + users = client.get("/api/users", headers=superuser).json() + return next(u["id"] for u in users if f"{u['first_name']} {u['last_name']}" == name) + + +def test_staff_add_a_guest_who_then_reads_writes_and_leaves(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)) + lotte = logged_in_student(client, superuser, "Lotte") + lotte_id = user_id(client, superuser, "Lotte Peeters") + send(client, teacher, drone, "Before Lotte joined") + + candidates = client.get(f"{guests_url(drone)}/candidates", headers=teacher).json() + added = client.post(guests_url(drone), json={"user_id": lotte_id}, headers=teacher) + + # By last name; the Product Owner and the Member are already in. + assert [c["name"] for c in candidates] == ["Superuser Account", "Lotte Peeters"] + assert added.status_code == 201 + assert people(client, lotte, drone) == [ + ("Teacher Account", "Product Owner"), + ("Student Account", "Member"), + ("Lotte Peeters", "Guest"), + ] + chat = client.get(f"/api/projects/{drone}/chat", headers=lotte).json() + assert chat["is_guest"] is True and chat["can_manage_guests"] is False + # A Guest sees the whole history and counts the chat as theirs. + assert unread(client, lotte) == {"count": 1, "has_chat": True} + assert send(client, lotte, drone, "Hi, I'm helping out").status_code == 201 + + left = client.delete(f"{guests_url(drone)}/{lotte_id}", headers=lotte) + + assert left.status_code == 204 + assert send(client, lotte, drone, "Still here?").status_code == 403 + assert unread(client, lotte) == {"count": 0, "has_chat": False} + + +def test_members_cannot_add_or_remove_guests(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") + logged_in_student(client, superuser, "Lotte") + lotte_id = user_id(client, superuser, "Lotte Peeters") + + refused = client.post( + guests_url(drone), json={"user_id": lotte_id}, headers=student + ) + client.post(guests_url(drone), json={"user_id": lotte_id}, headers=superuser) + removal = client.delete(f"{guests_url(drone)}/{lotte_id}", headers=student) + + assert refused.status_code == 403 + assert refused.json()["detail"] == ( + "Only the Product Owner, Teachers and the Superuser add or remove Guests." + ) + assert removal.status_code == 403 + removed = client.delete(f"{guests_url(drone)}/{lotte_id}", headers=superuser) + assert removed.status_code == 204 + assert ("Lotte Peeters", "Guest") not in people(client, teacher, drone) + + +def test_who_cannot_be_added(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + member = student_id(client, teacher) + add_member(client, teacher, drone, member) + logged_in_student(client, superuser, "Lotte") + lotte_id = user_id(client, superuser, "Lotte Peeters") + client.post(f"/api/users/{lotte_id}/deactivate", headers=superuser) + + as_member = client.post( + guests_url(drone), json={"user_id": member}, headers=teacher + ) + deactivated = client.post( + guests_url(drone), json={"user_id": lotte_id}, headers=teacher + ) + teacher_id = user_id(client, superuser, "Teacher Account") + product_owner = client.post( + guests_url(drone), json={"user_id": teacher_id}, headers=superuser + ) + + assert as_member.status_code == 409 + assert as_member.json()["detail"] == "Student Account is already in this chat." + assert deactivated.status_code == 422 + assert product_owner.status_code == 409 + + +def test_a_teacher_counts_guest_chats_as_theirs(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + robot = create_project(client, teacher, "Robot") + superuser_id = user_id(client, superuser, "Superuser Account") + send(client, teacher, drone, "For the team") + send(client, teacher, robot, "Also for a team") + + before = client.get("/api/chats", params={"mine": True}, headers=superuser).json() + client.post(guests_url(drone), json={"user_id": superuser_id}, headers=teacher) + after = client.get("/api/chats", params={"mine": True}, headers=superuser).json() + + assert before == [] + assert [(c["title"], c["unread"]) for c in after] == [("Drone", 1)] + assert unread(client, superuser)["count"] == 1 + # Without the filter, every chat is still listed. + assert len(client.get("/api/chats", headers=superuser).json()) == 2 + + +def test_a_student_guest_without_a_project_has_a_chat(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + robot = create_project(client, teacher, "Robot") + add_member(client, teacher, robot, student_id(client, teacher)) + student = ready_to_work(client, "student") + lotte = logged_in_student(client, superuser, "Lotte") + lotte_id = user_id(client, superuser, "Lotte Peeters") + student_user = student_id(client, teacher) + + assert unread(client, lotte)["has_chat"] is False + client.post(guests_url(drone), json={"user_id": lotte_id}, headers=teacher) + client.post(guests_url(drone), json={"user_id": student_user}, headers=teacher) + + assert [c["title"] for c in client.get("/api/chats", headers=lotte).json()] == [ + "Drone" + ] + # A Member of Robot who is also a Guest in Drone has both chats. + assert [c["title"] for c in client.get("/api/chats", headers=student).json()] == [ + "Drone", + "Robot", + ] + + +def test_archiving_ends_guest_access(client: TestClient): + teacher = ready_to_work(client, "teacher") + superuser = ready_to_work(client, "superuser") + drone = create_project(client, teacher, "Drone") + lotte = logged_in_student(client, superuser, "Lotte") + lotte_id = user_id(client, superuser, "Lotte Peeters") + client.post(guests_url(drone), json={"user_id": lotte_id}, headers=teacher) + + client.post(f"/api/projects/{drone}/archive", headers=teacher) + + response = client.get(f"/api/projects/{drone}/chat/messages", headers=lotte) + assert response.status_code == 403 + assert people(client, teacher, drone) == [("Teacher Account", "Product Owner")] + added = client.post(guests_url(drone), json={"user_id": lotte_id}, headers=teacher) + assert added.status_code == 409 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c564dc6..5a82ddb 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -243,6 +243,7 @@ Grilling Q1–Q22 with the team, spec #91 and tickets #92–#98. Built one ticke |---|---|---|---| | 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 | | Fixed | The restore test after a Deploy failed when the site wrote meanwhile (first seen after #99, with the new chat in use): it now compares with the row counts taken just before and after the dump, and backs up again (max 3 times) when they differ | – | #100 | +| New | **Chat Guests:** the Product Owner, a Teacher or the Superuser adds any active User to a chat (People list, Add Guest, Remove); Guests can leave; "Only my chats" on the Chats screen (on by default); a Teacher's badge also counts chats where they are a Guest; Students in several chats get a list; migration `0015` | #93 | #101 | ### 2026-10-22: team change requests diff --git a/frontend/src/pmp_frontend/api.py b/frontend/src/pmp_frontend/api.py index 02f0276..a274c53 100644 --- a/frontend/src/pmp_frontend/api.py +++ b/frontend/src/pmp_frontend/api.py @@ -697,12 +697,47 @@ async def mark_chat_read(token: str, project_id: int, message_id: int) -> None: ) -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 chats(token: str, *, mine: bool = False) -> list[dict[str, Any]]: + """The Chats the User sees (with `mine`: only those where they are one of the + Chat people): project_id, title, unread, mine.""" + query = "?mine=true" if mine else "" + return await _request("GET", f"/api/chats{query}", 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) + + +async def chat_people(token: str, project_id: int) -> list[dict[str, Any]]: + """The Product Owner, the Members, then the Guests: id, name, kind.""" + return await _request("GET", f"/api/projects/{project_id}/chat/people", token=token) + + +async def chat_guest_candidates(token: str, project_id: int) -> list[dict[str, Any]]: + """Active Users who could join the Chat as a Guest: id, name.""" + return await _request( + "GET", f"/api/projects/{project_id}/chat/guests/candidates", token=token + ) + + +async def add_chat_guest(token: str, project_id: int, user_id: int) -> dict[str, Any]: + return await _request( + "POST", + f"/api/projects/{project_id}/chat/guests", + token=token, + json={"user_id": user_id}, + ) + + +async def remove_chat_guest(token: str, project_id: int, user_id: int) -> None: + """Remove a Guest; with your own id, leave the Chat.""" + await _request( + "DELETE", f"/api/projects/{project_id}/chat/guests/{user_id}", token=token + ) + + +async def leave_chat(token: str, project_id: int) -> None: + """A Guest leaves the Chat.""" + await _request("POST", f"/api/projects/{project_id}/chat/leave", token=token) diff --git a/frontend/src/pmp_frontend/chat_page.py b/frontend/src/pmp_frontend/chat_page.py index cbe9ab9..980b01b 100644 --- a/frontend/src/pmp_frontend/chat_page.py +++ b/frontend/src/pmp_frontend/chat_page.py @@ -10,7 +10,14 @@ 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.shell import ( + ErrorMessage, + confirm, + guard, + page_frame, + role_page, + token, +) from pmp_frontend.times import shown STAFF = {"superuser", "teacher"} @@ -120,7 +127,14 @@ async def send() -> None: box.value = "" show([sent]) - ui.label(f"Chat: {chat['title']}").classes("text-h4").mark("chat-title") + with ui.row().classes("w-full items-center"): + ui.label(f"Chat: {chat['title']}").classes("text-h4").mark("chat-title") + ui.space() + if chat["is_guest"]: + ui.button( + "Leave chat", icon="logout", on_click=lambda: _leave(project_id) + ).props("outline color=dark").mark("leave-chat") + await _people(project_id, chat) with ui.card().classes("w-full p-0"): scroll = ui.scroll_area().classes("w-full h-[60vh]").mark("chat-messages") with scroll: @@ -147,30 +161,150 @@ async def send() -> None: ui.timer(POLL_SECONDS, poll) +async def _leave(project_id: int) -> None: + question = "Leave this chat? You can only come back when someone adds you again." + if not await confirm(question, "Leave"): + return + try: + await api.leave_chat(token(), project_id) + except api.ApiError as error: + ui.notify(error.message, type="negative") + return + ui.notify("You left the chat.") + ui.navigate.to("/") + + +async def _people(project_id: int, chat: dict[str, Any]) -> None: + """Who is in the Chat (Product Owner, Members, Guests); staff add and remove + Guests here.""" + manage = chat["can_manage_guests"] + + async def remove(person: dict[str, Any]) -> None: + if not await confirm(f"Remove {person['name']} from this chat?", "Remove"): + return + try: + await api.remove_chat_guest(token(), project_id, person["id"]) + except api.ApiError as error: + ui.notify(error.message, type="negative") + return + ui.notify(f"{person['name']} was removed from the chat.") + people_list.refresh() + + async def open_add_dialog() -> None: + candidates = await api.chat_guest_candidates(token(), project_id) + + async def add() -> None: + if choice.value is None: + error.show("Choose a person first.") + return + try: + added = await api.add_chat_guest(token(), project_id, choice.value) + except api.ApiError as problem: + error.show(problem.message) + return + dialog.close() + ui.notify(f"{added['name']} joined the chat as a Guest.") + people_list.refresh() + + with ui.dialog() as dialog, ui.card().classes("w-full max-w-md"): + ui.label("Add a Guest").classes("text-h6") + ui.label( + "Guests read the whole chat and write like the team, until they " + "leave or are removed." + ).classes("text-caption") + choice = ( + ui.select( + {person["id"]: person["name"] for person in candidates}, + label="Person", + with_input=True, + ) + .classes("w-full") + .mark("guest-choice") + ) + error = ErrorMessage() + with ui.row(): + ui.button("Add", on_click=add).mark("confirm-add-guest") + ui.button("Cancel", on_click=dialog.close).props("flat color=dark") + dialog.open() + + @ui.refreshable + async def people_list() -> None: + try: + found = await api.chat_people(token(), project_id) + except api.ApiError as error: + ui.label(error.message) + return + with ( + ui.expansion(f"People ({len(found)})", icon="group") + .classes("w-full bg-white rounded-lg") + .mark("chat-people") + ): + for person in found: + with ui.row().classes("w-full items-center"): + ui.label(person["name"]).mark(f"person-{person['id']}") + ui.label(person["kind"]).classes("text-caption").style( + "color: #666666" + ) + ui.space() + if manage and person["kind"] == "Guest": + ui.button("Remove", on_click=lambda p=person: remove(p)).props( + "flat dense color=dark" + ).mark(f"remove-guest-{person['id']}") + if manage: + ui.button( + "Add Guest", icon="person_add", on_click=open_add_dialog + ).props("outline color=dark").mark("add-guest") + + await people_list() + + +def _chats_table(found: list[dict[str, Any]]) -> None: + """Chats by title with their Unread count; a row opens its Chat.""" + 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']}"), + ) + + def register() -> None: @role_page("/chat", "Chat", {"student"}) async def student_chat(user: api.CurrentUser) -> None: found = await api.chats(token()) + if len(found) == 1: + await chat_view(found[0]["project_id"]) + return + ui.label("Chat").classes("text-h4") 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"]) + # Their own Project's Chat and those where they are a Guest. + _chats_table(found) @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']}"), - ) + only_mine = ui.switch( + "Only my chats", value=True, on_change=lambda: listed.refresh() + ).mark("only-my-chats") + + @ui.refreshable + async def listed() -> None: + found = await api.chats(token(), mine=only_mine.value) + if found: + _chats_table(found) + elif only_mine.value: + ui.label( + "You're not in any chat yet: Projects where you are the Product " + 'Owner or a Guest appear here. Turn off "Only my chats" to see ' + "every chat." + ) + else: + ui.label("No Active Projects, so no chats yet.") + + await listed() @ui.page("/chats/{project_id}") async def chat_page(project_id: int) -> Response | None: diff --git a/frontend/tests/test_chat_guests.py b/frontend/tests/test_chat_guests.py new file mode 100644 index 0000000..3537bc5 --- /dev/null +++ b/frontend/tests/test_chat_guests.py @@ -0,0 +1,106 @@ +from nicegui import ui +from nicegui.testing import User + +from pmp_frontend import api +from tests.conftest import eventually, ready_to_work +from tests.test_chat import api_token, found, navigation_targets + + +async def project_owned_by_the_teacher(token: str, title: str) -> int: + 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": title, + "description": None, + "product_owner_id": owner, + "team_size_min": 1, + "team_size_max": 3, + }, + ) + return project["id"] + + +async def the_student(token: str) -> int: + students = await api.students(token) + return next(s["id"] for s in students if s["name"] == "Student Account") + + +async def test_a_teacher_adds_and_removes_a_guest(user: User): + superuser = await api_token("superuser") + drone = await project_owned_by_the_teacher(superuser, "Drone") + student = await the_student(superuser) + await ready_to_work(user, "teacher") + + await user.open(f"/chats/{drone}") + await user.should_see(marker="add-guest", retries=30) + user.find(marker="add-guest").click() + await user.should_see(marker="guest-choice", retries=30) + choice = user.find(marker="guest-choice").elements.pop() + assert isinstance(choice, ui.select) + choice.set_value(student) + user.find(marker="confirm-add-guest").click() + + await user.should_see("Student Account joined the chat as a Guest.", retries=30) + await user.should_see(marker=f"remove-guest-{student}", retries=30) + await user.should_see("People (2)") + + user.find(marker=f"remove-guest-{student}").click() + await user.should_see("Remove Student Account from this chat?", retries=10) + user.find(marker="confirm").click() + await user.should_see("Student Account was removed from the chat.", retries=30) + await user.should_see("People (1)", retries=30) + + +async def test_a_student_guest_chats_and_leaves(user: User): + superuser = await api_token("superuser") + drone = await project_owned_by_the_teacher(superuser, "Drone") + await api.add_chat_guest(superuser, drone, await the_student(superuser)) + await ready_to_work(user, "student") + + # A Guest without a Project still gets the Chat item, straight into the chat. + await eventually(lambda: "/chat" in navigation_targets(user), retries=100) + await user.open("/chat") + await user.should_see("Chat: Drone", retries=30) + assert not found(user, "add-guest") + + user.find(marker="leave-chat").click() + await user.should_see("Leave this chat?", retries=10) + user.find(marker="confirm").click() + + await user.should_see("You left the chat.", retries=30) + assert await api.chats(superuser) == [ + {"project_id": drone, "title": "Drone", "unread": 0, "mine": False} + ] + + +async def test_only_my_chats_is_on_by_default(user: User): + teacher = await api_token("teacher") + await project_owned_by_the_teacher(teacher, "Drone") + await ready_to_work(user, "superuser") + + await user.open("/chats") + await user.should_see("You're not in any chat yet", retries=30) + switch = user.find(marker="only-my-chats").elements.pop() + assert isinstance(switch, ui.switch) + switch.set_value(False) + + await user.should_see(marker="chats", retries=30) + table = user.find(marker="chats").elements.pop() + assert [row["title"] for row in table.rows] == ["Drone"] + + +async def test_a_student_in_two_chats_chooses_one(user: User): + superuser = await api_token("superuser") + drone = await project_owned_by_the_teacher(superuser, "Drone") + robot = await project_owned_by_the_teacher(superuser, "Robot") + student = await the_student(superuser) + await api.add_member(superuser, robot, student) + await api.add_chat_guest(superuser, drone, student) + await ready_to_work(user, "student") + + await user.open("/chat") + await user.should_see(marker="chats", retries=30) + table = user.find(marker="chats").elements.pop() + assert [row["title"] for row in table.rows] == ["Drone", "Robot"]