fix: cleanup codebase and sync SPEC with actual permissions
Phase 1: Ruff lint fixes - Remove unused imports across all files - Remove unused variables (now_utc, tz, ctx) - Fix f-string without placeholders - Fix E402 import order with noqa comments Phase 2: Remove confusing hard delete from storage - Removed delete_bounty() from RoomStorage Protocol (never used by app) - Removed delete_bounty() from JsonFileRoomStorage (was hard delete) - Removed corresponding tests (hard delete was never used) Phase 3: Sync SPEC.md with actual code behavior - Updated overview: admins can add/edit/delete (not 'anyone' + 'creator') - Updated command table: /add, /edit, /delete are admin only - Updated error handling messages Test results: 96 passed (2 hard delete tests removed)
This commit is contained in:
10
SPEC.md
10
SPEC.md
@@ -8,7 +8,7 @@
|
|||||||
|
|
||||||
JIGAIDO is a Telegram bot that lets groups and individuals track bounties — tasks, obligations, and deadlines — with optional due dates and personal tracking.
|
JIGAIDO is a Telegram bot that lets groups and individuals track bounties — tasks, obligations, and deadlines — with optional due dates and personal tracking.
|
||||||
|
|
||||||
- **Group mode**: Each Telegram group has its own bounty list. Anyone can add bounties. Only creator can edit/delete.
|
- **Group mode**: Each Telegram group has its own bounty list. Admins can add/edit/delete bounties. Anyone can track.
|
||||||
- **DM mode**: Personal bounty list. Anyone can manage their own bounties.
|
- **DM mode**: Personal bounty list. Anyone can manage their own bounties.
|
||||||
- **Tracking**: Users can track any bounty (group or personal) to their tracking list.
|
- **Tracking**: Users can track any bounty (group or personal) to their tracking list.
|
||||||
- **Due dates**: Free-form text (`"april 15"`, `"in 3 days"`, `"tomorrow"`) parsed at add time, stored as Unix timestamp. If unparseable, stored as `NULL`.
|
- **Due dates**: Free-form text (`"april 15"`, `"in 3 days"`, `"tomorrow"`) parsed at add time, stored as Unix timestamp. If unparseable, stored as `NULL`.
|
||||||
@@ -122,9 +122,9 @@ Data is stored at `~/.jigaido/` (home directory), NOT inside the repository.
|
|||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `/bounty` | anyone | List all bounties in this group |
|
| `/bounty` | anyone | List all bounties in this group |
|
||||||
| `/my` | anyone | List bounties tracked by you in this group |
|
| `/my` | anyone | List bounties tracked by you in this group |
|
||||||
| `/add <text> [link] [due date]` | anyone | Add a new bounty to the group |
|
| `/add <text> [link] [due date]` | admin only | Add a new bounty to the group |
|
||||||
| `/edit <bounty_id> [text] [link] [due_date]` | creator only | Edit an existing bounty |
|
| `/edit <bounty_id> [text] [link] [due_date]` | admin only | Edit an existing bounty |
|
||||||
| `/delete <bounty_id>` | creator only | Delete a bounty |
|
| `/delete <bounty_id>` | admin only | Delete a bounty |
|
||||||
| `/track <bounty_id>` | anyone | Track a group bounty |
|
| `/track <bounty_id>` | anyone | Track a group bounty |
|
||||||
| `/untrack <bounty_id>` | anyone | Stop tracking a bounty |
|
| `/untrack <bounty_id>` | anyone | Stop tracking a bounty |
|
||||||
|
|
||||||
@@ -170,7 +170,7 @@ Stored as Unix timestamp. User-facing display can be localized/converted to any
|
|||||||
## Error Handling
|
## Error Handling
|
||||||
|
|
||||||
- Unknown command → help text with available commands
|
- Unknown command → help text with available commands
|
||||||
- `/edit`/`/delete` by non-creator → "⛔ Only the creator can edit/delete this bounty."
|
- `/add`/`/edit`/`/delete` by non-admin → "⛔ Only admins can add/edit/delete bounties."
|
||||||
- `/track` already tracked → "Already tracking" (idempotent)
|
- `/track` already tracked → "Already tracking" (idempotent)
|
||||||
- `/untrack` not tracked → "Not tracking" (idempotent)
|
- `/untrack` not tracked → "Not tracking" (idempotent)
|
||||||
- Bounty not found → "Bounty not found"
|
- Bounty not found → "Bounty not found"
|
||||||
|
|||||||
@@ -12,7 +12,6 @@ import tempfile
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
||||||
from core.ports import RoomStorage, TrackingStorage
|
|
||||||
|
|
||||||
|
|
||||||
class JsonFileRoomStorage:
|
class JsonFileRoomStorage:
|
||||||
@@ -119,15 +118,6 @@ class JsonFileRoomStorage:
|
|||||||
|
|
||||||
self.save(room_data)
|
self.save(room_data)
|
||||||
|
|
||||||
def delete_bounty(self, room_id: int, bounty_id: int) -> None:
|
|
||||||
"""Delete a bounty from a room."""
|
|
||||||
room_data = self.load(room_id)
|
|
||||||
if room_data is None:
|
|
||||||
return
|
|
||||||
|
|
||||||
room_data.bounties = [b for b in room_data.bounties if b.id != bounty_id]
|
|
||||||
self.save(room_data)
|
|
||||||
|
|
||||||
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
||||||
"""Get a specific bounty from a room by ID."""
|
"""Get a specific bounty from a room by ID."""
|
||||||
room_data = self.load(room_id)
|
room_data = self.load(room_id)
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
"""JIGAIDO Telegram bot entrypoint."""
|
"""JIGAIDO Telegram bot entrypoint."""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
import os
|
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
sys.path.insert(0, "/home/shoko/repositories/jigaido")
|
sys.path.insert(0, "/home/shoko/repositories/jigaido")
|
||||||
@@ -9,9 +8,7 @@ sys.path.insert(0, "/home/shoko/repositories/jigaido")
|
|||||||
from telegram.ext import (
|
from telegram.ext import (
|
||||||
Application,
|
Application,
|
||||||
CommandHandler,
|
CommandHandler,
|
||||||
MessageHandler,
|
|
||||||
CallbackQueryHandler,
|
CallbackQueryHandler,
|
||||||
filters,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
from commands import (
|
from commands import (
|
||||||
@@ -38,7 +35,7 @@ logging.basicConfig(
|
|||||||
)
|
)
|
||||||
log = logging.getLogger(__name__)
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
from config import config
|
from config import config # noqa: E402
|
||||||
|
|
||||||
BOT_TOKEN = config.bot_token or ""
|
BOT_TOKEN = config.bot_token or ""
|
||||||
|
|
||||||
|
|||||||
@@ -1,8 +1,7 @@
|
|||||||
"""Telegram command handlers for JIGAIDO - Thin wrappers around core services."""
|
"""Telegram command handlers for JIGAIDO - Thin wrappers around core services."""
|
||||||
|
|
||||||
import time
|
import time
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime
|
||||||
from functools import wraps
|
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
|
from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
|
||||||
|
|
||||||
@@ -175,7 +174,6 @@ def format_bounty(
|
|||||||
except (KeyError, ZoneInfoNotFoundError):
|
except (KeyError, ZoneInfoNotFoundError):
|
||||||
tz = ZoneInfo("UTC")
|
tz = ZoneInfo("UTC")
|
||||||
|
|
||||||
now_utc = int(time.time())
|
|
||||||
dt_now = datetime.now(tz)
|
dt_now = datetime.now(tz)
|
||||||
dt_due = datetime.fromtimestamp(b.due_date_ts, tz=tz)
|
dt_due = datetime.fromtimestamp(b.due_date_ts, tz=tz)
|
||||||
|
|
||||||
@@ -459,11 +457,6 @@ async def cmd_update(update: Update, ctx: ContextTypes.DEFAULT_TYPE) -> None:
|
|||||||
|
|
||||||
timezone_str = BOUNTY_SERVICE.get_timezone(room_id)
|
timezone_str = BOUNTY_SERVICE.get_timezone(room_id)
|
||||||
|
|
||||||
try:
|
|
||||||
tz = ZoneInfo(timezone_str)
|
|
||||||
except (KeyError, ZoneInfoNotFoundError):
|
|
||||||
tz = ZoneInfo("UTC")
|
|
||||||
|
|
||||||
text, link, due_date_ts, clear_link, clear_date = parse_args(args[1:], timezone_str)
|
text, link, due_date_ts, clear_link, clear_date = parse_args(args[1:], timezone_str)
|
||||||
if (
|
if (
|
||||||
not text
|
not text
|
||||||
@@ -1034,7 +1027,7 @@ async def cmd_admin(update: Update, ctx: ContextTypes.DEFAULT_TYPE) -> None:
|
|||||||
]
|
]
|
||||||
reply_markup = InlineKeyboardMarkup(keyboard)
|
reply_markup = InlineKeyboardMarkup(keyboard)
|
||||||
await update.message.reply_text(
|
await update.message.reply_text(
|
||||||
f"Room Admins:\n" + "\n".join(f"- {m}" for m in admin_mentions),
|
"Room Admins:\n" + "\n".join(f"- {m}" for m in admin_mentions),
|
||||||
parse_mode=ParseMode.HTML,
|
parse_mode=ParseMode.HTML,
|
||||||
reply_markup=reply_markup,
|
reply_markup=reply_markup,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ os.chdir("/home/shoko/repositories/jigaido/apps/telegram-bot")
|
|||||||
sys.path.insert(0, "/home/shoko/repositories/jigaido")
|
sys.path.insert(0, "/home/shoko/repositories/jigaido")
|
||||||
|
|
||||||
# Import main from the local bot module
|
# Import main from the local bot module
|
||||||
import bot as bot_module
|
import bot as bot_module # noqa: E402
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
if not bot_module.BOT_TOKEN:
|
if not bot_module.BOT_TOKEN:
|
||||||
|
|||||||
@@ -1,10 +1,8 @@
|
|||||||
"""Pytest fixtures for telegram-bot tests."""
|
"""Pytest fixtures for telegram-bot tests."""
|
||||||
|
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
# Add the app directory to path so imports work when running pytest
|
# Add the app directory to path so imports work when running pytest
|
||||||
sys.path.insert(0, str(Path(__file__).parent.parent))
|
sys.path.insert(0, str(Path(__file__).parent.parent))
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
"""Tests for commands.py — parsing, formatting, and command handlers."""
|
"""Tests for commands.py — parsing, formatting, and command handlers."""
|
||||||
|
|
||||||
import time
|
import time
|
||||||
from unittest.mock import MagicMock, patch, AsyncMock, sentinel
|
from unittest.mock import MagicMock, patch, AsyncMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from telegram import Update, Message, User, Chat, CallbackQuery
|
from telegram import Update, Message, User, Chat
|
||||||
from telegram.ext import ContextTypes
|
from telegram.ext import ContextTypes
|
||||||
|
|
||||||
from commands import (
|
from commands import (
|
||||||
@@ -368,7 +368,6 @@ class TestCmdAdd:
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_add_needs_text_or_link(self):
|
async def test_add_needs_text_or_link(self):
|
||||||
update = create_mock_update(message_text="/add")
|
update = create_mock_update(message_text="/add")
|
||||||
ctx = MagicMock(spec=ContextTypes.DEFAULT_TYPE)
|
|
||||||
|
|
||||||
_, link, _ = parse_args([])
|
_, link, _ = parse_args([])
|
||||||
if not "test" and not link:
|
if not "test" and not link:
|
||||||
|
|||||||
@@ -2,7 +2,6 @@
|
|||||||
|
|
||||||
import argparse
|
import argparse
|
||||||
import sys
|
import sys
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
import dateparser
|
import dateparser
|
||||||
|
|
||||||
|
|||||||
@@ -32,10 +32,6 @@ class RoomStorage(Protocol):
|
|||||||
"""Update an existing bounty in a room."""
|
"""Update an existing bounty in a room."""
|
||||||
...
|
...
|
||||||
|
|
||||||
def delete_bounty(self, room_id: int, bounty_id: int) -> None:
|
|
||||||
"""Delete a bounty from a room."""
|
|
||||||
...
|
|
||||||
|
|
||||||
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
||||||
"""Get a specific bounty from a room by ID."""
|
"""Get a specific bounty from a room by ID."""
|
||||||
...
|
...
|
||||||
|
|||||||
@@ -3,7 +3,6 @@
|
|||||||
import pytest
|
import pytest
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
from io import StringIO
|
from io import StringIO
|
||||||
import sys
|
|
||||||
|
|
||||||
from core.models import Bounty
|
from core.models import Bounty
|
||||||
from core.ports import RoomStorage, TrackingStorage
|
from core.ports import RoomStorage, TrackingStorage
|
||||||
@@ -237,7 +236,7 @@ class TestCLIValidation:
|
|||||||
main()
|
main()
|
||||||
mock_bounty_service.update_bounty.assert_called_once()
|
mock_bounty_service.update_bounty.assert_called_once()
|
||||||
call_kwargs = mock_bounty_service.update_bounty.call_args
|
call_kwargs = mock_bounty_service.update_bounty.call_args
|
||||||
assert call_kwargs.kwargs.get("clear_link") == True
|
assert call_kwargs.kwargs.get("clear_link") is True
|
||||||
|
|
||||||
def test_update_clear_due_flag(self):
|
def test_update_clear_due_flag(self):
|
||||||
"""Test update with --clear-due flag."""
|
"""Test update with --clear-due flag."""
|
||||||
@@ -255,7 +254,7 @@ class TestCLIValidation:
|
|||||||
main()
|
main()
|
||||||
mock_bounty_service.update_bounty.assert_called_once()
|
mock_bounty_service.update_bounty.assert_called_once()
|
||||||
call_kwargs = mock_bounty_service.update_bounty.call_args
|
call_kwargs = mock_bounty_service.update_bounty.call_args
|
||||||
assert call_kwargs.kwargs.get("clear_due") == True
|
assert call_kwargs.kwargs.get("clear_due") is True
|
||||||
|
|
||||||
|
|
||||||
class TestCLIOutput:
|
class TestCLIOutput:
|
||||||
|
|||||||
@@ -2,11 +2,9 @@
|
|||||||
|
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import tempfile
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from config import Config, DEFAULT_DATA_DIR
|
from config import Config, DEFAULT_DATA_DIR
|
||||||
|
|
||||||
|
|||||||
@@ -1,11 +1,9 @@
|
|||||||
"""Tests for adapters/storage/json_file.py — JSON file storage adapter."""
|
"""Tests for adapters/storage/json_file.py — JSON file storage adapter."""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import os
|
|
||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from adapters.storage.json_file import JsonFileRoomStorage, JsonFileTrackingStorage
|
from adapters.storage.json_file import JsonFileRoomStorage, JsonFileTrackingStorage
|
||||||
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
||||||
@@ -99,15 +97,6 @@ class TestJsonFileRoomStorage:
|
|||||||
|
|
||||||
assert self.storage.load(-1001) is None
|
assert self.storage.load(-1001) is None
|
||||||
|
|
||||||
def test_delete_bounty(self):
|
|
||||||
"""Test that delete_bounty removes a bounty."""
|
|
||||||
bounty = self._create_bounty(id=1)
|
|
||||||
self.storage.add_bounty(-1001, bounty)
|
|
||||||
self.storage.delete_bounty(-1001, 1)
|
|
||||||
|
|
||||||
loaded = self.storage.load(-1001)
|
|
||||||
assert len(loaded.bounties) == 0
|
|
||||||
|
|
||||||
def test_get_bounty_found(self):
|
def test_get_bounty_found(self):
|
||||||
"""Test that get_bounty returns the bounty when found."""
|
"""Test that get_bounty returns the bounty when found."""
|
||||||
bounty = self._create_bounty(id=1)
|
bounty = self._create_bounty(id=1)
|
||||||
|
|||||||
@@ -1,8 +1,6 @@
|
|||||||
"""Tests for core/models.py — domain dataclasses."""
|
"""Tests for core/models.py — domain dataclasses."""
|
||||||
|
|
||||||
import time
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from core.models import (
|
from core.models import (
|
||||||
Bounty,
|
Bounty,
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
"""Tests for core/ports.py — storage interfaces."""
|
"""Tests for core/ports.py — storage interfaces."""
|
||||||
|
|
||||||
import pytest
|
|
||||||
|
|
||||||
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
||||||
from core.ports import RoomStorage, TrackingStorage
|
from core.ports import RoomStorage, TrackingStorage
|
||||||
@@ -35,12 +34,6 @@ class SimpleRoomStorage:
|
|||||||
self._rooms[room_id].bounties[i] = bounty
|
self._rooms[room_id].bounties[i] = bounty
|
||||||
break
|
break
|
||||||
|
|
||||||
def delete_bounty(self, room_id: int, bounty_id: int) -> None:
|
|
||||||
if room_id in self._rooms:
|
|
||||||
self._rooms[room_id].bounties = [
|
|
||||||
b for b in self._rooms[room_id].bounties if b.id != bounty_id
|
|
||||||
]
|
|
||||||
|
|
||||||
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
||||||
if room_id in self._rooms:
|
if room_id in self._rooms:
|
||||||
for b in self._rooms[room_id].bounties:
|
for b in self._rooms[room_id].bounties:
|
||||||
@@ -120,12 +113,6 @@ class MockRoomStorage:
|
|||||||
self._rooms[room_id].bounties[i] = bounty
|
self._rooms[room_id].bounties[i] = bounty
|
||||||
break
|
break
|
||||||
|
|
||||||
def delete_bounty(self, room_id: int, bounty_id: int) -> None:
|
|
||||||
if room_id in self._rooms:
|
|
||||||
self._rooms[room_id].bounties = [
|
|
||||||
b for b in self._rooms[room_id].bounties if b.id != bounty_id
|
|
||||||
]
|
|
||||||
|
|
||||||
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None:
|
||||||
if room_id in self._rooms:
|
if room_id in self._rooms:
|
||||||
for b in self._rooms[room_id].bounties:
|
for b in self._rooms[room_id].bounties:
|
||||||
@@ -243,19 +230,6 @@ class TestRoomStorage:
|
|||||||
assert result is not None
|
assert result is not None
|
||||||
assert result.text == "Updated"
|
assert result.text == "Updated"
|
||||||
|
|
||||||
def test_delete_bounty(self):
|
|
||||||
storage = MockRoomStorage()
|
|
||||||
bounty = Bounty(
|
|
||||||
id=1,
|
|
||||||
text="Test",
|
|
||||||
link=None,
|
|
||||||
due_date_ts=None,
|
|
||||||
created_at=0,
|
|
||||||
created_by_user_id=123,
|
|
||||||
)
|
|
||||||
storage.add_bounty(-1001, bounty)
|
|
||||||
storage.delete_bounty(-1001, 1)
|
|
||||||
assert storage.get_bounty(-1001, 1) is None
|
|
||||||
|
|
||||||
|
|
||||||
class TestTrackingStorage:
|
class TestTrackingStorage:
|
||||||
|
|||||||
@@ -1,9 +1,8 @@
|
|||||||
"""Tests for core/services.py — business logic services."""
|
"""Tests for core/services.py — business logic services."""
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from unittest.mock import MagicMock
|
|
||||||
|
|
||||||
from core.models import Bounty, RoomData, TrackingData, TrackedBounty
|
from core.models import RoomData
|
||||||
from core.services import BountyService, TrackingService
|
from core.services import BountyService, TrackingService
|
||||||
from tests.test_ports import MockRoomStorage, MockTrackingStorage
|
from tests.test_ports import MockRoomStorage, MockTrackingStorage
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user