Implement Phase 5: Product Owner Dashboard (User Story 3)
Add complete dashboard functionality for product owners and administrators to view, filter, search, and manage feedback submissions following Test-First Discipline. Tests (T093-T105): - Add 12 contract tests for dashboard routes (authentication, listing, filtering, search, detail view, status updates, attachment downloads, access control) - Add 2 integration tests for complete dashboard workflow and access control enforcement - All tests written first and verified to fail before implementation Services (T111-T118): - Enhance FeedbackStorageService with load_feedback_list() for pagination, filtering, searching, and sorting - Add load_feedback_detail() to load complete feedback with attachments and analysis - Add update_feedback_status_by_id() for status management - Add get_attachment_path() with path traversal prevention Routes (T119-T134): - Implement GET /dashboard with filters, search, and pagination (50 items/page) - Implement GET /feedback/<id> detail view with role-based access control - Implement POST /feedback/<id>/status for status updates - Implement GET /feedback/<id>/attachment/<filename> for secure file downloads - Add access control helpers (administrators see all products, owners see only assigned) Templates (T135-T136): - Create dashboard/list.html with filter form, search, and pagination - Create dashboard/detail.html with status update form and attachment links - Create error_403.html for access denied - Create error_404.html for not found Integration & Bug Fixes: - Update auth routes to remove /auth prefix and redirect to dashboard after login - Update Feedback.VALID_STATUSES to include dashboard statuses (in_progress, resolved, closed) - Register error handlers for 403 and 404 in app factory - Fix test fixtures to use correct users.yaml format and User.hash_password() Test Results: 39 passed, 1 skipped (all Phase 5 tests passing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude
@@ -0,0 +1,324 @@
|
||||
"""Contract tests for dashboard routes"""
|
||||
import pytest
|
||||
import os
|
||||
import yaml
|
||||
import io
|
||||
from app.models.user import User
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_product(app):
|
||||
"""Create a test product with feedback"""
|
||||
with app.app_context():
|
||||
# Create test product directory and config
|
||||
product_dir = os.path.join(app.config['DATA_DIR'], 'products', 'test-product')
|
||||
os.makedirs(product_dir, exist_ok=True)
|
||||
|
||||
# Create product config
|
||||
config_file = os.path.join(product_dir, 'config.yaml')
|
||||
config_data = {
|
||||
'product_id': 'test-product',
|
||||
'name': 'Test Product',
|
||||
'submission_url_slug': 'test-product',
|
||||
'owner_language': 'en',
|
||||
'assigned_owner_ids': ['usr_owner1'],
|
||||
'status': 'active'
|
||||
}
|
||||
|
||||
with open(config_file, 'w') as f:
|
||||
yaml.dump(config_data, f)
|
||||
|
||||
# Create feedback directory
|
||||
feedback_dir = os.path.join(product_dir, 'feedback')
|
||||
os.makedirs(feedback_dir, exist_ok=True)
|
||||
|
||||
# Create test feedback
|
||||
feedback_id = 'test-feedback-001'
|
||||
feedback_path = os.path.join(feedback_dir, feedback_id)
|
||||
os.makedirs(feedback_path, exist_ok=True)
|
||||
|
||||
# Create feedback metadata
|
||||
metadata = {
|
||||
'feedback_id': feedback_id,
|
||||
'product_id': 'test-product',
|
||||
'status': 'new',
|
||||
'submitted_at': '2025-10-16T10:00:00Z',
|
||||
'has_attachments': True,
|
||||
'attachment_count': 1,
|
||||
'category': 'bug',
|
||||
'original_language': 'en'
|
||||
}
|
||||
|
||||
with open(os.path.join(feedback_path, 'metadata.yaml'), 'w') as f:
|
||||
yaml.dump(metadata, f)
|
||||
|
||||
# Create feedback content
|
||||
with open(os.path.join(feedback_path, 'content.txt'), 'w') as f:
|
||||
f.write('Test feedback content')
|
||||
|
||||
# Create attachments directory and file
|
||||
attachments_dir = os.path.join(feedback_path, 'attachments')
|
||||
os.makedirs(attachments_dir, exist_ok=True)
|
||||
|
||||
with open(os.path.join(attachments_dir, 'test.txt'), 'w') as f:
|
||||
f.write('test attachment content')
|
||||
|
||||
yield {
|
||||
'product_id': 'test-product',
|
||||
'feedback_id': feedback_id
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_users(app):
|
||||
"""Create test users (admin and product owner)"""
|
||||
users_file = os.path.join(app.config['DATA_DIR'], 'users.yaml')
|
||||
|
||||
# User model expects format: {'users': {user_id: user_data}}
|
||||
users_data = {
|
||||
'users': {
|
||||
'usr_admin': {
|
||||
'user_id': 'usr_admin',
|
||||
'username': 'admin',
|
||||
'email': 'admin@example.com',
|
||||
'password_hash': User.hash_password('admin123'),
|
||||
'role': 'administrator',
|
||||
'product_ids': [],
|
||||
'is_active': True
|
||||
},
|
||||
'usr_owner1': {
|
||||
'user_id': 'usr_owner1',
|
||||
'username': 'owner1',
|
||||
'email': 'owner1@example.com',
|
||||
'password_hash': User.hash_password('owner123'),
|
||||
'role': 'product_owner',
|
||||
'product_ids': ['test-product'],
|
||||
'is_active': True
|
||||
},
|
||||
'usr_owner2': {
|
||||
'user_id': 'usr_owner2',
|
||||
'username': 'owner2',
|
||||
'email': 'owner2@example.com',
|
||||
'password_hash': User.hash_password('owner456'),
|
||||
'role': 'product_owner',
|
||||
'product_ids': ['other-product'],
|
||||
'is_active': True
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
with open(users_file, 'w') as f:
|
||||
yaml.dump(users_data, f)
|
||||
|
||||
yield users_data
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_login(client):
|
||||
"""T093: Contract test for GET /login
|
||||
|
||||
Expected: 200 OK with HTML login form
|
||||
"""
|
||||
response = client.get('/login')
|
||||
|
||||
assert response.status_code == 200
|
||||
assert b'<form' in response.data
|
||||
assert b'username' in response.data.lower() or b'email' in response.data.lower()
|
||||
assert b'password' in response.data.lower()
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_post_login_valid_credentials(client, app, test_users):
|
||||
"""T094: Contract test for POST /login with valid credentials
|
||||
|
||||
Expected: 302 redirect to dashboard with session established
|
||||
"""
|
||||
data = {
|
||||
'username': 'admin',
|
||||
'password': 'admin123'
|
||||
}
|
||||
|
||||
response = client.post('/login', data=data, follow_redirects=False)
|
||||
|
||||
# Should redirect (302) to dashboard or home
|
||||
assert response.status_code == 302
|
||||
|
||||
# Follow redirect and verify user is logged in
|
||||
response_redirected = client.get(response.location, follow_redirects=True)
|
||||
assert response_redirected.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_post_login_invalid_credentials(client, app, test_users):
|
||||
"""T095: Contract test for POST /login with invalid credentials (401)
|
||||
|
||||
Expected: 401 Unauthorized or redirect back to login with error message
|
||||
"""
|
||||
data = {
|
||||
'username': 'admin',
|
||||
'password': 'wrongpassword'
|
||||
}
|
||||
|
||||
response = client.post('/login', data=data)
|
||||
|
||||
# Should return error (401 or 200 with error message)
|
||||
assert response.status_code in [200, 401]
|
||||
|
||||
if response.status_code == 200:
|
||||
# If returns 200, should show error message
|
||||
assert b'invalid' in response.data.lower() or b'incorrect' in response.data.lower() or b'error' in response.data.lower()
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_logout(client, app, test_users):
|
||||
"""T096: Contract test for GET /logout
|
||||
|
||||
Expected: 302 redirect to login or home, session cleared
|
||||
"""
|
||||
# First login
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
# Then logout
|
||||
response = client.get('/logout', follow_redirects=False)
|
||||
|
||||
assert response.status_code == 302
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_dashboard_authenticated(client, app, test_users, test_product):
|
||||
"""T097: Contract test for GET /dashboard (authenticated)
|
||||
|
||||
Expected: 200 OK with dashboard showing feedback list
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
response = client.get('/dashboard')
|
||||
|
||||
assert response.status_code == 200
|
||||
assert b'feedback' in response.data.lower() or b'dashboard' in response.data.lower()
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_dashboard_unauthenticated(client):
|
||||
"""T098: Contract test for GET /dashboard (unauthenticated redirect)
|
||||
|
||||
Expected: 302 redirect to login page
|
||||
"""
|
||||
response = client.get('/dashboard', follow_redirects=False)
|
||||
|
||||
# Should redirect to login
|
||||
assert response.status_code == 302
|
||||
assert '/login' in response.location
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_dashboard_with_filters(client, app, test_users, test_product):
|
||||
"""T099: Contract test for GET /dashboard with filters
|
||||
|
||||
Expected: 200 OK with filtered feedback list
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
# Request with filters
|
||||
response = client.get('/dashboard?category=bug&status=new')
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_dashboard_with_search(client, app, test_users, test_product):
|
||||
"""T100: Contract test for GET /dashboard with search query
|
||||
|
||||
Expected: 200 OK with search results
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
# Request with search query
|
||||
response = client.get('/dashboard?search=test')
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_feedback_detail(client, app, test_users, test_product):
|
||||
"""T101: Contract test for GET /feedback/{id} detail view
|
||||
|
||||
Expected: 200 OK with feedback detail page showing content, metadata, attachments
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
feedback_id = test_product['feedback_id']
|
||||
response = client.get(f'/feedback/{feedback_id}')
|
||||
|
||||
assert response.status_code == 200
|
||||
assert b'Test feedback content' in response.data or b'feedback' in response.data.lower()
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_post_feedback_status_update(client, app, test_users, test_product):
|
||||
"""T102: Contract test for POST /feedback/{id}/status update
|
||||
|
||||
Expected: 200/302 success, metadata.yaml updated with new status
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
feedback_id = test_product['feedback_id']
|
||||
|
||||
data = {
|
||||
'status': 'in_progress'
|
||||
}
|
||||
|
||||
response = client.post(f'/feedback/{feedback_id}/status', data=data)
|
||||
|
||||
# Should succeed
|
||||
assert response.status_code in [200, 302]
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_get_attachment_download(client, app, test_users, test_product):
|
||||
"""T103: Contract test for GET /feedback/{id}/attachment/{filename} download
|
||||
|
||||
Expected: 200 OK with file content, correct Content-Disposition header
|
||||
"""
|
||||
# Login first
|
||||
client.post('/login', data={'username': 'admin', 'password': 'admin123'})
|
||||
|
||||
feedback_id = test_product['feedback_id']
|
||||
|
||||
response = client.get(f'/feedback/{feedback_id}/attachment/test.txt')
|
||||
|
||||
assert response.status_code == 200
|
||||
assert b'test attachment content' in response.data
|
||||
# Should have download headers
|
||||
assert 'Content-Disposition' in response.headers or 'content-disposition' in response.headers
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
def test_access_control_owner_products(client, app, test_users, test_product):
|
||||
"""T104: Contract test for access control (owner sees only assigned products)
|
||||
|
||||
Expected: Product owner can only access feedback for their assigned products
|
||||
"""
|
||||
# Login as owner1 (has access to test-product)
|
||||
client.post('/login', data={'username': 'owner1', 'password': 'owner123'})
|
||||
|
||||
feedback_id = test_product['feedback_id']
|
||||
|
||||
# Should have access to feedback from test-product
|
||||
response = client.get(f'/feedback/{feedback_id}')
|
||||
assert response.status_code == 200
|
||||
|
||||
# Logout
|
||||
client.get('/logout')
|
||||
|
||||
# Login as owner2 (only has access to other-product)
|
||||
client.post('/login', data={'username': 'owner2', 'password': 'owner456'})
|
||||
|
||||
# Should NOT have access to feedback from test-product
|
||||
response = client.get(f'/feedback/{feedback_id}')
|
||||
assert response.status_code == 403 # Forbidden
|
||||
@@ -0,0 +1,247 @@
|
||||
"""Integration test for complete dashboard access flow"""
|
||||
import pytest
|
||||
import os
|
||||
import yaml
|
||||
from app.models.user import User
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def test_product_with_feedback(app):
|
||||
"""Create a test product with multiple feedback items"""
|
||||
with app.app_context():
|
||||
# Create test product directory and config
|
||||
product_dir = os.path.join(app.config['DATA_DIR'], 'products', 'dashboard-test-product')
|
||||
os.makedirs(product_dir, exist_ok=True)
|
||||
|
||||
# Create product config
|
||||
config_file = os.path.join(product_dir, 'config.yaml')
|
||||
config_data = {
|
||||
'product_id': 'dashboard-test-product',
|
||||
'name': 'Dashboard Test Product',
|
||||
'submission_url_slug': 'dashboard-test-product',
|
||||
'owner_language': 'en',
|
||||
'assigned_owner_ids': ['usr_dashboard_owner'],
|
||||
'status': 'active'
|
||||
}
|
||||
|
||||
with open(config_file, 'w') as f:
|
||||
yaml.dump(config_data, f)
|
||||
|
||||
# Create feedback directory
|
||||
feedback_dir = os.path.join(product_dir, 'feedback')
|
||||
os.makedirs(feedback_dir, exist_ok=True)
|
||||
|
||||
# Create multiple test feedback items
|
||||
feedback_items = [
|
||||
{
|
||||
'id': 'feedback-bug-001',
|
||||
'category': 'bug',
|
||||
'status': 'new',
|
||||
'content': 'Found a critical bug in the login system',
|
||||
'has_attachments': True,
|
||||
'attachment': 'bug-screenshot.png'
|
||||
},
|
||||
{
|
||||
'id': 'feedback-feature-001',
|
||||
'category': 'feature_request',
|
||||
'status': 'new',
|
||||
'content': 'Please add dark mode to the application',
|
||||
'has_attachments': False,
|
||||
'attachment': None
|
||||
},
|
||||
{
|
||||
'id': 'feedback-bug-002',
|
||||
'category': 'bug',
|
||||
'status': 'in_progress',
|
||||
'content': 'Error when uploading large files',
|
||||
'has_attachments': True,
|
||||
'attachment': 'error.log'
|
||||
}
|
||||
]
|
||||
|
||||
for item in feedback_items:
|
||||
feedback_path = os.path.join(feedback_dir, item['id'])
|
||||
os.makedirs(feedback_path, exist_ok=True)
|
||||
|
||||
# Create metadata
|
||||
metadata = {
|
||||
'feedback_id': item['id'],
|
||||
'product_id': 'dashboard-test-product',
|
||||
'status': item['status'],
|
||||
'submitted_at': '2025-10-16T10:00:00Z',
|
||||
'has_attachments': item['has_attachments'],
|
||||
'attachment_count': 1 if item['has_attachments'] else 0,
|
||||
'category': item['category'],
|
||||
'original_language': 'en'
|
||||
}
|
||||
|
||||
with open(os.path.join(feedback_path, 'metadata.yaml'), 'w') as f:
|
||||
yaml.dump(metadata, f)
|
||||
|
||||
# Create content
|
||||
with open(os.path.join(feedback_path, 'content.txt'), 'w') as f:
|
||||
f.write(item['content'])
|
||||
|
||||
# Create attachment if needed
|
||||
if item['has_attachments']:
|
||||
attachments_dir = os.path.join(feedback_path, 'attachments')
|
||||
os.makedirs(attachments_dir, exist_ok=True)
|
||||
|
||||
with open(os.path.join(attachments_dir, item['attachment']), 'w') as f:
|
||||
f.write(f'Attachment content for {item["id"]}')
|
||||
|
||||
yield {
|
||||
'product_id': 'dashboard-test-product',
|
||||
'feedback_items': feedback_items
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def dashboard_test_users(app):
|
||||
"""Create test users for dashboard testing"""
|
||||
users_file = os.path.join(app.config['DATA_DIR'], 'users.yaml')
|
||||
|
||||
# User model expects format: {'users': {user_id: user_data}}
|
||||
users_data = {
|
||||
'users': {
|
||||
'usr_dashboard_owner': {
|
||||
'user_id': 'usr_dashboard_owner',
|
||||
'username': 'dashboard_owner',
|
||||
'email': 'dashboard@example.com',
|
||||
'password_hash': User.hash_password('dashboard123'),
|
||||
'role': 'product_owner',
|
||||
'product_ids': ['dashboard-test-product'],
|
||||
'is_active': True
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
with open(users_file, 'w') as f:
|
||||
yaml.dump(users_data, f)
|
||||
|
||||
yield users_data
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
def test_complete_dashboard_access_flow(client, app, test_product_with_feedback, dashboard_test_users):
|
||||
"""T105: Integration test for complete dashboard access flow
|
||||
|
||||
Test the entire product owner journey:
|
||||
1. Owner logs in with credentials
|
||||
2. Owner views dashboard with feedback list
|
||||
3. Owner filters feedback by category
|
||||
4. Owner searches for specific feedback
|
||||
5. Owner views feedback detail
|
||||
6. Owner downloads attachment
|
||||
7. Owner updates feedback status
|
||||
8. Owner logs out
|
||||
"""
|
||||
# Step 1: Login as product owner
|
||||
login_response = client.post('/login', data={
|
||||
'username': 'dashboard_owner',
|
||||
'password': 'dashboard123'
|
||||
}, follow_redirects=False)
|
||||
|
||||
assert login_response.status_code == 302 # Redirect after successful login
|
||||
|
||||
# Step 2: View dashboard with feedback list
|
||||
dashboard_response = client.get('/dashboard')
|
||||
assert dashboard_response.status_code == 200
|
||||
assert b'feedback' in dashboard_response.data.lower() or b'dashboard' in dashboard_response.data.lower()
|
||||
|
||||
# Verify feedback items are shown
|
||||
# (At least should show some feedback indicators)
|
||||
|
||||
# Step 3: Filter feedback by category (bug)
|
||||
filter_response = client.get('/dashboard?category=bug')
|
||||
assert filter_response.status_code == 200
|
||||
|
||||
# Step 4: Search for specific feedback
|
||||
search_response = client.get('/dashboard?search=login')
|
||||
assert search_response.status_code == 200
|
||||
|
||||
# Step 5: View feedback detail
|
||||
feedback_id = test_product_with_feedback['feedback_items'][0]['id']
|
||||
detail_response = client.get(f'/feedback/{feedback_id}')
|
||||
|
||||
assert detail_response.status_code == 200
|
||||
# Should show the feedback content
|
||||
assert b'Found a critical bug in the login system' in detail_response.data or b'feedback' in detail_response.data.lower()
|
||||
|
||||
# Step 6: Download attachment
|
||||
attachment_response = client.get(f'/feedback/{feedback_id}/attachment/bug-screenshot.png')
|
||||
|
||||
assert attachment_response.status_code == 200
|
||||
assert b'Attachment content' in attachment_response.data
|
||||
# Should have download headers
|
||||
assert 'Content-Disposition' in attachment_response.headers or 'content-disposition' in attachment_response.headers
|
||||
|
||||
# Step 7: Update feedback status
|
||||
status_update_response = client.post(f'/feedback/{feedback_id}/status', data={
|
||||
'status': 'in_progress'
|
||||
}, follow_redirects=False)
|
||||
|
||||
assert status_update_response.status_code in [200, 302]
|
||||
|
||||
# Verify status was updated in filesystem
|
||||
with app.app_context():
|
||||
data_dir = app.config['DATA_DIR']
|
||||
metadata_file = os.path.join(
|
||||
data_dir,
|
||||
'products',
|
||||
'dashboard-test-product',
|
||||
'feedback',
|
||||
feedback_id,
|
||||
'metadata.yaml'
|
||||
)
|
||||
|
||||
with open(metadata_file, 'r') as f:
|
||||
metadata = yaml.safe_load(f)
|
||||
|
||||
assert metadata['status'] == 'in_progress'
|
||||
|
||||
# Step 8: Logout
|
||||
logout_response = client.get('/logout', follow_redirects=False)
|
||||
assert logout_response.status_code == 302
|
||||
|
||||
# Verify user is logged out (accessing dashboard should redirect to login)
|
||||
protected_response = client.get('/dashboard', follow_redirects=False)
|
||||
assert protected_response.status_code == 302
|
||||
assert '/login' in protected_response.location
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
def test_dashboard_access_control_enforcement(client, app, test_product_with_feedback, dashboard_test_users):
|
||||
"""Integration test for access control - owner can only see assigned products"""
|
||||
# Create another product owner with different product access
|
||||
users_file = os.path.join(app.config['DATA_DIR'], 'users.yaml')
|
||||
|
||||
with open(users_file, 'r') as f:
|
||||
users_data = yaml.safe_load(f)
|
||||
|
||||
# Add new user to the users dict
|
||||
users_data['users']['usr_other_owner'] = {
|
||||
'user_id': 'usr_other_owner',
|
||||
'username': 'other_owner',
|
||||
'email': 'other@example.com',
|
||||
'password_hash': User.hash_password('other123'),
|
||||
'role': 'product_owner',
|
||||
'product_ids': ['different-product'],
|
||||
'is_active': True
|
||||
}
|
||||
|
||||
with open(users_file, 'w') as f:
|
||||
yaml.dump(users_data, f)
|
||||
|
||||
# Login as owner without access to dashboard-test-product
|
||||
client.post('/login', data={
|
||||
'username': 'other_owner',
|
||||
'password': 'other123'
|
||||
})
|
||||
|
||||
# Try to access feedback from product they don't own
|
||||
feedback_id = test_product_with_feedback['feedback_items'][0]['id']
|
||||
response = client.get(f'/feedback/{feedback_id}')
|
||||
|
||||
# Should be denied access (403 Forbidden)
|
||||
assert response.status_code == 403
|
||||
Reference in New Issue
Block a user