feat: switch admin identification from user_id to username
- Replace admin_user_ids (list[int]) with admin_usernames (list[str]) - Update all service methods to use username for permission checks - Add delete button to bot responses for message cleanup - Update tests to match new implementation Note: Breaking change - existing data files need fresh start
This commit is contained in:
@@ -15,32 +15,34 @@ class TestBountyService:
|
||||
"""Set up fresh storage and service for each test."""
|
||||
self.storage = MockRoomStorage()
|
||||
self.service = BountyService(self.storage)
|
||||
self.admin_user_id = 123
|
||||
self._make_admin(-1001, self.admin_user_id)
|
||||
self.admin_username = "admin"
|
||||
self._make_admin(-1001, self.admin_username)
|
||||
|
||||
def _make_admin(self, room_id: int, user_id: int):
|
||||
def _make_admin(self, room_id: int, username: str):
|
||||
"""Helper to set up a room with an admin user."""
|
||||
room_data = self.storage.load(room_id)
|
||||
if room_data is None:
|
||||
room_data = RoomData(
|
||||
room_id=room_id, bounties=[], next_id=0, admin_user_ids=[]
|
||||
room_id=room_id, bounties=[], next_id=0, admin_usernames=[]
|
||||
)
|
||||
if user_id not in (room_data.admin_user_ids or []):
|
||||
room_data.admin_user_ids = room_data.admin_user_ids or []
|
||||
room_data.admin_user_ids.append(user_id)
|
||||
if username not in (room_data.admin_usernames or []):
|
||||
room_data.admin_usernames = room_data.admin_usernames or []
|
||||
room_data.admin_usernames.append(username)
|
||||
self.storage.save(room_data)
|
||||
|
||||
def test_add_bounty_creates_room_if_not_exists(self):
|
||||
"""Test that add_bounty creates a new room if it doesn't exist."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001,
|
||||
user_id=self.admin_user_id,
|
||||
user_id=123,
|
||||
username=self.admin_username,
|
||||
text="Fix bug",
|
||||
link="https://github.com/issue/1",
|
||||
created_by_username=self.admin_username,
|
||||
)
|
||||
assert bounty.id == 1
|
||||
assert bounty.text == "Fix bug"
|
||||
assert bounty.created_by_user_id == self.admin_user_id
|
||||
assert bounty.created_by_user_id == 123
|
||||
|
||||
room = self.storage.load(-1001)
|
||||
assert room is not None
|
||||
@@ -49,13 +51,13 @@ class TestBountyService:
|
||||
def test_add_bounty_increments_id(self):
|
||||
"""Test that add_bounty increments bounty ID for each new bounty."""
|
||||
b1 = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="First"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="First"
|
||||
)
|
||||
b2 = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Second"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Second"
|
||||
)
|
||||
b3 = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Third"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Third"
|
||||
)
|
||||
|
||||
assert b1.id == 1
|
||||
@@ -65,7 +67,9 @@ class TestBountyService:
|
||||
def test_add_bounty_requires_admin(self):
|
||||
"""Test that add_bounty raises PermissionError when non-admin tries to add."""
|
||||
with pytest.raises(PermissionError, match="Only admins can add bounties"):
|
||||
self.service.add_bounty(room_id=-1001, user_id=999, text="Not admin")
|
||||
self.service.add_bounty(
|
||||
room_id=-1001, user_id=999, username="nonadmin", text="Not admin"
|
||||
)
|
||||
|
||||
def test_list_bounties_empty_room(self):
|
||||
"""Test list_bounties returns empty list for non-existent room."""
|
||||
@@ -74,14 +78,16 @@ class TestBountyService:
|
||||
|
||||
def test_list_bounties_returns_all_bounties(self):
|
||||
"""Test list_bounties returns all bounties in a room."""
|
||||
self.service.add_bounty(room_id=-1001, user_id=self.admin_user_id, text="First")
|
||||
self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Second"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="First"
|
||||
)
|
||||
self.service.add_bounty(
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Second"
|
||||
)
|
||||
# Add bounty to different room to verify isolation
|
||||
self._make_admin(-999, self.admin_user_id)
|
||||
self._make_admin(-999, "otheradmin")
|
||||
self.service.add_bounty(
|
||||
room_id=-999, user_id=self.admin_user_id, text="Other room"
|
||||
room_id=-999, user_id=456, username="otheradmin", text="Other room"
|
||||
)
|
||||
|
||||
bounties = self.service.list_bounties(-1001)
|
||||
@@ -91,7 +97,7 @@ class TestBountyService:
|
||||
def test_get_bounty_found(self):
|
||||
"""Test get_bounty returns bounty when it exists."""
|
||||
created = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Test"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Test"
|
||||
)
|
||||
found = self.service.get_bounty(-1001, created.id)
|
||||
assert found is not None
|
||||
@@ -104,19 +110,21 @@ class TestBountyService:
|
||||
|
||||
def test_get_bounty_wrong_room(self):
|
||||
"""Test get_bounty returns None when bounty is in different room."""
|
||||
self.service.add_bounty(room_id=-1001, user_id=self.admin_user_id, text="Test")
|
||||
self.service.add_bounty(
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Test"
|
||||
)
|
||||
found = self.service.get_bounty(-999, 1) # room -999 doesn't have bounty 1
|
||||
assert found is None
|
||||
|
||||
def test_update_bounty_success(self):
|
||||
"""Test update_bounty succeeds when admin updates their bounty."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Original"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Original"
|
||||
)
|
||||
result = self.service.update_bounty(
|
||||
room_id=-1001,
|
||||
bounty_id=bounty.id,
|
||||
user_id=self.admin_user_id,
|
||||
username=self.admin_username,
|
||||
text="Updated",
|
||||
)
|
||||
assert result is True
|
||||
@@ -126,13 +134,13 @@ class TestBountyService:
|
||||
def test_update_bounty_not_admin_raises_permission_error(self):
|
||||
"""Test update_bounty raises PermissionError when non-admin tries to update."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="Original"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="Original"
|
||||
)
|
||||
with pytest.raises(PermissionError, match="Only admins can edit bounties"):
|
||||
self.service.update_bounty(
|
||||
room_id=-1001,
|
||||
bounty_id=bounty.id,
|
||||
user_id=999, # different user, not admin
|
||||
username="nonadmin", # different user, not admin
|
||||
text="Hacked",
|
||||
)
|
||||
|
||||
@@ -141,7 +149,7 @@ class TestBountyService:
|
||||
result = self.service.update_bounty(
|
||||
room_id=-1001,
|
||||
bounty_id=999,
|
||||
user_id=self.admin_user_id,
|
||||
username=self.admin_username,
|
||||
text="Updated",
|
||||
)
|
||||
assert result is False
|
||||
@@ -150,14 +158,15 @@ class TestBountyService:
|
||||
"""Test update_bounty only updates provided fields."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001,
|
||||
user_id=self.admin_user_id,
|
||||
user_id=123,
|
||||
username=self.admin_username,
|
||||
text="Original",
|
||||
link="https://original.link",
|
||||
)
|
||||
self.service.update_bounty(
|
||||
room_id=-1001,
|
||||
bounty_id=bounty.id,
|
||||
user_id=self.admin_user_id,
|
||||
username=self.admin_username,
|
||||
text="Updated only text",
|
||||
)
|
||||
updated = self.service.get_bounty(-1001, bounty.id)
|
||||
@@ -168,14 +177,15 @@ class TestBountyService:
|
||||
"""Test update_bounty can clear link."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001,
|
||||
user_id=self.admin_user_id,
|
||||
user_id=123,
|
||||
username=self.admin_username,
|
||||
text="Test",
|
||||
link="https://original.link",
|
||||
)
|
||||
self.service.update_bounty(
|
||||
room_id=-1001,
|
||||
bounty_id=bounty.id,
|
||||
user_id=self.admin_user_id,
|
||||
username=self.admin_username,
|
||||
clear_link=True,
|
||||
)
|
||||
updated = self.service.get_bounty(-1001, bounty.id)
|
||||
@@ -184,9 +194,9 @@ class TestBountyService:
|
||||
def test_delete_bounty_success(self):
|
||||
"""Test delete_bounty soft deletes when admin deletes their bounty."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete"
|
||||
)
|
||||
result = self.service.delete_bounty(-1001, bounty.id, self.admin_user_id)
|
||||
result = self.service.delete_bounty(-1001, bounty.id, self.admin_username)
|
||||
assert result is True
|
||||
# Soft delete - bounty should not be found via get_bounty
|
||||
assert self.service.get_bounty(-1001, bounty.id) is None
|
||||
@@ -198,28 +208,26 @@ class TestBountyService:
|
||||
def test_delete_bounty_not_admin_raises_permission_error(self):
|
||||
"""Test delete_bounty raises PermissionError when non-admin tries to delete."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete"
|
||||
)
|
||||
with pytest.raises(PermissionError, match="Only admins can delete bounties"):
|
||||
self.service.delete_bounty(
|
||||
-1001, bounty.id, 999
|
||||
) # different user, not admin
|
||||
self.service.delete_bounty(-1001, bounty.id, "nonadmin")
|
||||
|
||||
def test_delete_bounty_not_found(self):
|
||||
"""Test delete_bounty returns False when bounty doesn't exist."""
|
||||
result = self.service.delete_bounty(-1001, 999, self.admin_user_id)
|
||||
result = self.service.delete_bounty(-1001, 999, self.admin_username)
|
||||
assert result is False
|
||||
|
||||
def test_delete_bounties_multi_id_success(self):
|
||||
"""Test delete_bounties returns individual results for multiple bounties."""
|
||||
bounty1 = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete 1"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete 1"
|
||||
)
|
||||
bounty2 = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete 2"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete 2"
|
||||
)
|
||||
results = self.service.delete_bounties(
|
||||
-1001, [bounty1.id, bounty2.id], self.admin_user_id
|
||||
-1001, [bounty1.id, bounty2.id], self.admin_username
|
||||
)
|
||||
assert results == {bounty1.id: "deleted", bounty2.id: "deleted"}
|
||||
# Verify both are soft deleted
|
||||
@@ -229,22 +237,22 @@ class TestBountyService:
|
||||
def test_delete_bounties_mixed_results(self):
|
||||
"""Test delete_bounties returns not_found for non-existent bounties."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete"
|
||||
)
|
||||
results = self.service.delete_bounties(
|
||||
-1001, [bounty.id, 999, 888], self.admin_user_id
|
||||
-1001, [bounty.id, 999, 888], self.admin_username
|
||||
)
|
||||
assert results == {bounty.id: "deleted", 999: "not_found", 888: "not_found"}
|
||||
|
||||
def test_delete_bounties_permission_denied(self):
|
||||
"""Test delete_bounties returns permission_denied for non-admin users."""
|
||||
bounty = self.service.add_bounty(
|
||||
room_id=-1001, user_id=self.admin_user_id, text="To delete"
|
||||
room_id=-1001, user_id=123, username=self.admin_username, text="To delete"
|
||||
)
|
||||
results = self.service.delete_bounties(
|
||||
-1001,
|
||||
[bounty.id],
|
||||
999, # non-admin user
|
||||
"nonadmin", # non-admin user
|
||||
)
|
||||
assert results == {bounty.id: "permission_denied"}
|
||||
# Verify bounty was NOT deleted
|
||||
@@ -259,29 +267,31 @@ class TestTrackingService:
|
||||
self.room_storage = MockRoomStorage()
|
||||
self.tracking_storage = MockTrackingStorage()
|
||||
self.service = TrackingService(self.tracking_storage, self.room_storage)
|
||||
self.admin_user_id = 123
|
||||
self._make_admin(-1001, self.admin_user_id)
|
||||
self.admin_username = "admin"
|
||||
self._make_admin(-1001, self.admin_username)
|
||||
|
||||
def _make_admin(self, room_id: int, user_id: int):
|
||||
def _make_admin(self, room_id: int, username: str):
|
||||
"""Helper to set up a room with an admin user."""
|
||||
room_data = self.room_storage.load(room_id)
|
||||
if room_data is None:
|
||||
room_data = RoomData(
|
||||
room_id=room_id, bounties=[], next_id=0, admin_user_ids=[]
|
||||
room_id=room_id, bounties=[], next_id=0, admin_usernames=[]
|
||||
)
|
||||
if user_id not in (room_data.admin_user_ids or []):
|
||||
room_data.admin_user_ids = room_data.admin_user_ids or []
|
||||
room_data.admin_user_ids.append(user_id)
|
||||
if username not in (room_data.admin_usernames or []):
|
||||
room_data.admin_usernames = room_data.admin_usernames or []
|
||||
room_data.admin_usernames.append(username)
|
||||
self.room_storage.save(room_data)
|
||||
|
||||
def _add_bounty(self, room_id=-1001, user_id=123, text="Test bounty"):
|
||||
def _add_bounty(self, room_id=-1001, username="admin", text="Test bounty"):
|
||||
"""Helper to add a bounty for tracking tests."""
|
||||
if self.room_storage.load(room_id) is None or user_id not in (
|
||||
self.room_storage.load(room_id).admin_user_ids or []
|
||||
if self.room_storage.load(room_id) is None or username not in (
|
||||
self.room_storage.load(room_id).admin_usernames or []
|
||||
):
|
||||
self._make_admin(room_id, user_id)
|
||||
self._make_admin(room_id, username)
|
||||
bounty_service = BountyService(self.room_storage)
|
||||
return bounty_service.add_bounty(room_id=room_id, user_id=user_id, text=text)
|
||||
return bounty_service.add_bounty(
|
||||
room_id=room_id, user_id=123, username=username, text=text
|
||||
)
|
||||
|
||||
def test_track_bounty_success(self):
|
||||
"""Test track_bounty successfully tracks a bounty."""
|
||||
@@ -369,11 +379,13 @@ class TestTrackingService:
|
||||
def test_get_tracked_bounties_ignores_deleted_bounties(self):
|
||||
"""Test get_tracked_bounties ignores bounties that were deleted."""
|
||||
bounty_service = BountyService(self.room_storage)
|
||||
bounty = bounty_service.add_bounty(room_id=-1001, user_id=123, text="To delete")
|
||||
bounty = bounty_service.add_bounty(
|
||||
room_id=-1001, user_id=123, username="admin", text="To delete"
|
||||
)
|
||||
self.service.track_bounty(-1001, 123456, bounty.id)
|
||||
|
||||
# Delete the bounty
|
||||
bounty_service.delete_bounty(-1001, bounty.id, 123)
|
||||
bounty_service.delete_bounty(-1001, bounty.id, "admin")
|
||||
|
||||
tracked = self.service.get_tracked_bounties(-1001, 123456)
|
||||
assert len(tracked) == 0 # deleted bounty not returned
|
||||
|
||||
Reference in New Issue
Block a user