From 4885be0752ea336c785f1f88b9b798f473901e53 Mon Sep 17 00:00:00 2001 From: shokollm <270575765+shokollm@users.noreply.github.com> Date: Thu, 9 Apr 2026 10:01:02 +0000 Subject: [PATCH] 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) --- SPEC.md | 10 ++++----- adapters/storage/json_file.py | 10 --------- apps/telegram-bot/bot.py | 5 +---- apps/telegram-bot/commands.py | 11 ++-------- apps/telegram-bot/run_bot.py | 2 +- apps/telegram-bot/tests/conftest.py | 2 -- apps/telegram-bot/tests/test_commands.py | 5 ++--- cli/main.py | 1 - core/ports.py | 4 ---- tests/test_cli.py | 5 ++--- tests/test_config.py | 2 -- tests/test_json_file.py | 11 ---------- tests/test_models.py | 2 -- tests/test_ports.py | 26 ------------------------ tests/test_services.py | 3 +-- 15 files changed, 14 insertions(+), 85 deletions(-) diff --git a/SPEC.md b/SPEC.md index bf1ae58..fef021e 100644 --- a/SPEC.md +++ b/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. -- **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. - **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`. @@ -122,9 +122,9 @@ Data is stored at `~/.jigaido/` (home directory), NOT inside the repository. |---|---|---| | `/bounty` | anyone | List all bounties in this group | | `/my` | anyone | List bounties tracked by you in this group | -| `/add [link] [due date]` | anyone | Add a new bounty to the group | -| `/edit [text] [link] [due_date]` | creator only | Edit an existing bounty | -| `/delete ` | creator only | Delete a bounty | +| `/add [link] [due date]` | admin only | Add a new bounty to the group | +| `/edit [text] [link] [due_date]` | admin only | Edit an existing bounty | +| `/delete ` | admin only | Delete a bounty | | `/track ` | anyone | Track a group bounty | | `/untrack ` | anyone | Stop tracking a bounty | @@ -170,7 +170,7 @@ Stored as Unix timestamp. User-facing display can be localized/converted to any ## Error Handling - 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) - `/untrack` not tracked → "Not tracking" (idempotent) - Bounty not found → "Bounty not found" diff --git a/adapters/storage/json_file.py b/adapters/storage/json_file.py index 951a275..db989ee 100644 --- a/adapters/storage/json_file.py +++ b/adapters/storage/json_file.py @@ -12,7 +12,6 @@ import tempfile from pathlib import Path from core.models import Bounty, RoomData, TrackingData, TrackedBounty -from core.ports import RoomStorage, TrackingStorage class JsonFileRoomStorage: @@ -119,15 +118,6 @@ class JsonFileRoomStorage: 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: """Get a specific bounty from a room by ID.""" room_data = self.load(room_id) diff --git a/apps/telegram-bot/bot.py b/apps/telegram-bot/bot.py index dd4445a..e251741 100644 --- a/apps/telegram-bot/bot.py +++ b/apps/telegram-bot/bot.py @@ -1,7 +1,6 @@ """JIGAIDO Telegram bot entrypoint.""" import logging -import os import sys sys.path.insert(0, "/home/shoko/repositories/jigaido") @@ -9,9 +8,7 @@ sys.path.insert(0, "/home/shoko/repositories/jigaido") from telegram.ext import ( Application, CommandHandler, - MessageHandler, CallbackQueryHandler, - filters, ) from commands import ( @@ -38,7 +35,7 @@ logging.basicConfig( ) log = logging.getLogger(__name__) -from config import config +from config import config # noqa: E402 BOT_TOKEN = config.bot_token or "" diff --git a/apps/telegram-bot/commands.py b/apps/telegram-bot/commands.py index 6eea429..0870d3a 100644 --- a/apps/telegram-bot/commands.py +++ b/apps/telegram-bot/commands.py @@ -1,8 +1,7 @@ """Telegram command handlers for JIGAIDO - Thin wrappers around core services.""" import time -from datetime import datetime, timezone -from functools import wraps +from datetime import datetime from typing import Optional from zoneinfo import ZoneInfo, ZoneInfoNotFoundError @@ -175,7 +174,6 @@ def format_bounty( except (KeyError, ZoneInfoNotFoundError): tz = ZoneInfo("UTC") - now_utc = int(time.time()) dt_now = datetime.now(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) - 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) if ( not text @@ -1034,7 +1027,7 @@ async def cmd_admin(update: Update, ctx: ContextTypes.DEFAULT_TYPE) -> None: ] reply_markup = InlineKeyboardMarkup(keyboard) 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, reply_markup=reply_markup, ) diff --git a/apps/telegram-bot/run_bot.py b/apps/telegram-bot/run_bot.py index 67309c7..8774548 100644 --- a/apps/telegram-bot/run_bot.py +++ b/apps/telegram-bot/run_bot.py @@ -8,7 +8,7 @@ os.chdir("/home/shoko/repositories/jigaido/apps/telegram-bot") sys.path.insert(0, "/home/shoko/repositories/jigaido") # Import main from the local bot module -import bot as bot_module +import bot as bot_module # noqa: E402 if __name__ == "__main__": if not bot_module.BOT_TOKEN: diff --git a/apps/telegram-bot/tests/conftest.py b/apps/telegram-bot/tests/conftest.py index 818ea7d..98a7447 100644 --- a/apps/telegram-bot/tests/conftest.py +++ b/apps/telegram-bot/tests/conftest.py @@ -1,10 +1,8 @@ """Pytest fixtures for telegram-bot tests.""" import sys -import tempfile from pathlib import Path -import pytest # Add the app directory to path so imports work when running pytest sys.path.insert(0, str(Path(__file__).parent.parent)) diff --git a/apps/telegram-bot/tests/test_commands.py b/apps/telegram-bot/tests/test_commands.py index 74d2ae1..2898c31 100644 --- a/apps/telegram-bot/tests/test_commands.py +++ b/apps/telegram-bot/tests/test_commands.py @@ -1,11 +1,11 @@ """Tests for commands.py — parsing, formatting, and command handlers.""" import time -from unittest.mock import MagicMock, patch, AsyncMock, sentinel +from unittest.mock import MagicMock, patch, AsyncMock import pytest -from telegram import Update, Message, User, Chat, CallbackQuery +from telegram import Update, Message, User, Chat from telegram.ext import ContextTypes from commands import ( @@ -368,7 +368,6 @@ class TestCmdAdd: @pytest.mark.asyncio async def test_add_needs_text_or_link(self): update = create_mock_update(message_text="/add") - ctx = MagicMock(spec=ContextTypes.DEFAULT_TYPE) _, link, _ = parse_args([]) if not "test" and not link: diff --git a/cli/main.py b/cli/main.py index 06faaba..3562712 100644 --- a/cli/main.py +++ b/cli/main.py @@ -2,7 +2,6 @@ import argparse import sys -from pathlib import Path import dateparser diff --git a/core/ports.py b/core/ports.py index 0e98834..c7ebcce 100644 --- a/core/ports.py +++ b/core/ports.py @@ -32,10 +32,6 @@ class RoomStorage(Protocol): """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: """Get a specific bounty from a room by ID.""" ... diff --git a/tests/test_cli.py b/tests/test_cli.py index 8e7a49e..f5d4262 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,7 +3,6 @@ import pytest from unittest.mock import patch, MagicMock from io import StringIO -import sys from core.models import Bounty from core.ports import RoomStorage, TrackingStorage @@ -237,7 +236,7 @@ class TestCLIValidation: main() mock_bounty_service.update_bounty.assert_called_once() 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): """Test update with --clear-due flag.""" @@ -255,7 +254,7 @@ class TestCLIValidation: main() mock_bounty_service.update_bounty.assert_called_once() 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: diff --git a/tests/test_config.py b/tests/test_config.py index c5666cb..a5cd592 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,11 +2,9 @@ import json import os -import tempfile from pathlib import Path from unittest.mock import patch -import pytest from config import Config, DEFAULT_DATA_DIR diff --git a/tests/test_json_file.py b/tests/test_json_file.py index 5a1bb72..01bd38c 100644 --- a/tests/test_json_file.py +++ b/tests/test_json_file.py @@ -1,11 +1,9 @@ """Tests for adapters/storage/json_file.py — JSON file storage adapter.""" import json -import os import tempfile from pathlib import Path -import pytest from adapters.storage.json_file import JsonFileRoomStorage, JsonFileTrackingStorage from core.models import Bounty, RoomData, TrackingData, TrackedBounty @@ -99,15 +97,6 @@ class TestJsonFileRoomStorage: 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): """Test that get_bounty returns the bounty when found.""" bounty = self._create_bounty(id=1) diff --git a/tests/test_models.py b/tests/test_models.py index 9564412..3d0320a 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,8 +1,6 @@ """Tests for core/models.py — domain dataclasses.""" -import time -import pytest from core.models import ( Bounty, diff --git a/tests/test_ports.py b/tests/test_ports.py index acb63fb..d1ee1fd 100644 --- a/tests/test_ports.py +++ b/tests/test_ports.py @@ -1,6 +1,5 @@ """Tests for core/ports.py — storage interfaces.""" -import pytest from core.models import Bounty, RoomData, TrackingData, TrackedBounty from core.ports import RoomStorage, TrackingStorage @@ -35,12 +34,6 @@ class SimpleRoomStorage: self._rooms[room_id].bounties[i] = bounty 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: if room_id in self._rooms: for b in self._rooms[room_id].bounties: @@ -120,12 +113,6 @@ class MockRoomStorage: self._rooms[room_id].bounties[i] = bounty 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: if room_id in self._rooms: for b in self._rooms[room_id].bounties: @@ -243,19 +230,6 @@ class TestRoomStorage: assert result is not None 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: diff --git a/tests/test_services.py b/tests/test_services.py index c0a3381..50b6885 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -1,9 +1,8 @@ """Tests for core/services.py — business logic services.""" 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 tests.test_ports import MockRoomStorage, MockTrackingStorage