Files
StarPunk/docs/design/v1.0.0/phase1-completion-guide.md
Phil Skentelbery f10d0679da feat(tags): Add database schema and tags module (v1.3.0 Phase 1)
Implements tag/category system backend following microformats2 p-category specification.

Database changes:
- Migration 008: Add tags and note_tags tables
- Normalized tag storage (case-insensitive lookup, display name preserved)
- Indexes for performance

New module:
- starpunk/tags.py: Tag management functions
  - normalize_tag: Normalize tag strings
  - get_or_create_tag: Get or create tag records
  - add_tags_to_note: Associate tags with notes (replaces existing)
  - get_note_tags: Retrieve note tags (alphabetically ordered)
  - get_tag_by_name: Lookup tag by normalized name
  - get_notes_by_tag: Get all notes with specific tag
  - parse_tag_input: Parse comma-separated tag input

Model updates:
- Note.tags property (lazy-loaded, prefer pre-loading in routes)
- Note.to_dict() add include_tags parameter

CRUD updates:
- create_note() accepts tags parameter
- update_note() accepts tags parameter (None = no change, [] = remove all)

Micropub integration:
- Pass tags to create_note() (tags already extracted by extract_tags())
- Return tags in q=source response

Per design doc: docs/design/v1.3.0/microformats-tags-design.md

Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-10 11:24:23 -07:00

240 lines
7.1 KiB
Markdown

# Phase 1 Completion Guide: Test Cleanup and Commit
## Architectural Decision Summary
After reviewing your Phase 1 implementation, I've made the following architectural decisions:
### 1. Implementation Assessment: ✅ EXCELLENT
Your Phase 1 implementation is correct and complete. You've successfully:
- Removed the authorization endpoint cleanly
- Preserved admin functionality
- Documented everything properly
- Identified all test impacts
### 2. Test Strategy: DELETE ALL 30 FAILING TESTS NOW
**Rationale**: These tests are testing removed functionality. Keeping them provides no value and creates confusion.
### 3. Phase Strategy: ACCELERATE WITH COMBINED PHASES
After completing Phase 1, combine Phases 2+3 for faster delivery.
## Immediate Actions Required (30 minutes)
### Step 1: Analyze Failing Tests (5 minutes)
First, let's identify exactly which tests to remove:
```bash
# Get a clean list of failing test locations
uv run pytest --tb=no -q 2>&1 | grep "FAILED" | cut -d':' -f1-3 | sort -u
```
### Step 2: Remove OAuth Metadata Tests (5 minutes)
Edit `/home/phil/Projects/starpunk/tests/test_routes_public.py`:
**Delete these entire test classes**:
- `TestOAuthMetadataEndpoint` (all 10 tests)
- `TestIndieAuthMetadataLink` (all 3 tests)
These tested the `/.well-known/oauth-authorization-server` endpoint which no longer exists.
### Step 3: Handle State Token Tests (10 minutes)
Edit `/home/phil/Projects/starpunk/tests/test_auth.py`:
**Critical**: Some state token tests might be for admin login. Check each one:
```python
# If test references authorization flow -> DELETE
# If test references admin login -> KEEP AND FIX
```
Tests to review:
- `test_verify_valid_state_token` - Check if this is admin login
- `test_verify_invalid_state_token` - Check if this is admin login
- `test_verify_expired_state_token` - Check if this is admin login
- `test_state_tokens_are_single_use` - Check if this is admin login
- `test_initiate_login_success` - Likely admin login, may need fixing
- `test_handle_callback_*` - Check each for admin vs authorization
**Decision Logic**:
- If the test is validating state tokens for admin login via IndieLogin.com -> FIX IT
- If the test is validating state tokens for Micropub authorization -> DELETE IT
### Step 4: Fix Migration Tests (5 minutes)
Edit `/home/phil/Projects/starpunk/tests/test_migrations.py`:
For these two tests:
- `test_is_schema_current_with_code_verifier`
- `test_run_migrations_fresh_database`
**Action**: Remove any assertions about `code_verifier` or `code_challenge` columns. These PKCE fields are gone.
### Step 5: Remove Client Discovery Tests (2 minutes)
Edit `/home/phil/Projects/starpunk/tests/test_templates.py`:
**Delete the entire class**: `TestIndieAuthClientDiscovery`
This tested h-app microformats for Micropub client discovery, which is no longer relevant.
### Step 6: Fix Dev Auth Test (3 minutes)
Edit `/home/phil/Projects/starpunk/tests/test_routes_dev_auth.py`:
The test `test_dev_mode_requires_dev_admin_me` is failing. Investigate why and fix or remove based on current functionality.
## Verification Commands
After making changes:
```bash
# Run tests to verify all pass
uv run pytest
# Expected output:
# =============== XXX passed in X.XXs ===============
# (No failures!)
# Count remaining tests
uv run pytest --co -q | wc -l
# Should be around 539 tests (down from 569)
```
## Git Commit Strategy
### Commit 1: Test Cleanup
```bash
git add tests/
git commit -m "test: Remove tests for deleted IndieAuth authorization functionality
- Remove OAuth metadata endpoint tests (13 tests)
- Remove authorization-specific state token tests
- Remove authorization callback tests
- Remove h-app client discovery tests (5 tests)
- Update migration tests to match current schema
All removed tests validated functionality that was intentionally
deleted in Phase 1 of the IndieAuth removal plan.
Test suite now: 100% passing"
```
### Commit 2: Phase 1 Implementation
```bash
git add .
git commit -m "feat!: Phase 1 - Remove IndieAuth authorization server
BREAKING CHANGE: Removed built-in IndieAuth authorization endpoint
Removed:
- /auth/authorization endpoint and handler
- Authorization consent UI template
- Authorization-related imports and functions
- PKCE implementation tests
Preserved:
- Admin login via IndieLogin.com
- Session management
- Token endpoint (for Phase 2 removal)
This completes Phase 1 of 5 in the IndieAuth removal plan.
Version: 1.0.0-rc.4
Refs: ADR-050, ADR-051
Docs: docs/architecture/indieauth-removal-phases.md
Report: docs/reports/2025-11-24-phase1-indieauth-server-removal.md"
```
### Commit 3: Architecture Documentation
```bash
git add docs/
git commit -m "docs: Add architecture decisions and reports for Phase 1
- ADR-051: Test strategy and implementation review
- Phase 1 completion guide
- Implementation reports
These document the architectural decisions made during
Phase 1 implementation and provide guidance for remaining phases."
```
## Decision Points During Cleanup
### For State Token Tests
Ask yourself:
1. Does this test verify state tokens for `/auth/callback` (admin login)?
- **YES** → Fix the test to work with current code
- **NO** → Delete it
2. Does the test reference authorization codes or Micropub clients?
- **YES** → Delete it
- **NO** → Keep and fix
### For Callback Tests
Ask yourself:
1. Is this testing the IndieLogin.com callback for admin?
- **YES** → Fix it
- **NO** → Delete it
2. Does it reference authorization approval/denial?
- **YES** → Delete it
- **NO** → Keep and fix
## Success Criteria
You'll know Phase 1 is complete when:
1. ✅ All tests pass (100% green)
2. ✅ No references to authorization endpoint in tests
3. ✅ Admin login tests still present and passing
4. ✅ Clean git commits with clear messages
5. ✅ Documentation updated
## Next Steps: Combined Phase 2+3
After committing Phase 1, immediately proceed with:
1. **Phase 2+3 Combined** (2 hours):
- Remove `/auth/token` endpoint
- Delete `starpunk/tokens.py` entirely
- Create database migration to drop tables
- Remove all token-related tests
- Version: 1.0.0-rc.5
2. **Phase 4** (2 hours):
- Implement external token verification
- Add caching layer
- Update Micropub to use external verification
- Version: 1.0.0-rc.6
3. **Phase 5** (1 hour):
- Add discovery links
- Update all documentation
- Final version: 1.0.0
## Architecture Principles Maintained
Throughout this cleanup:
- **Simplicity First**: Remove complexity, don't reorganize it
- **Clean States**: No partially-broken states
- **Clear Intent**: Deleted code is better than commented code
- **Test Confidence**: Green tests or no tests, never red tests
## Questions?
If you encounter any test that you're unsure about:
1. Check if it tests admin functionality (keep/fix)
2. Check if it tests authorization functionality (delete)
3. When in doubt, trace the code path it's testing
Remember: We're removing an entire subsystem. It's better to be thorough than cautious.
---
**Time Estimate**: 30 minutes
**Complexity**: Low
**Risk**: Minimal (tests only)
**Confidence**: High - clear architectural decision