Merge bugfix/custom-slug-extraction: Fix mp-slug extraction
Fix custom slug extraction bug where mp-slug was being filtered out by normalize_properties() before it could be used. Changes: - Extract mp-slug from raw request data before normalization - Add tests for both form-encoded and JSON formats - All 13 Micropub tests passing Fixes issue where Quill-specified custom slugs were ignored. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **Custom Slug Extraction** - Fixed bug where mp-slug was ignored in Micropub requests
|
||||
- Root cause: mp-slug was extracted after normalize_properties() filtered it out
|
||||
- Solution: Extract mp-slug from raw request data before normalization
|
||||
- Affects both form-encoded and JSON Micropub requests
|
||||
- See docs/reports/custom-slug-bug-diagnosis.md for detailed analysis
|
||||
|
||||
## [1.1.0] - 2025-11-25
|
||||
|
||||
### Added
|
||||
|
||||
205
docs/reports/custom-slug-bug-implementation.md
Normal file
205
docs/reports/custom-slug-bug-implementation.md
Normal file
@@ -0,0 +1,205 @@
|
||||
# Custom Slug Bug Fix - Implementation Report
|
||||
|
||||
**Date**: 2025-11-25
|
||||
**Developer**: StarPunk Developer Subagent
|
||||
**Branch**: bugfix/custom-slug-extraction
|
||||
**Status**: Complete - Ready for Testing
|
||||
|
||||
## Executive Summary
|
||||
|
||||
Successfully fixed the custom slug extraction bug in the Micropub handler. Custom slugs specified via `mp-slug` parameter are now correctly extracted and used when creating notes.
|
||||
|
||||
## Problem Statement
|
||||
|
||||
Custom slugs specified via the `mp-slug` property in Micropub requests were being completely ignored. The system was falling back to auto-generated slugs even when a custom slug was provided by the client (e.g., Quill).
|
||||
|
||||
**Root Cause**: `mp-slug` was being extracted from normalized properties after it had already been filtered out by `normalize_properties()` which removes all `mp-*` parameters.
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### Files Modified
|
||||
|
||||
1. **starpunk/micropub.py** (lines 290-307)
|
||||
- Moved `mp-slug` extraction to BEFORE property normalization
|
||||
- Added support for both form-encoded and JSON request formats
|
||||
- Added clear comments explaining the timing requirement
|
||||
|
||||
2. **tests/test_micropub.py** (added lines 191-246)
|
||||
- Added `test_micropub_create_with_custom_slug_form()` - tests form-encoded requests
|
||||
- Added `test_micropub_create_with_custom_slug_json()` - tests JSON requests
|
||||
- Both tests verify the custom slug is actually used in the created note
|
||||
|
||||
### Code Changes
|
||||
|
||||
#### Before (Broken)
|
||||
```python
|
||||
# Normalize and extract properties
|
||||
try:
|
||||
properties = normalize_properties(data) # mp-slug gets filtered here!
|
||||
content = extract_content(properties)
|
||||
title = extract_title(properties)
|
||||
tags = extract_tags(properties)
|
||||
published_date = extract_published_date(properties)
|
||||
|
||||
# Extract custom slug if provided (Micropub extension)
|
||||
custom_slug = None
|
||||
if 'mp-slug' in properties: # BUG: mp-slug not in properties!
|
||||
slug_values = properties.get('mp-slug', [])
|
||||
if slug_values and len(slug_values) > 0:
|
||||
custom_slug = slug_values[0]
|
||||
```
|
||||
|
||||
#### After (Fixed)
|
||||
```python
|
||||
# Extract mp-slug BEFORE normalizing properties (it's not a property!)
|
||||
# mp-slug is a Micropub server extension parameter that gets filtered during normalization
|
||||
custom_slug = None
|
||||
if isinstance(data, dict) and 'mp-slug' in data:
|
||||
# Handle both form-encoded (list) and JSON (could be string or list)
|
||||
slug_value = data.get('mp-slug')
|
||||
if isinstance(slug_value, list) and slug_value:
|
||||
custom_slug = slug_value[0]
|
||||
elif isinstance(slug_value, str):
|
||||
custom_slug = slug_value
|
||||
|
||||
# Normalize and extract properties
|
||||
try:
|
||||
properties = normalize_properties(data)
|
||||
content = extract_content(properties)
|
||||
title = extract_title(properties)
|
||||
tags = extract_tags(properties)
|
||||
published_date = extract_published_date(properties)
|
||||
```
|
||||
|
||||
### Why This Fix Works
|
||||
|
||||
1. **Extracts before filtering**: Gets `mp-slug` from raw request data before `normalize_properties()` filters it out
|
||||
2. **Handles both formats**:
|
||||
- Form-encoded: `mp-slug` is a list `["slug-value"]`
|
||||
- JSON: `mp-slug` can be string `"slug-value"` or list `["slug-value"]`
|
||||
3. **Preserves existing flow**: The `custom_slug` variable was already being passed to `create_note()` correctly
|
||||
4. **Architecturally correct**: Treats `mp-slug` as a server parameter (not a property), which aligns with Micropub spec
|
||||
|
||||
## Test Results
|
||||
|
||||
### Micropub Test Suite
|
||||
All 13 Micropub tests passed:
|
||||
```
|
||||
tests/test_micropub.py::test_micropub_no_token PASSED
|
||||
tests/test_micropub.py::test_micropub_invalid_token PASSED
|
||||
tests/test_micropub.py::test_micropub_insufficient_scope PASSED
|
||||
tests/test_micropub.py::test_micropub_create_note_form PASSED
|
||||
tests/test_micropub.py::test_micropub_create_note_json PASSED
|
||||
tests/test_micropub.py::test_micropub_create_with_name PASSED
|
||||
tests/test_micropub.py::test_micropub_create_with_categories PASSED
|
||||
tests/test_micropub.py::test_micropub_create_with_custom_slug_form PASSED # NEW
|
||||
tests/test_micropub.py::test_micropub_create_with_custom_slug_json PASSED # NEW
|
||||
tests/test_micropub.py::test_micropub_query_config PASSED
|
||||
tests/test_micropub.py::test_micropub_query_source PASSED
|
||||
tests/test_micropub.py::test_micropub_missing_content PASSED
|
||||
tests/test_micropub.py::test_micropub_unsupported_action PASSED
|
||||
```
|
||||
|
||||
### New Test Coverage
|
||||
|
||||
**Test 1: Form-encoded with custom slug**
|
||||
- Request: `POST /micropub` with `content=...&mp-slug=my-custom-slug`
|
||||
- Verifies: Location header ends with `/notes/my-custom-slug`
|
||||
- Verifies: Note exists in database with correct slug
|
||||
|
||||
**Test 2: JSON with custom slug**
|
||||
- Request: `POST /micropub` with JSON body including `"mp-slug": "json-custom-slug"`
|
||||
- Verifies: Location header ends with `/notes/json-custom-slug`
|
||||
- Verifies: Note exists in database with correct slug
|
||||
|
||||
### Regression Testing
|
||||
|
||||
All existing Micropub tests continue to pass, confirming:
|
||||
- Authentication still works correctly
|
||||
- Scope checking still works correctly
|
||||
- Auto-generated slugs still work when no `mp-slug` provided
|
||||
- Content extraction still works correctly
|
||||
- Title and category handling still works correctly
|
||||
|
||||
## Validation Against Requirements
|
||||
|
||||
Per the architect's bug report (`docs/reports/custom-slug-bug-diagnosis.md`):
|
||||
|
||||
- [x] Extract `mp-slug` from raw request data
|
||||
- [x] Extract BEFORE calling `normalize_properties()`
|
||||
- [x] Handle both form-encoded (list) and JSON (string or list) formats
|
||||
- [x] Pass `custom_slug` to `create_note()`
|
||||
- [x] Add tests for both request formats
|
||||
- [x] Ensure existing tests still pass
|
||||
|
||||
## Architecture Compliance
|
||||
|
||||
The fix maintains architectural correctness:
|
||||
|
||||
1. **Separation of Concerns**: `mp-slug` is correctly treated as a server extension parameter, not a Micropub property
|
||||
2. **Existing Validation Pipeline**: The slug still goes through all validation in `create_note()`:
|
||||
- Reserved slug checking
|
||||
- Uniqueness checking with suffix generation if needed
|
||||
- Sanitization
|
||||
3. **No Breaking Changes**: All existing functionality preserved
|
||||
4. **Micropub Spec Compliance**: Aligns with how `mp-*` extensions should be handled
|
||||
|
||||
## Deployment Notes
|
||||
|
||||
### What to Test in Production
|
||||
|
||||
1. **Create note with custom slug via Quill**:
|
||||
- Use Quill client to create a note
|
||||
- Specify a custom slug in the slug field
|
||||
- Verify the created note uses your specified slug
|
||||
|
||||
2. **Create note without custom slug**:
|
||||
- Create a note without specifying a slug
|
||||
- Verify auto-generation still works
|
||||
|
||||
3. **Reserved slug handling**:
|
||||
- Try to create a note with slug "api" or "admin"
|
||||
- Should be rejected with validation error
|
||||
|
||||
4. **Duplicate slug handling**:
|
||||
- Create a note with slug "test-slug"
|
||||
- Try to create another with the same slug
|
||||
- Should get "test-slug-xxxx" with random suffix
|
||||
|
||||
### Known Issues
|
||||
|
||||
None. The fix is clean and complete.
|
||||
|
||||
### Version Impact
|
||||
|
||||
This fix will be included in **v1.1.0-rc.2** (or next release).
|
||||
|
||||
## Git Information
|
||||
|
||||
**Branch**: `bugfix/custom-slug-extraction`
|
||||
**Commit**: 894e5e3
|
||||
**Commit Message**: "fix: Extract mp-slug before property normalization"
|
||||
|
||||
**Files Changed**:
|
||||
- `starpunk/micropub.py` (69 insertions, 8 deletions)
|
||||
- `tests/test_micropub.py` (added 2 comprehensive tests)
|
||||
|
||||
## Next Steps
|
||||
|
||||
1. Merge `bugfix/custom-slug-extraction` into `main`
|
||||
2. Deploy to production
|
||||
3. Test with Quill client in production environment
|
||||
4. Update CHANGELOG.md with fix details
|
||||
5. Close any related issue tickets
|
||||
|
||||
## References
|
||||
|
||||
- **Bug Diagnosis**: `/home/phil/Projects/starpunk/docs/reports/custom-slug-bug-diagnosis.md`
|
||||
- **Micropub Spec**: https://www.w3.org/TR/micropub/
|
||||
- **Related ADR**: ADR-029 (Micropub Property Mapping)
|
||||
|
||||
## Conclusion
|
||||
|
||||
The custom slug feature is now fully functional. The bug was a simple timing issue in the extraction logic - trying to get `mp-slug` after it had been filtered out. The fix is clean, well-tested, and maintains all existing functionality while enabling the custom slug feature as originally designed.
|
||||
|
||||
The implementation follows the architect's design exactly and adds comprehensive test coverage for future regression prevention.
|
||||
@@ -287,6 +287,17 @@ def handle_create(data: dict, token_info: dict):
|
||||
"insufficient_scope", "Token lacks create scope", status_code=403
|
||||
)
|
||||
|
||||
# Extract mp-slug BEFORE normalizing properties (it's not a property!)
|
||||
# mp-slug is a Micropub server extension parameter that gets filtered during normalization
|
||||
custom_slug = None
|
||||
if isinstance(data, dict) and 'mp-slug' in data:
|
||||
# Handle both form-encoded (list) and JSON (could be string or list)
|
||||
slug_value = data.get('mp-slug')
|
||||
if isinstance(slug_value, list) and slug_value:
|
||||
custom_slug = slug_value[0]
|
||||
elif isinstance(slug_value, str):
|
||||
custom_slug = slug_value
|
||||
|
||||
# Normalize and extract properties
|
||||
try:
|
||||
properties = normalize_properties(data)
|
||||
@@ -295,14 +306,6 @@ def handle_create(data: dict, token_info: dict):
|
||||
tags = extract_tags(properties)
|
||||
published_date = extract_published_date(properties)
|
||||
|
||||
# Extract custom slug if provided (Micropub extension)
|
||||
custom_slug = None
|
||||
if 'mp-slug' in properties:
|
||||
# mp-slug is an array in Micropub format
|
||||
slug_values = properties.get('mp-slug', [])
|
||||
if slug_values and len(slug_values) > 0:
|
||||
custom_slug = slug_values[0]
|
||||
|
||||
except MicropubValidationError as e:
|
||||
raise e
|
||||
except Exception as e:
|
||||
|
||||
@@ -188,6 +188,64 @@ def test_micropub_create_with_categories(client, app, mock_valid_token):
|
||||
assert 'Location' in response.headers
|
||||
|
||||
|
||||
def test_micropub_create_with_custom_slug_form(client, app, mock_valid_token):
|
||||
"""Test creating a note with custom slug via form-encoded request"""
|
||||
with patch('starpunk.routes.micropub.verify_external_token', mock_valid_token):
|
||||
response = client.post(
|
||||
'/micropub',
|
||||
data={
|
||||
'h': 'entry',
|
||||
'content': 'This is a test for custom slugs',
|
||||
'mp-slug': 'my-custom-slug'
|
||||
},
|
||||
headers={'Authorization': 'Bearer valid_token'}
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert 'Location' in response.headers
|
||||
|
||||
# Verify the custom slug was used
|
||||
location = response.headers['Location']
|
||||
assert location.endswith('/notes/my-custom-slug')
|
||||
|
||||
# Verify note exists with the custom slug
|
||||
with app.app_context():
|
||||
note = get_note('my-custom-slug')
|
||||
assert note is not None
|
||||
assert note.slug == 'my-custom-slug'
|
||||
assert note.content == 'This is a test for custom slugs'
|
||||
|
||||
|
||||
def test_micropub_create_with_custom_slug_json(client, app, mock_valid_token):
|
||||
"""Test creating a note with custom slug via JSON request"""
|
||||
with patch('starpunk.routes.micropub.verify_external_token', mock_valid_token):
|
||||
response = client.post(
|
||||
'/micropub',
|
||||
json={
|
||||
'type': ['h-entry'],
|
||||
'properties': {
|
||||
'content': ['JSON test with custom slug']
|
||||
},
|
||||
'mp-slug': 'json-custom-slug'
|
||||
},
|
||||
headers={'Authorization': 'Bearer valid_token'}
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert 'Location' in response.headers
|
||||
|
||||
# Verify the custom slug was used
|
||||
location = response.headers['Location']
|
||||
assert location.endswith('/notes/json-custom-slug')
|
||||
|
||||
# Verify note exists with the custom slug
|
||||
with app.app_context():
|
||||
note = get_note('json-custom-slug')
|
||||
assert note is not None
|
||||
assert note.slug == 'json-custom-slug'
|
||||
assert note.content == 'JSON test with custom slug'
|
||||
|
||||
|
||||
# Query Tests
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user