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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .claude/rules/backend/models.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Two responsibilities: define fields, and define properties for convenient access

**Encode identity with one discriminant; derive the rest.** Prefer a single `kind` (or similar enum) over parallel flags or “has related row” checks (`hasattr(user, "bot_profile")`). Convenience APIs are `@property` methods on the model (`is_bot`).

**`Member` is a participant in a game, not necessarily a player.** `Member.kind` discriminates the roles; a game master holds a member row with no nation, no phase state, and no seat against the variant's nation count. Anything that means *player* — seat counting, nation assignment, phase states, draws, victory, abandonment — must go through `Member.objects.players()` / `game.members.players()`, and a new query over `game.members` has to decide which it means. Do not reintroduce a second source of truth for the role by re-deriving it from `Game.game_master`.

**Do not leave unused domain in the schema for a future feature.** If fields are not used yet, remove them and re-add when the feature lands. Absent beats switched-off scaffolding.

Query optimisation belongs on a custom QuerySet, never in a view:
Expand Down
4 changes: 4 additions & 0 deletions .claude/rules/backend/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ class IsGameMaster(BasePermission):
return member.is_game_master
```

## Player or participant

Permission names say which kind of member they admit: a `…GamePlayer` class admits only members holding a seat, a `…GameParticipant` class admits any member row, so a non-playing game master passes. Reach for the participant variant only for capabilities a game master genuinely shares with players — reading and posting in public press — and the player variant for everything tied to a nation. A permission that lets a game master submit orders or claim a seat is a bug, not a generous default.

## Permissions vs validation

Permissions answer questions about the resource being acted on and the identity of the requester — game status, membership, ownership, mode. They must not need request data. Validation that depends on the payload belongs in the serializer.
Expand Down
1 change: 1 addition & 0 deletions packages/web/src/api/generated/endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export interface ChannelMember {
/** @nullable */
readonly commitment: string | null;
nation: Nation | null;
readonly isGameMaster: boolean;
}

export interface ChannelMessage {
Expand Down
1 change: 1 addition & 0 deletions packages/web/src/mocks/fixtures/builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ export const makeMessage = (
commitment: sender.commitment,
isBot: sender.isBot,
nation: nation((sender.nation ?? "england").toLowerCase()),
isGameMaster: false,
},
createdAt,
});
Expand Down
4 changes: 2 additions & 2 deletions service/channel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def get_queryset(self):
return ChannelQuerySet(self.model, using=self._db)

def create_from_member_ids(self, user, member_ids, game):
member_ids = member_ids + [game.members.get(user=user).id]
channel_members = game.members.filter(id__in=member_ids)
member_ids = member_ids + [game.members.players().get(user=user).id]
channel_members = game.members.players().filter(id__in=member_ids)
nations = sorted([m.nation.name for m in channel_members])
channel_name = ", ".join(nations)
channel = self.create(name=channel_name, private=True, game=game)
Expand Down
3 changes: 2 additions & 1 deletion service/channel/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

class ChannelMemberSerializer(BaseMemberSerializer):
nation = NationSerializer(allow_null=True)
is_game_master = serializers.BooleanField(read_only=True)


class ChannelMessageSerializer(serializers.Serializer):
Expand Down Expand Up @@ -57,7 +58,7 @@ def validate_member_ids(self, value):
current_member = self.context["current_game_member"]

member_ids = value + [current_member.id]
channel_members = game.members.filter(id__in=member_ids)
channel_members = game.members.players().filter(id__in=member_ids)

if channel_members.count() != len(member_ids):
raise serializers.ValidationError("One or more members are not part of the game.")
Expand Down
92 changes: 91 additions & 1 deletion service/channel/tests/test_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.utils import timezone
from rest_framework import status
from rest_framework.test import APIRequestFactory
from channel.models import Channel, ChannelMessage
from channel.models import Channel, ChannelMember, ChannelMessage
from nation.models import Nation
from game.models import Game
from game.serializers import GameRetrieveSerializer
Expand Down Expand Up @@ -113,6 +113,19 @@ def test_create_channel_non_member(
response = authenticated_client_for_secondary_user.post(url, payload, format="json")
assert response.status_code == status.HTTP_403_FORBIDDEN

@pytest.mark.django_db
def test_game_master_cannot_create_private_channel(
self, authenticated_client, active_game_with_game_master_factory
):
game = active_game_with_game_master_factory()
player = game.members.players().first()

url = reverse("channel-create", args=[game.id])
response = authenticated_client.post(url, {"member_ids": [player.id]}, format="json")

assert response.status_code == status.HTTP_403_FORBIDDEN
assert not Channel.objects.filter(game=game, private=True).exists()

@pytest.mark.django_db
def test_create_channel_sandbox_game_forbidden(self, authenticated_client, sandbox_game_factory):
game = sandbox_game_factory()
Expand Down Expand Up @@ -329,6 +342,51 @@ def test_create_message_in_public_channel_without_explicit_members(
assert len(recipient_ids) == 1
assert sender_user_id not in recipient_ids

@pytest.mark.django_db
def test_game_master_can_post_in_public_press(
self, authenticated_client, active_game_with_game_master_factory, in_memory_procrastinate
):
game = active_game_with_game_master_factory()
channel = Channel.objects.get(game=game, private=False)

url = reverse("channel-message-create", args=[game.id, channel.id])
response = authenticated_client.post(url, {"body": "Deadline moves to Friday."}, format="json")

assert response.status_code == status.HTTP_201_CREATED
assert response.data["sender"]["is_game_master"] is True
assert response.data["sender"]["nation"] is None

@pytest.mark.django_db
def test_game_master_cannot_post_in_private_channel(
self, authenticated_client, active_game_with_game_master_factory, in_memory_procrastinate
):
game = active_game_with_game_master_factory()
players = list(game.members.players()[:2])
channel = Channel.objects.create(game=game, name="Private", private=True)
channel.members.set(players)

url = reverse("channel-message-create", args=[game.id, channel.id])
response = authenticated_client.post(url, {"body": "Let me in"}, format="json")

assert response.status_code == status.HTTP_403_FORBIDDEN
assert not ChannelMessage.objects.filter(channel=channel).exists()

@pytest.mark.django_db
def test_public_press_message_notifies_game_master(
self, api_client, active_game_with_game_master_factory, primary_user, in_memory_procrastinate
):
game = active_game_with_game_master_factory()
channel = Channel.objects.get(game=game, private=False)
sender = game.members.players().first()
api_client.force_authenticate(user=sender.user)

url = reverse("channel-message-create", args=[game.id, channel.id])
response = api_client.post(url, {"body": "Anyone want Munich?"}, format="json")

assert response.status_code == status.HTTP_201_CREATED
recipient_ids = list(_channel_message_notifications().values_list("recipient_id", flat=True))
assert primary_user.id in recipient_ids

@pytest.mark.django_db
def test_create_message_in_private_channel_notifies_only_channel_members(
self,
Expand Down Expand Up @@ -539,6 +597,22 @@ def test_mark_read_success(self, authenticated_client, game_with_public_channel_
channel_member.refresh_from_db()
assert channel_member.last_read_at > original_last_read_at

@pytest.mark.django_db
def test_game_master_can_mark_public_press_read(
self, authenticated_client, active_game_with_game_master_factory, primary_user
):
game = active_game_with_game_master_factory()
channel = Channel.objects.get(game=game, private=False)
channel_member = ChannelMember.objects.get(member__user=primary_user, channel=channel)
original_last_read_at = channel_member.last_read_at

url = reverse("channel-mark-read", args=[game.id, channel.id])
response = authenticated_client.post(url)

assert response.status_code == status.HTTP_204_NO_CONTENT
channel_member.refresh_from_db()
assert channel_member.last_read_at > original_last_read_at

@pytest.mark.django_db
def test_mark_read_unauthenticated(self, unauthenticated_client, game_with_public_channel_and_messages):
game = game_with_public_channel_and_messages
Expand Down Expand Up @@ -652,6 +726,22 @@ def test_channel_list_includes_unread_count(self, authenticated_client, game_wit
channel_data = next(ch for ch in response.data if ch["name"] == "Public Press")
assert channel_data["unread_message_count"] == 2

@pytest.mark.django_db
def test_game_master_sees_unread_count_in_public_press(
self, authenticated_client, active_game_with_game_master_factory
):
game = active_game_with_game_master_factory()
channel = Channel.objects.get(game=game, private=False)
ChannelMessage.objects.create(
channel=channel, sender=game.members.players().first(), body="Orders are in"
)

url = reverse("channel-list", args=[game.id])
response = authenticated_client.get(url)

channel_data = next(c for c in response.data if c["id"] == channel.id)
assert channel_data["unread_message_count"] == 1

@pytest.mark.django_db
def test_unread_count_resets_after_mark_read(self, authenticated_client, game_with_public_channel_and_messages):
game = game_with_public_channel_and_messages
Expand Down
8 changes: 4 additions & 4 deletions service/channel/views.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
from rest_framework import permissions, generics, status
from rest_framework.response import Response
from common.permissions import IsActiveOrCompletedGame, IsGameMember, IsChannelMember, IsNotKickedGameMember, IsNotSandboxGame, IsNotNoPressActiveGame
from common.permissions import IsActiveOrCompletedGame, IsGameParticipant, IsChannelMember, IsNotKickedGamePlayer, IsNotKickedGameParticipant, IsNotSandboxGame, IsNotNoPressActiveGame

from .models import Channel
from .serializers import ChannelSerializer, ChannelMessageSerializer, ChannelMarkReadSerializer
from common.views import SelectedGameMixin, SelectedChannelMixin, CurrentGameMemberMixin


class ChannelCreateView(SelectedGameMixin, CurrentGameMemberMixin, generics.CreateAPIView):
permission_classes = [permissions.IsAuthenticated, IsActiveOrCompletedGame, IsNotKickedGameMember, IsNotSandboxGame, IsNotNoPressActiveGame]
permission_classes = [permissions.IsAuthenticated, IsActiveOrCompletedGame, IsNotKickedGamePlayer, IsNotSandboxGame, IsNotNoPressActiveGame]
serializer_class = ChannelSerializer


class ChannelMessageCreateView(SelectedGameMixin, SelectedChannelMixin, CurrentGameMemberMixin, generics.CreateAPIView):
permission_classes = [permissions.IsAuthenticated, IsNotKickedGameMember, IsChannelMember, IsNotSandboxGame, IsNotNoPressActiveGame]
permission_classes = [permissions.IsAuthenticated, IsNotKickedGameParticipant, IsChannelMember, IsNotSandboxGame, IsNotNoPressActiveGame]
serializer_class = ChannelMessageSerializer


class ChannelMarkReadView(SelectedGameMixin, SelectedChannelMixin, CurrentGameMemberMixin, generics.CreateAPIView):
permission_classes = [permissions.IsAuthenticated, IsGameMember, IsChannelMember]
permission_classes = [permissions.IsAuthenticated, IsGameParticipant, IsChannelMember]
serializer_class = ChannelMarkReadSerializer

def create(self, request, *args, **kwargs):
Expand Down
10 changes: 10 additions & 0 deletions service/common/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,16 @@ class UserKind:
BOT_KINDS = (LLM, DUMBBOT)


class MemberKind:
PLAYER = "player"
GAME_MASTER = "game_master"

KIND_CHOICES = (
(PLAYER, "Player"),
(GAME_MASTER, "Game Master"),
)


class CommitmentRequirement:
OPEN = "open"
COMMITTED = "committed"
Expand Down
44 changes: 33 additions & 11 deletions service/common/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,37 @@ def has_permission(self, request, view):
return game.status in (GameStatus.ACTIVE, GameStatus.COMPLETED, GameStatus.ABANDONED)


class IsGameMember(BasePermission):
class IsGamePlayer(BasePermission):
message = "User is not a player in the game."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
return game.members.players().filter(user=request.user).exists()


class IsGameParticipant(BasePermission):
message = "User is not a member of the game."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
return game.members.filter(user=request.user).exists()


class IsNotKickedGameMember(BasePermission):
class IsNotKickedGamePlayer(BasePermission):
message = "User is not a player in the game."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
member = game.members.players().filter(user=request.user).first()
if not member:
return False
if member.kicked:
self.message = "Cannot perform action for kicked players."
return False
return True


class IsNotKickedGameParticipant(BasePermission):
message = "User is not a member of the game."

def has_permission(self, request, view):
Expand All @@ -50,15 +72,15 @@ def has_permission(self, request, view):
return True


class IsActiveGameMember(BasePermission):
class IsActiveGamePlayer(BasePermission):
message = "User cannot perform this action."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))

member = game.members.filter(user=request.user).first()
member = game.members.players().filter(user=request.user).first()
if not member:
self.message = "User is not a member of the game."
self.message = "User is not a player in the game."
return False

if member.eliminated:
Expand Down Expand Up @@ -116,20 +138,20 @@ def has_permission(self, request, view):
return game.status in (GameStatus.PENDING, GameStatus.MUSTERING, GameStatus.ACTIVE)


class IsNotGameMember(BasePermission):
class IsNotGamePlayer(BasePermission):
message = "User is already a member of the game."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
return not game.members.filter(user=request.user).exists()
return not game.members.players().filter(user=request.user).exists()


class IsSpaceAvailable(BasePermission):
message = "Game already has the maximum number of players."

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
return game.members.count() < game.variant.nations.filter(non_playable=False).count()
return game.members.players().count() < game.variant.nations.filter(non_playable=False).count()


class MeetsReliabilityRequirement(BasePermission):
Expand Down Expand Up @@ -173,7 +195,7 @@ class IsUserPhaseStateExists(BasePermission):

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
member = game.members.filter(user=request.user).first()
member = game.members.players().filter(user=request.user).first()
if not member:
return False
current_phase = game.phases.last()
Expand Down Expand Up @@ -275,9 +297,9 @@ class IsInCivilDisorder(BasePermission):

def has_permission(self, request, view):
game = resolve_game(request, view.kwargs.get("game_id"))
member = game.members.filter(user=request.user).first()
member = game.members.players().filter(user=request.user).first()
if not member:
self.message = "User is not a member of the game."
self.message = "User is not a player in the game."
return False
return member.civil_disorder

Expand Down
3 changes: 3 additions & 0 deletions service/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from common.constants import (
DeadlineMode,
GameStatus,
MemberKind,
MovementPhaseDuration,
OrderType,
PhaseFrequency,
Expand Down Expand Up @@ -2193,6 +2194,7 @@ def _create(game_master=None):
admin=game_master,
)
game.channels.create(name="Public Press", private=False)
game.seat(game_master, kind=MemberKind.GAME_MASTER)
return game

return _create
Expand All @@ -2214,6 +2216,7 @@ def _create(game_master=None):
admin=game_master,
)
game.channels.create(name="Public Press", private=False)
game.seat(game_master, kind=MemberKind.GAME_MASTER)

for i in range(game.variant.nations.count()):
other_user = User.objects.create_user(f"gm_player{i}@test.com", password="testpass")
Expand Down
2 changes: 1 addition & 1 deletion service/draw_proposal/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def with_related_data(self):
def create_proposal(self, game, created_by):
phase = game.current_phase

all_active_members = list(game.members.filter(
all_active_members = list(game.members.players().filter(
eliminated=False, kicked=False
))

Expand Down
Loading