docs: add audit and category spec from feynman
This commit is contained in:
544
docs/AUDIT_AND_SPEC.md
Normal file
544
docs/AUDIT_AND_SPEC.md
Normal file
@@ -0,0 +1,544 @@
|
||||
# JIGAIDO Audit & Feature Specification
|
||||
|
||||
> Document created: 2026-04-09
|
||||
> Purpose: Repository audit findings and category feature specification
|
||||
|
||||
---
|
||||
|
||||
# Part I: Repository Audit
|
||||
|
||||
## 1.1 Current Architecture
|
||||
|
||||
JIGAIDO follows **hexagonal architecture** with clear separation:
|
||||
|
||||
```
|
||||
jigaido/
|
||||
├── core/ # Domain layer (pure Python, no deps)
|
||||
│ ├── models.py # Domain dataclasses
|
||||
│ ├── ports.py # Storage interfaces
|
||||
│ └── services.py # Business logic
|
||||
├── adapters/
|
||||
│ └── storage/
|
||||
│ └── json_file.py # JSON file persistence
|
||||
├── apps/
|
||||
│ └── telegram-bot/
|
||||
│ ├── bot.py # Bot entrypoint
|
||||
│ └── commands.py # Command handlers
|
||||
├── tests/ # Unit tests (98 tests passing)
|
||||
├── config.py # Configuration
|
||||
├── SPEC.md # Original design spec
|
||||
└── README.md # Overview
|
||||
```
|
||||
|
||||
## 1.2 Features Implemented
|
||||
|
||||
| Feature | Status | Location |
|
||||
|---------|--------|----------|
|
||||
| Group bounty management | ✅ Done | `BountyService` |
|
||||
| Personal DM bounties | ✅ Done | Same service, different room_id |
|
||||
| Admin management | ✅ Done | `add_admin`, `remove_admin`, `list_admins` |
|
||||
| Soft delete (recoverable) | ✅ Done | `delete_bounty`, `recover_bounty` |
|
||||
| Due date with timezone | ✅ Done | `dateparser` + `ZoneInfo` |
|
||||
| Link deduplication | ✅ Done | `check_link_unique` |
|
||||
| Tracking/untracking | ✅ Done | `TrackingService` |
|
||||
| `/track` in groups only | ✅ Done | Command handler |
|
||||
| Expired bounty filtering | ✅ Done | 24h cutoff logic |
|
||||
| Timezone per room | ✅ Done | `set_timezone`, `get_timezone` |
|
||||
|
||||
## 1.3 Bugs & Issues Found
|
||||
|
||||
### Bug 1: Admin Promotion Logic Edge Case
|
||||
**Location**: `core/services.py` - `add_admin()`
|
||||
|
||||
**Issue**: The first admin can self-promote, but if the Telegram group creator joins later, they won't be recognized as admin since they're not in `admin_usernames`.
|
||||
|
||||
**Code**:
|
||||
```python
|
||||
# core/services.py:44-49
|
||||
has_no_admins = room_data is None or not room_data.admin_usernames
|
||||
is_self_promotion = requesting_username == username
|
||||
|
||||
if not self.is_admin(room_id, requesting_username):
|
||||
if not (has_no_admins and is_self_promotion):
|
||||
raise PermissionError("Only admins can add admins.")
|
||||
```
|
||||
|
||||
**Recommendation**: Document this behavior or enhance to auto-detect Telegram group creator.
|
||||
|
||||
---
|
||||
|
||||
### Bug 2: Hard Delete Method in Storage
|
||||
**Location**: `adapters/storage/json_file.py:113-119`
|
||||
|
||||
**Issue**: `delete_bounty()` in storage does hard delete, while `BountyService.delete_bounty()` does soft delete. The storage method is unused but confusing.
|
||||
|
||||
**Code**:
|
||||
```python
|
||||
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)
|
||||
```
|
||||
|
||||
**Recommendation**: Remove or mark as deprecated.
|
||||
|
||||
---
|
||||
|
||||
### Issue 3: Spec vs Code Inconsistency
|
||||
**Location**: `SPEC.md` vs `commands.py`
|
||||
|
||||
**Issue**: SPEC.md says anyone can `/add` in groups, but code requires admin.
|
||||
|
||||
| Command | SPEC.md | Code |
|
||||
|---------|---------|------|
|
||||
| `/add` | anyone | admin only |
|
||||
| `/edit` | creator only | admin only |
|
||||
| `/delete` | creator only | admin only |
|
||||
|
||||
**Recommendation**: Update SPEC.md to reflect actual implementation.
|
||||
|
||||
---
|
||||
|
||||
### Issue 4: No Input Sanitization
|
||||
**Location**: `commands.py` - `parse_args()`
|
||||
|
||||
**Issue**: Links without proper scheme (e.g., `github.com/user/repo`) are accepted as-is.
|
||||
|
||||
**Recommendation**: Consider normalizing URL scheme or validating format.
|
||||
|
||||
---
|
||||
|
||||
### Issue 5: No Rate Limiting
|
||||
**Location**: `/add` command
|
||||
|
||||
**Issue**: No limit on:
|
||||
- Number of bounties per room
|
||||
- Text length
|
||||
- Request rate
|
||||
|
||||
**Recommendation**: Add rate limiting for production use.
|
||||
|
||||
---
|
||||
|
||||
## 1.4 Test Status
|
||||
|
||||
```bash
|
||||
$ PYTHONPATH=. python -m pytest tests/
|
||||
======================== 98 passed, 1 warning ========================
|
||||
```
|
||||
|
||||
**Note**: Tests require `PYTHONPATH=.` to run. Consider adding `pytest.ini` or `pyproject.toml`.
|
||||
|
||||
---
|
||||
|
||||
# Part II: Category Feature Specification
|
||||
|
||||
## 2.1 Overview
|
||||
|
||||
Add category support to JIGAIDO to allow filtering and organizing bounties.
|
||||
|
||||
**Goals**:
|
||||
- Admin-only category management (create, delete)
|
||||
- Multiple categories per bounty (no duplicates)
|
||||
- Filter bounties by category
|
||||
- Show categories on `/show` command
|
||||
- Backward compatibility (existing bounties work without categories)
|
||||
|
||||
## 2.2 Data Model
|
||||
|
||||
### Category
|
||||
```python
|
||||
@dataclass
|
||||
class Category:
|
||||
"""A category for organizing bounties in a room."""
|
||||
id: str # slug: lowercase alphabetic only, e.g., "bug", "feature"
|
||||
name: str # display name: e.g., "Bug", "Feature"
|
||||
created_at: int
|
||||
deleted_at: int | None = None # soft delete
|
||||
```
|
||||
|
||||
**Constraints**:
|
||||
- `id` (slug): lowercase alphabetic only, no symbols, e.g., `^[a-z]+$`
|
||||
- `name`: human-readable display
|
||||
- Unique within room (slug must be unique)
|
||||
- Soft delete preserves data
|
||||
|
||||
### Bounty (Modified)
|
||||
```python
|
||||
@dataclass
|
||||
class Bounty:
|
||||
# ... existing fields ...
|
||||
id: int
|
||||
text: str | None
|
||||
link: str | None
|
||||
due_date_ts: int | None
|
||||
created_at: int
|
||||
created_by_user_id: int
|
||||
deleted_at: int | None = None
|
||||
created_by_username: str | None = None
|
||||
category_ids: list[str] = field(default_factory=list) # NEW
|
||||
```
|
||||
|
||||
### RoomData (Modified)
|
||||
```python
|
||||
@dataclass
|
||||
class RoomData:
|
||||
room_id: int
|
||||
bounties: list[Bounty]
|
||||
next_id: int
|
||||
timezone: str | None = None
|
||||
admin_usernames: list[str] | None = None
|
||||
categories: list[Category] = field(default_factory=list) # NEW
|
||||
```
|
||||
|
||||
## 2.3 Category Scope
|
||||
|
||||
- **Per room**: Same as bounties, each room (group/DM) has independent categories
|
||||
- **Admin only**: Only admins can create/delete categories
|
||||
- **User access**: Regular users can only filter by category
|
||||
|
||||
## 2.4 Service Layer API
|
||||
|
||||
All methods require admin permission unless specified otherwise.
|
||||
|
||||
### Category Management
|
||||
|
||||
```python
|
||||
class BountyService:
|
||||
# ... existing methods ...
|
||||
|
||||
# --- Category Management ---
|
||||
|
||||
def add_category(
|
||||
self,
|
||||
room_id: int,
|
||||
slug: str,
|
||||
name: str,
|
||||
username: str | None
|
||||
) -> Category:
|
||||
"""Create a new category. Admin only.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
slug: Category ID (lowercase alphabetic, e.g., "bug")
|
||||
name: Display name (e.g., "Bug Report")
|
||||
username: Requesting admin's username
|
||||
|
||||
Returns:
|
||||
Created Category
|
||||
|
||||
Raises:
|
||||
PermissionError: If not admin
|
||||
ValueError: If slug already exists or invalid
|
||||
"""
|
||||
...
|
||||
|
||||
def delete_category(
|
||||
self,
|
||||
room_id: int,
|
||||
slug: str,
|
||||
username: str | None
|
||||
) -> bool:
|
||||
"""Soft delete a category. Admin only.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
slug: Category slug to delete
|
||||
username: Requesting admin's username
|
||||
|
||||
Returns:
|
||||
True if deleted, False if not found
|
||||
"""
|
||||
...
|
||||
|
||||
def list_categories(self, room_id: int) -> list[Category]:
|
||||
"""List active categories (excludes soft-deleted).
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
|
||||
Returns:
|
||||
List of active categories
|
||||
"""
|
||||
...
|
||||
|
||||
def get_category(self, room_id: int, slug: str) -> Category | None:
|
||||
"""Get a category by slug (excludes soft-deleted).
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
slug: Category slug
|
||||
|
||||
Returns:
|
||||
Category or None if not found
|
||||
"""
|
||||
...
|
||||
```
|
||||
|
||||
### Category-to-Bounty Association
|
||||
|
||||
```python
|
||||
def add_category_to_bounty(
|
||||
self,
|
||||
room_id: int,
|
||||
bounty_id: int,
|
||||
category_slug: str,
|
||||
username: str | None
|
||||
) -> bool:
|
||||
"""Add category to a bounty. Admin only.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
bounty_id: Bounty ID
|
||||
category_slug: Category slug to add
|
||||
username: Requesting admin's username
|
||||
|
||||
Returns:
|
||||
True if newly added, False if already exists
|
||||
|
||||
Raises:
|
||||
PermissionError: If not admin
|
||||
ValueError: If bounty or category not found
|
||||
"""
|
||||
...
|
||||
|
||||
def remove_category_from_bounty(
|
||||
self,
|
||||
room_id: int,
|
||||
bounty_id: int,
|
||||
category_slug: str,
|
||||
username: str | None
|
||||
) -> bool:
|
||||
"""Remove category from a bounty. Admin only.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
bounty_id: Bounty ID
|
||||
category_slug: Category slug to remove
|
||||
username: Requesting admin's username
|
||||
|
||||
Returns:
|
||||
True if removed, False if not found
|
||||
"""
|
||||
...
|
||||
|
||||
def update_bounty_categories(
|
||||
self,
|
||||
room_id: int,
|
||||
bounty_id: int,
|
||||
category_slugs: list[str],
|
||||
username: str | None
|
||||
) -> bool:
|
||||
"""Replace all categories on a bounty. Admin only.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
bounty_id: Bounty ID
|
||||
category_slugs: New list of category slugs
|
||||
username: Requesting admin's username
|
||||
|
||||
Returns:
|
||||
True if updated
|
||||
|
||||
Raises:
|
||||
PermissionError: If not admin
|
||||
ValueError: If bounty or any category not found
|
||||
"""
|
||||
...
|
||||
```
|
||||
|
||||
### Bounty Listing with Category Filter
|
||||
|
||||
```python
|
||||
def list_bounties(
|
||||
self,
|
||||
room_id: int,
|
||||
category_slugs: list[str] | None = None,
|
||||
include_expired: bool = False
|
||||
) -> list[Bounty]:
|
||||
"""List bounties with optional category filtering.
|
||||
|
||||
Args:
|
||||
room_id: Room identifier
|
||||
category_slugs: If provided, filter by ANY of these categories (OR)
|
||||
include_expired: If True, include bounties past due date
|
||||
|
||||
Returns:
|
||||
List of non-deleted bounties, sorted by due date
|
||||
"""
|
||||
...
|
||||
```
|
||||
|
||||
## 2.5 Filtering Logic
|
||||
|
||||
- **Single category**: `/bounty -c bug` → bounties with "bug"
|
||||
- **Multiple categories (OR)**: `/bounty -c bug,feature` → bounties with "bug" OR "feature"
|
||||
- **No filter**: `/bounty` → all bounties (current behavior)
|
||||
|
||||
## 2.6 Command Syntax
|
||||
|
||||
### Category Management
|
||||
```
|
||||
/category - list categories
|
||||
/category add <slug> <name> - create category (admin)
|
||||
/category delete <slug> - soft delete category (admin)
|
||||
```
|
||||
|
||||
### Bounty with Category
|
||||
```
|
||||
/add <text> [link] [date] -cat <slug> - add with category
|
||||
/add <text> [link] [date] -cat <slug1>,<slug2> - add with multiple categories
|
||||
|
||||
/update <id> -cat <slug> - add category to bounty
|
||||
/update <id> -cat <slug1>,<slug2> - set categories (replace all)
|
||||
/update <id> -cat - - clear all categories
|
||||
/update <id> -remove-cat <slug> - remove specific category
|
||||
```
|
||||
|
||||
### Bounty Listing with Filter
|
||||
```
|
||||
/bounty - all bounties (current)
|
||||
/bounty -c <slug> - filter by category
|
||||
/bounty -c <slug1>,<slug2> - filter by multiple categories (OR)
|
||||
/bounty all - show expired (current)
|
||||
/bounty all -c <slug> - show expired + filter by category
|
||||
```
|
||||
|
||||
### Show Bounty
|
||||
```
|
||||
/show <id> - show bounty details with categories
|
||||
```
|
||||
|
||||
## 2.7 Display Format
|
||||
|
||||
### `/category` output
|
||||
```
|
||||
Categories:
|
||||
- bug → Bug Report
|
||||
- feature → Feature Request
|
||||
- docs → Documentation
|
||||
```
|
||||
|
||||
### `/show <id>` output (with categories)
|
||||
```
|
||||
[#1] Fix login bug
|
||||
🔗 https://github.com/...
|
||||
📅 15 April 2026 14:30 (Asia/Jakarta)
|
||||
📂 Categories: bug | feature
|
||||
👤 @username
|
||||
📌 Created: 2026-04-01 10:00
|
||||
```
|
||||
|
||||
### `/bounty -c bug` output
|
||||
```
|
||||
Filtering with 🐛 bug category:
|
||||
Showing 3 of 10 bounties:
|
||||
[#5] ...
|
||||
[#1] ...
|
||||
[#3] ...
|
||||
```
|
||||
|
||||
### `/bounty -c bug,feature` output
|
||||
```
|
||||
Filtering with 🐛 bug, ✨ feature categories:
|
||||
Showing 5 of 10 bounties:
|
||||
[#5] ...
|
||||
[#1] ...
|
||||
```
|
||||
|
||||
## 2.8 Edge Cases
|
||||
|
||||
| Scenario | Behavior |
|
||||
|----------|----------|
|
||||
| Delete category | Soft delete - existing bounties keep category in data, but filter won't find it |
|
||||
| Filter by deleted category | Show "No bounties with this category" or error |
|
||||
| Add duplicate category to bounty | No-op, return False |
|
||||
| Add invalid slug (uppercase/symbols) | Reject with validation error |
|
||||
| Category slug conflict | Reject with "Category already exists" |
|
||||
| Bounty without categories | `category_ids = []` (backward compatible) |
|
||||
|
||||
## 2.9 Test Cases to Add
|
||||
|
||||
```python
|
||||
# Category Management
|
||||
def test_add_category_requires_admin():
|
||||
def test_add_category_duplicate_slug_fails():
|
||||
def test_add_category_invalid_slug_fails():
|
||||
def test_add_category_valid():
|
||||
def test_delete_category_soft_deletes():
|
||||
def test_deleted_category_not_listed():
|
||||
def test_deleted_category_still_in_bounty_data():
|
||||
def test_list_categories_empty():
|
||||
def test_list_categories_returns_active():
|
||||
def test_get_category_not_found():
|
||||
def test_get_category_deleted_returns_none():
|
||||
|
||||
# Category-to-Bounty
|
||||
def test_add_category_to_bounty():
|
||||
def test_add_duplicate_category_to_bounty_noop():
|
||||
def test_add_category_to_bounty_invalid_category():
|
||||
def test_remove_category_from_bounty():
|
||||
def test_remove_category_not_on_bounty_returns_false():
|
||||
def test_update_bounty_categories_replace_all():
|
||||
def test_update_bounty_categories_validates():
|
||||
|
||||
# Bounty Listing with Filter
|
||||
def test_list_bounties_filter_by_single_category():
|
||||
def test_list_bounties_filter_by_multiple_categories_or():
|
||||
def test_list_bounties_no_category_returns_all():
|
||||
def test_list_bounties_category_excludes_deleted_bounties():
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Part III: Implementation Checklist
|
||||
|
||||
## Models Layer
|
||||
- [ ] Add `Category` dataclass to `core/models.py`
|
||||
- [ ] Add `category_ids` field to `Bounty` dataclass
|
||||
- [ ] Add `categories` field to `RoomData` dataclass
|
||||
|
||||
## Storage Layer
|
||||
- [ ] Update `JsonFileRoomStorage.load()` to deserialize categories
|
||||
- [ ] Update `JsonFileRoomStorage.save()` to serialize categories
|
||||
|
||||
## Service Layer
|
||||
- [ ] Implement `add_category()`
|
||||
- [ ] Implement `delete_category()`
|
||||
- [ ] Implement `list_categories()`
|
||||
- [ ] Implement `get_category()`
|
||||
- [ ] Implement `add_category_to_bounty()`
|
||||
- [ ] Implement `remove_category_from_bounty()`
|
||||
- [ ] Implement `update_bounty_categories()`
|
||||
- [ ] Update `list_bounties()` to support category filter
|
||||
|
||||
## Command Layer
|
||||
- [ ] Add `/category` command handler
|
||||
- [ ] Add `-cat` flag parsing to `/add`
|
||||
- [ ] Add `-cat` and `-remove-cat` flags to `/update`
|
||||
- [ ] Add `-c` flag to `/bounty` for category filter
|
||||
- [ ] Update `/show` to display categories
|
||||
|
||||
## Tests
|
||||
- [ ] Add category management tests
|
||||
- [ ] Add category-to-bounty tests
|
||||
- [ ] Add category filter tests
|
||||
|
||||
## Documentation
|
||||
- [ ] Update README.md with category feature
|
||||
- [ ] Update command help text
|
||||
|
||||
---
|
||||
|
||||
# Part IV: Open Questions
|
||||
|
||||
1. **Category icon/emoji**: Should categories have optional emoji? (Not in initial spec, can add later)
|
||||
2. **Category reactivation**: Should soft-deleted categories be reactable? (Not in initial spec)
|
||||
3. **Bulk category operations**: Should we support `/category add bulk`? (Not in initial spec)
|
||||
|
||||
---
|
||||
|
||||
*End of Audit & Specification Document*
|
||||
Reference in New Issue
Block a user