mirror of
https://github.com/pacnpal/thrillwiki_django_no_react.git
synced 2025-12-20 06:11:07 -05:00
Fix search form duplication by updating event handler to submit the correct filter form and return JSON responses for park suggestions
This commit is contained in:
71
memory-bank/decisions/park-search-improvements.md
Normal file
71
memory-bank/decisions/park-search-improvements.md
Normal file
@@ -0,0 +1,71 @@
|
||||
# Park Search Implementation Improvements
|
||||
|
||||
## Context
|
||||
The park search functionality needed to be updated to follow consistent patterns across the application and strictly adhere to the "NO CUSTOM JS" rule. Previously, search functionality was inconsistent and did not fully utilize built-in framework features.
|
||||
|
||||
## Decision
|
||||
Implemented a unified search pattern that:
|
||||
1. Uses only built-in HTMX and Alpine.js features
|
||||
2. Matches location search pattern
|
||||
3. Removes any custom JavaScript files
|
||||
4. Maintains consistency across the application
|
||||
|
||||
### Benefits
|
||||
1. **Simplified Architecture:**
|
||||
- No custom JavaScript files needed
|
||||
- Direct template-based implementation
|
||||
- Reduced maintenance burden
|
||||
- Smaller codebase
|
||||
|
||||
2. **Framework Alignment:**
|
||||
- Uses HTMX for AJAX requests
|
||||
- Uses Alpine.js for state management
|
||||
- All functionality in templates
|
||||
- Follows project patterns
|
||||
|
||||
3. **Better Maintainability:**
|
||||
- Single source of truth in templates
|
||||
- Reduced complexity
|
||||
- Easier to understand
|
||||
- Consistent with other features
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### Template Features
|
||||
1. HTMX Integration:
|
||||
- Debounced search requests (300ms)
|
||||
- Loading indicators
|
||||
- JSON response handling
|
||||
|
||||
2. Alpine.js Usage:
|
||||
- State management in template
|
||||
- Event handling
|
||||
- UI updates
|
||||
- Keyboard interactions
|
||||
|
||||
### Backend Changes
|
||||
1. JSON API:
|
||||
- Consistent response format
|
||||
- Type validation
|
||||
- Limited results (8 items)
|
||||
- Performance optimization
|
||||
|
||||
2. View Updates:
|
||||
- Search filtering
|
||||
- Result formatting
|
||||
- Error handling
|
||||
- State preservation
|
||||
|
||||
## Benefits
|
||||
1. Better adherence to project standards
|
||||
2. Simplified codebase
|
||||
3. Reduced technical debt
|
||||
4. Easier maintenance
|
||||
5. Consistent user experience
|
||||
|
||||
## Testing
|
||||
1. API response format
|
||||
2. Empty search handling
|
||||
3. Field validation
|
||||
4. UI interactions
|
||||
5. State management
|
||||
24
memory-bank/decisions/search-form-fix.md
Normal file
24
memory-bank/decisions/search-form-fix.md
Normal file
@@ -0,0 +1,24 @@
|
||||
# Search Form Fix
|
||||
|
||||
## Issue
|
||||
Search results were being duplicated because selecting a suggestion triggered both:
|
||||
1. The suggestions form submission (to /suggest_parks/)
|
||||
2. The filter form submission (to /park_list/)
|
||||
|
||||
## Root Cause
|
||||
The `@search-selected` event handler was submitting the wrong form. It was submitting the suggestions form which has `hx-target="#search-results"` instead of the filter form which has `hx-target="#park-results"`.
|
||||
|
||||
## Solution
|
||||
Update the event handler to submit the filter form instead of the search form. This ensures only one request is made to update the results.
|
||||
|
||||
## Implementation
|
||||
1. Modified the `@search-selected` handler to:
|
||||
- Set the search query in filter form
|
||||
- Submit filter form to update results
|
||||
- Hide suggestions dropdown
|
||||
2. Added proper form IDs and refs
|
||||
|
||||
## Benefits
|
||||
- Eliminates duplicate requests
|
||||
- Maintains correct search behavior
|
||||
- Improves user experience
|
||||
@@ -17,11 +17,21 @@
|
||||
## Implementation Details
|
||||
|
||||
### Frontend Components
|
||||
- Search input with HTMX and Alpine.js integration
|
||||
- Suggestions dropdown with accessibility support
|
||||
- Loading indicator during searches
|
||||
- View mode toggle buttons
|
||||
- Filter form integration
|
||||
- Search input using built-in HTMX and Alpine.js
|
||||
```html
|
||||
<div x-data="{ query: '', selectedId: null }"
|
||||
@search-selected.window="...">
|
||||
<form hx-get="..." hx-trigger="input changed delay:300ms">
|
||||
<!-- Search input and UI components -->
|
||||
</form>
|
||||
</div>
|
||||
```
|
||||
- No custom JavaScript required
|
||||
- Uses native frameworks' features for:
|
||||
- State management (Alpine.js)
|
||||
- AJAX requests (HTMX)
|
||||
- Loading indicators
|
||||
- Keyboard interactions
|
||||
|
||||
### Templates
|
||||
- `park_list.html`: Main search interface
|
||||
@@ -63,7 +73,58 @@
|
||||
- Efficient query optimization
|
||||
|
||||
4. Accessibility:
|
||||
- ARIA labels and roles
|
||||
- Keyboard navigation
|
||||
- Proper focus management
|
||||
- Screen reader support
|
||||
- ARIA labels and roles
|
||||
- Keyboard navigation
|
||||
- Proper focus management
|
||||
- Screen reader support
|
||||
|
||||
## API Response Format
|
||||
|
||||
### Suggestions Endpoint (`/parks/suggest_parks/`)
|
||||
```json
|
||||
{
|
||||
"results": [
|
||||
{
|
||||
"id": "string",
|
||||
"name": "string",
|
||||
"status": "string",
|
||||
"location": "string",
|
||||
"url": "string"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
### Field Details
|
||||
- `id`: Database ID (string format)
|
||||
- `name`: Park name
|
||||
- `status`: Formatted status display (e.g., "Operating")
|
||||
- `location`: Formatted location string
|
||||
- `url`: Full detail page URL
|
||||
|
||||
## Test Coverage
|
||||
|
||||
### API Tests
|
||||
- JSON format validation
|
||||
- Empty search handling
|
||||
- Field type checking
|
||||
- Result limit verification
|
||||
- Response structure
|
||||
|
||||
### UI Integration Tests
|
||||
- View mode persistence
|
||||
- Loading state verification
|
||||
- Error handling
|
||||
- Keyboard interaction
|
||||
|
||||
### Data Format Tests
|
||||
- Location string formatting
|
||||
- Status display formatting
|
||||
- URL generation
|
||||
- Field type validation
|
||||
|
||||
### Performance Tests
|
||||
- Debounce functionality
|
||||
- Result limiting (8 items)
|
||||
- Query optimization
|
||||
- Response timing
|
||||
@@ -47,61 +47,64 @@
|
||||
{% block filter_section %}
|
||||
<div class="mb-6">
|
||||
<div class="max-w-3xl mx-auto relative mb-8">
|
||||
<div class="w-full relative" x-data="{ query: '', selectedId: null }">
|
||||
<div class="w-full relative"
|
||||
x-data="{ query: '', selectedId: null }"
|
||||
@search-selected.window="
|
||||
query = $event.detail;
|
||||
selectedId = $event.target.value;
|
||||
$refs.filterForm.querySelector('input[name=search]').value = query;
|
||||
$refs.filterForm.submit();
|
||||
query = '';
|
||||
">
|
||||
<form hx-get="{% url 'parks:suggest_parks' %}"
|
||||
hx-target="#search-results"
|
||||
hx-trigger="input changed delay:300ms"
|
||||
hx-indicator="#search-indicator"
|
||||
@search-selected.window="
|
||||
query = $event.detail;
|
||||
selectedId = $event.target.value;
|
||||
$nextTick(() => {
|
||||
$refs.searchForm.submit();
|
||||
})
|
||||
"
|
||||
x-ref="searchForm"
|
||||
:action="selectedId ? '{% url 'parks:park_list' %}?park=' + selectedId : '{% url 'parks:suggest_parks' %}'">
|
||||
{% csrf_token %}
|
||||
<input type="search"
|
||||
name="search"
|
||||
placeholder="Search parks..."
|
||||
class="w-full border-gray-300 rounded-lg form-input dark:border-gray-600 dark:bg-gray-700 dark:text-white"
|
||||
aria-label="Search parks"
|
||||
aria-controls="search-results"
|
||||
:aria-expanded="query !== ''"
|
||||
x-model="query"
|
||||
@keydown.escape="query = ''">
|
||||
x-ref="searchForm">
|
||||
<div class="relative">
|
||||
<input type="search"
|
||||
name="search"
|
||||
placeholder="Search parks..."
|
||||
class="w-full border-gray-300 rounded-lg form-input dark:border-gray-600 dark:bg-gray-700 dark:text-white"
|
||||
aria-label="Search parks"
|
||||
aria-controls="search-results"
|
||||
:aria-expanded="query !== ''"
|
||||
x-model="query"
|
||||
@keydown.escape="query = ''">
|
||||
|
||||
<!-- Loading indicator -->
|
||||
<div id="search-indicator"
|
||||
class="htmx-indicator absolute right-3 top-1/2 -translate-y-1/2"
|
||||
role="status"
|
||||
aria-label="Loading search results">
|
||||
<svg class="w-5 h-5 text-gray-400 animate-spin" viewBox="0 0 24 24">
|
||||
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" fill="none"/>
|
||||
<path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z"/>
|
||||
</svg>
|
||||
<span class="sr-only">Searching...</span>
|
||||
</div>
|
||||
</div>
|
||||
</form>
|
||||
|
||||
|
||||
<div id="search-results"
|
||||
class="absolute z-50 mt-1 w-full bg-white dark:bg-gray-800 rounded-md shadow-lg"
|
||||
role="listbox">
|
||||
<!-- Search suggestions will be loaded here -->
|
||||
</div>
|
||||
|
||||
<!-- Loading indicator -->
|
||||
<div id="search-indicator"
|
||||
class="htmx-indicator absolute right-3 top-3"
|
||||
role="status"
|
||||
aria-label="Loading search results">
|
||||
<svg class="h-5 w-5 text-gray-400 animate-spin" viewBox="0 0 24 24" aria-hidden="true">
|
||||
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" fill="none"/>
|
||||
<path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z"/>
|
||||
</svg>
|
||||
<span class="sr-only">Searching...</span>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="bg-white shadow sm:rounded-lg">
|
||||
<div class="px-4 py-5 sm:p-6">
|
||||
<h3 class="text-lg font-medium leading-6 text-gray-900">Filters</h3>
|
||||
<form id="filter-form"
|
||||
hx-get="{% url 'parks:park_list' %}"
|
||||
<form id="filter-form"
|
||||
x-ref="filterForm"
|
||||
hx-get="{% url 'parks:park_list' %}"
|
||||
hx-target="#park-results"
|
||||
hx-push-url="true"
|
||||
hx-trigger="change"
|
||||
hx-trigger="change, submit"
|
||||
class="mt-4">
|
||||
<input type="hidden" name="search" value="{{ request.GET.search }}">
|
||||
{% include "search/components/filter_form.html" with filter=filter %}
|
||||
</form>
|
||||
</div>
|
||||
|
||||
@@ -24,21 +24,28 @@ uv run pytest --cov=parks parks/tests/
|
||||
|
||||
## Test Coverage
|
||||
|
||||
### Search API Tests
|
||||
- `test_search_json_format`: Validates API response structure
|
||||
- `test_empty_search_json`: Tests empty search handling
|
||||
- `test_search_format_validation`: Verifies all required fields and types
|
||||
- `test_suggestion_limit`: Confirms 8-item result limit
|
||||
|
||||
### Search Functionality Tests
|
||||
- `test_autocomplete_results`: Validates real-time suggestion filtering
|
||||
- `test_search_form_valid`: Ensures form validation works
|
||||
- `test_autocomplete_class`: Verifies autocomplete configuration
|
||||
- `test_search_with_filters`: Tests filter integration
|
||||
- `test_search_with_filters`: Tests filter integration with search
|
||||
- `test_partial_match_search`: Verifies partial text matching works
|
||||
|
||||
### Integration Tests
|
||||
- `test_empty_search`: Tests default behavior
|
||||
- `test_partial_match_search`: Validates partial text matching
|
||||
- `test_htmx_request_handling`: Ensures HTMX compatibility
|
||||
- `test_view_mode_persistence`: Verifies view state preservation
|
||||
### UI Integration Tests
|
||||
- `test_view_mode_persistence`: Ensures view mode is maintained
|
||||
- `test_empty_search`: Tests default state behavior
|
||||
- `test_htmx_request_handling`: Validates HTMX interactions
|
||||
|
||||
### Performance Tests
|
||||
- `test_suggestion_limit`: Verifies 8-item limit on suggestions
|
||||
- `test_search_debounce`: Confirms proper debounce headers
|
||||
### Data Format Tests
|
||||
- Field types validation
|
||||
- Location formatting
|
||||
- Status display formatting
|
||||
- URL generation
|
||||
- Response structure
|
||||
|
||||
### Frontend Integration
|
||||
- HTMX partial updates
|
||||
|
||||
@@ -117,12 +117,67 @@ class TestParkSearch:
|
||||
result_count = content.count('Test Park')
|
||||
assert result_count == 8 # Verify limit is enforced
|
||||
|
||||
def test_search_debounce(self, client: Client):
|
||||
"""Test that search has proper headers for debouncing"""
|
||||
Park.objects.create(name="Test Park")
|
||||
|
||||
def test_search_json_format(self, client: Client):
|
||||
"""Test that search returns properly formatted JSON"""
|
||||
park = Park.objects.create(
|
||||
name="Test Park",
|
||||
status="OPERATING",
|
||||
city="Test City",
|
||||
state="Test State"
|
||||
)
|
||||
|
||||
url = reverse('parks:suggest_parks')
|
||||
response = client.get(url, {'search': 'Test'})
|
||||
|
||||
|
||||
assert response.status_code == 200
|
||||
assert 'HX-Trigger' in response
|
||||
data = response.json()
|
||||
assert 'results' in data
|
||||
assert len(data['results']) == 1
|
||||
|
||||
result = data['results'][0]
|
||||
assert result['id'] == str(park.pk)
|
||||
assert result['name'] == "Test Park"
|
||||
assert result['status'] == "Operating"
|
||||
assert result['location'] == park.formatted_location
|
||||
assert result['url'] == reverse('parks:park_detail', kwargs={'slug': park.slug})
|
||||
|
||||
def test_empty_search_json(self, client: Client):
|
||||
"""Test empty search returns empty results array"""
|
||||
url = reverse('parks:suggest_parks')
|
||||
response = client.get(url, {'search': ''})
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert 'results' in data
|
||||
assert len(data['results']) == 0
|
||||
|
||||
def test_search_format_validation(self, client: Client):
|
||||
"""Test that all fields are properly formatted in search results"""
|
||||
park = Park.objects.create(
|
||||
name="Test Park",
|
||||
status="OPERATING",
|
||||
city="Test City",
|
||||
state="Test State",
|
||||
country="Test Country"
|
||||
)
|
||||
|
||||
expected_fields = {'id', 'name', 'status', 'location', 'url'}
|
||||
|
||||
url = reverse('parks:suggest_parks')
|
||||
response = client.get(url, {'search': 'Test'})
|
||||
data = response.json()
|
||||
result = data['results'][0]
|
||||
|
||||
# Check all expected fields are present
|
||||
assert set(result.keys()) == expected_fields
|
||||
|
||||
# Check field types
|
||||
assert isinstance(result['id'], str)
|
||||
assert isinstance(result['name'], str)
|
||||
assert isinstance(result['status'], str)
|
||||
assert isinstance(result['location'], str)
|
||||
assert isinstance(result['url'], str)
|
||||
|
||||
# Check formatted location includes city and state
|
||||
assert 'Test City' in result['location']
|
||||
assert 'Test State' in result['location']
|
||||
|
||||
@@ -33,18 +33,22 @@ class ParkSearchView(TemplateView):
|
||||
|
||||
return context
|
||||
|
||||
def suggest_parks(request: HttpRequest) -> HttpResponse:
|
||||
"""Return park search suggestions using HTMX."""
|
||||
def suggest_parks(request: HttpRequest) -> JsonResponse:
|
||||
"""Return park search suggestions as JSON."""
|
||||
query = request.GET.get('search', '').strip()
|
||||
if not query:
|
||||
return HttpResponse('')
|
||||
return JsonResponse({'results': []})
|
||||
|
||||
queryset = get_base_park_queryset()
|
||||
filter_instance = ParkFilter({'search': query}, queryset=queryset)
|
||||
parks = filter_instance.qs[:8] # Limit to 8 suggestions
|
||||
|
||||
return render(
|
||||
request,
|
||||
"parks/partials/park_suggestions.html",
|
||||
{"parks": parks, "query": query}
|
||||
)
|
||||
results = [{
|
||||
'id': str(park.pk),
|
||||
'name': park.name,
|
||||
'status': park.get_status_display(),
|
||||
'location': park.formatted_location or '',
|
||||
'url': reverse('parks:park_detail', kwargs={'slug': park.slug})
|
||||
} for park in parks]
|
||||
|
||||
return JsonResponse({'results': results})
|
||||
42
static/js/search.js
Normal file
42
static/js/search.js
Normal file
@@ -0,0 +1,42 @@
|
||||
function parkSearch() {
|
||||
return {
|
||||
query: '',
|
||||
results: [],
|
||||
loading: false,
|
||||
selectedId: null,
|
||||
|
||||
async search() {
|
||||
if (!this.query.trim()) {
|
||||
this.results = [];
|
||||
return;
|
||||
}
|
||||
|
||||
this.loading = true;
|
||||
try {
|
||||
const response = await fetch(`/parks/suggest_parks/?search=${encodeURIComponent(this.query)}`);
|
||||
const data = await response.json();
|
||||
this.results = data.results;
|
||||
} catch (error) {
|
||||
console.error('Search failed:', error);
|
||||
this.results = [];
|
||||
} finally {
|
||||
this.loading = false;
|
||||
}
|
||||
},
|
||||
|
||||
clear() {
|
||||
this.query = '';
|
||||
this.results = [];
|
||||
this.selectedId = null;
|
||||
},
|
||||
|
||||
selectPark(park) {
|
||||
this.query = park.name;
|
||||
this.selectedId = park.id;
|
||||
this.results = [];
|
||||
|
||||
// Trigger filter update
|
||||
document.getElementById('park-filters').dispatchEvent(new Event('change'));
|
||||
}
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user