refactor: tech debt reduction - extract hooks, deduplicate helpers, update deps, add CI
- Extract useCustomStepFlow hook from TreeNavigationPage (1040 → 759 lines) - Create core/filters.py with shared tree/step visibility filters - Create services/export_service.py from session export logic - Add GitHub Actions CI/CD pipeline (pytest + lint + build) - Add GIN index migration for full-text search on trees - Update FastAPI 0.128.5, Pydantic 2.12.5, SQLAlchemy 2.0.46, +5 more - Fix regex → pattern deprecation in Query() params Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
chihlasm
@@ -1,4 +1,3 @@
|
||||
import html
|
||||
from datetime import datetime, timezone
|
||||
from typing import Annotated, Optional
|
||||
from uuid import UUID
|
||||
@@ -14,6 +13,7 @@ from app.models.user import User
|
||||
from app.schemas.session import SessionCreate, SessionUpdate, SessionResponse, SessionExport, ScratchpadUpdate, SaveAsTreeRequest, SaveAsTreeResponse
|
||||
from app.api.deps import get_current_active_user
|
||||
from app.core.permissions import can_access_tree
|
||||
from app.services.export_service import generate_markdown_export, generate_text_export, generate_html_export
|
||||
|
||||
router = APIRouter(prefix="/sessions", tags=["sessions"])
|
||||
|
||||
@@ -283,13 +283,13 @@ async def export_session(
|
||||
|
||||
# Generate export based on format
|
||||
if export_options.format == "markdown":
|
||||
content = _generate_markdown_export(session, export_options)
|
||||
content = generate_markdown_export(session, export_options)
|
||||
media_type = "text/markdown"
|
||||
elif export_options.format == "html":
|
||||
content = _generate_html_export(session, export_options)
|
||||
content = generate_html_export(session, export_options)
|
||||
media_type = "text/html"
|
||||
else: # text
|
||||
content = _generate_text_export(session, export_options)
|
||||
content = generate_text_export(session, export_options)
|
||||
media_type = "text/plain"
|
||||
|
||||
# Mark as exported
|
||||
@@ -299,158 +299,6 @@ async def export_session(
|
||||
return PlainTextResponse(content=content, media_type=media_type)
|
||||
|
||||
|
||||
def _generate_markdown_export(session: Session, options: SessionExport) -> str:
|
||||
"""Generate markdown export."""
|
||||
lines = []
|
||||
|
||||
if options.include_tree_info:
|
||||
tree_name = session.tree_snapshot.get("name", "Troubleshooting Session")
|
||||
lines.append(f"# {tree_name}")
|
||||
lines.append("")
|
||||
if session.ticket_number:
|
||||
lines.append(f"**Ticket:** {session.ticket_number}")
|
||||
if session.client_name:
|
||||
lines.append(f"**Client:** {session.client_name}")
|
||||
if options.include_timestamps:
|
||||
lines.append(f"**Started:** {session.started_at.strftime('%Y-%m-%d %H:%M')}")
|
||||
if session.completed_at:
|
||||
lines.append(f"**Completed:** {session.completed_at.strftime('%Y-%m-%d %H:%M')}")
|
||||
lines.append("")
|
||||
lines.append("---")
|
||||
lines.append("")
|
||||
|
||||
# Scratchpad / Evidence section
|
||||
scratchpad = getattr(session, 'scratchpad', '') or ''
|
||||
if scratchpad.strip():
|
||||
lines.append("## Evidence / Reference")
|
||||
lines.append("")
|
||||
lines.append(scratchpad)
|
||||
lines.append("")
|
||||
lines.append("---")
|
||||
lines.append("")
|
||||
|
||||
lines.append("## Troubleshooting Steps")
|
||||
lines.append("")
|
||||
|
||||
for i, decision in enumerate(session.decisions, 1):
|
||||
question = decision.get("question") or decision.get("action_performed", "Step")
|
||||
answer = decision.get("answer", "")
|
||||
notes = decision.get("notes", "")
|
||||
|
||||
lines.append(f"### Step {i}: {question}")
|
||||
if answer:
|
||||
lines.append(f"**Answer:** {answer}")
|
||||
if notes:
|
||||
lines.append(f"**Notes:** {notes}")
|
||||
if options.include_timestamps and decision.get("timestamp"):
|
||||
lines.append(f"*{decision['timestamp']}*")
|
||||
lines.append("")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _generate_text_export(session: Session, options: SessionExport) -> str:
|
||||
"""Generate plain text export."""
|
||||
lines = []
|
||||
|
||||
if options.include_tree_info:
|
||||
tree_name = session.tree_snapshot.get("name", "Troubleshooting Session")
|
||||
lines.append(tree_name)
|
||||
lines.append("=" * len(tree_name))
|
||||
if session.ticket_number:
|
||||
lines.append(f"Ticket: {session.ticket_number}")
|
||||
if session.client_name:
|
||||
lines.append(f"Client: {session.client_name}")
|
||||
if options.include_timestamps:
|
||||
lines.append(f"Started: {session.started_at.strftime('%Y-%m-%d %H:%M')}")
|
||||
if session.completed_at:
|
||||
lines.append(f"Completed: {session.completed_at.strftime('%Y-%m-%d %H:%M')}")
|
||||
lines.append("")
|
||||
|
||||
# Scratchpad / Evidence section
|
||||
scratchpad = getattr(session, 'scratchpad', '') or ''
|
||||
if scratchpad.strip():
|
||||
lines.append("EVIDENCE / REFERENCE")
|
||||
lines.append("-" * 20)
|
||||
lines.append(scratchpad)
|
||||
lines.append("")
|
||||
|
||||
lines.append("TROUBLESHOOTING STEPS")
|
||||
lines.append("-" * 20)
|
||||
|
||||
for i, decision in enumerate(session.decisions, 1):
|
||||
question = decision.get("question") or decision.get("action_performed", "Step")
|
||||
answer = decision.get("answer", "")
|
||||
notes = decision.get("notes", "")
|
||||
|
||||
lines.append(f"\n{i}. {question}")
|
||||
if answer:
|
||||
lines.append(f" Answer: {answer}")
|
||||
if notes:
|
||||
lines.append(f" Notes: {notes}")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _generate_html_export(session: Session, options: SessionExport) -> str:
|
||||
"""Generate HTML export."""
|
||||
tree_name = html.escape(session.tree_snapshot.get("name", "Troubleshooting Session"))
|
||||
|
||||
html_parts = ['<!DOCTYPE html>', '<html>', '<head>',
|
||||
'<meta charset="UTF-8">',
|
||||
f'<title>{tree_name}</title>',
|
||||
'<style>',
|
||||
'body { font-family: Arial, sans-serif; max-width: 800px; margin: 0 auto; padding: 20px; }',
|
||||
'h1 { color: #333; }',
|
||||
'.meta { color: #666; margin-bottom: 20px; }',
|
||||
'.step { margin-bottom: 15px; padding: 10px; background: #f5f5f5; border-radius: 5px; }',
|
||||
'.step h3 { margin: 0 0 10px 0; color: #444; }',
|
||||
'.answer { font-weight: bold; }',
|
||||
'.notes { font-style: italic; color: #555; }',
|
||||
'.timestamp { font-size: 0.85em; color: #888; }',
|
||||
'</style>',
|
||||
'</head>', '<body>']
|
||||
|
||||
if options.include_tree_info:
|
||||
html_parts.append(f'<h1>{tree_name}</h1>')
|
||||
html_parts.append('<div class="meta">')
|
||||
if session.ticket_number:
|
||||
html_parts.append(f'<p><strong>Ticket:</strong> {html.escape(session.ticket_number)}</p>')
|
||||
if session.client_name:
|
||||
html_parts.append(f'<p><strong>Client:</strong> {html.escape(session.client_name)}</p>')
|
||||
if options.include_timestamps:
|
||||
html_parts.append(f'<p><strong>Started:</strong> {session.started_at.strftime("%Y-%m-%d %H:%M")}</p>')
|
||||
if session.completed_at:
|
||||
html_parts.append(f'<p><strong>Completed:</strong> {session.completed_at.strftime("%Y-%m-%d %H:%M")}</p>')
|
||||
html_parts.append('</div>')
|
||||
|
||||
# Scratchpad / Evidence section
|
||||
scratchpad = getattr(session, 'scratchpad', '') or ''
|
||||
if scratchpad.strip():
|
||||
html_parts.append('<h2>Evidence / Reference</h2>')
|
||||
html_parts.append(f'<div class="scratchpad" style="white-space: pre-wrap; background: #f9f9f9; padding: 12px; border-radius: 4px; margin-bottom: 20px;">{html.escape(scratchpad)}</div>')
|
||||
|
||||
html_parts.append('<h2>Troubleshooting Steps</h2>')
|
||||
|
||||
for i, decision in enumerate(session.decisions, 1):
|
||||
question = html.escape(decision.get("question") or decision.get("action_performed", "Step"))
|
||||
answer = html.escape(decision.get("answer", ""))
|
||||
notes = html.escape(decision.get("notes", ""))
|
||||
|
||||
html_parts.append('<div class="step">')
|
||||
html_parts.append(f'<h3>Step {i}: {question}</h3>')
|
||||
if answer:
|
||||
html_parts.append(f'<p class="answer">Answer: {answer}</p>')
|
||||
if notes:
|
||||
html_parts.append(f'<p class="notes">Notes: {notes}</p>')
|
||||
if options.include_timestamps and decision.get("timestamp"):
|
||||
html_parts.append(f'<p class="timestamp">{html.escape(str(decision["timestamp"]))}</p>')
|
||||
html_parts.append('</div>')
|
||||
|
||||
html_parts.extend(['</body>', '</html>'])
|
||||
return "\n".join(html_parts)
|
||||
|
||||
|
||||
# --- Save Session as Tree ---
|
||||
|
||||
|
||||
|
||||
@@ -3,12 +3,13 @@ from typing import Optional
|
||||
from datetime import datetime, timezone
|
||||
from decimal import Decimal
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
||||
from sqlalchemy import select, or_, and_, func, desc, Integer, case
|
||||
from sqlalchemy import select, func, desc, Integer, case
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.database import get_db
|
||||
from app.api.deps import get_current_active_user, require_engineer_or_admin
|
||||
from app.core.permissions import can_view_step, can_edit_step
|
||||
from app.core.filters import build_step_visibility_filter
|
||||
from app.models.user import User
|
||||
from app.models.step_library import StepLibrary, StepRating
|
||||
from app.models.step_category import StepCategory
|
||||
@@ -53,29 +54,15 @@ async def get_step_or_404(
|
||||
return step
|
||||
|
||||
|
||||
def build_visibility_filter(user: User):
|
||||
"""Build SQLAlchemy filter for step visibility based on user."""
|
||||
if user.account_id:
|
||||
return or_(
|
||||
StepLibrary.visibility == 'public',
|
||||
and_(StepLibrary.visibility == 'team', StepLibrary.account_id == user.account_id),
|
||||
StepLibrary.created_by == user.id # Own private steps
|
||||
)
|
||||
else:
|
||||
return or_(
|
||||
StepLibrary.visibility == 'public',
|
||||
StepLibrary.created_by == user.id
|
||||
)
|
||||
|
||||
|
||||
@router.get("", response_model=list[StepLibraryListResponse])
|
||||
async def list_steps(
|
||||
visibility: Optional[str] = Query(None, regex="^(private|team|public)$"),
|
||||
visibility: Optional[str] = Query(None, pattern="^(private|team|public)$"),
|
||||
category_id: Optional[UUID] = None,
|
||||
tags: Optional[list[str]] = Query(None),
|
||||
min_rating: Optional[float] = Query(None, ge=0, le=5),
|
||||
step_type: Optional[str] = Query(None, regex="^(decision|action|solution)$"),
|
||||
sort_by: str = Query("recent", regex="^(recent|popular|highest_rated|most_used)$"),
|
||||
step_type: Optional[str] = Query(None, pattern="^(decision|action|solution)$"),
|
||||
sort_by: str = Query("recent", pattern="^(recent|popular|highest_rated|most_used)$"),
|
||||
limit: int = Query(20, ge=1, le=100),
|
||||
offset: int = Query(0, ge=0),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
@@ -84,7 +71,7 @@ async def list_steps(
|
||||
"""List steps with filters and pagination."""
|
||||
query = select(StepLibrary).where(
|
||||
StepLibrary.is_active == True,
|
||||
build_visibility_filter(current_user)
|
||||
build_step_visibility_filter(current_user)
|
||||
)
|
||||
|
||||
# Apply filters
|
||||
@@ -165,7 +152,7 @@ async def search_steps(
|
||||
|
||||
query = select(StepLibrary).where(
|
||||
StepLibrary.is_active == True,
|
||||
build_visibility_filter(current_user),
|
||||
build_step_visibility_filter(current_user),
|
||||
func.to_tsvector('english', StepLibrary.title).match(search_query)
|
||||
).order_by(desc(StepLibrary.rating_average)).limit(limit)
|
||||
|
||||
@@ -218,7 +205,7 @@ async def get_popular_tags(
|
||||
func.count().label('count')
|
||||
).where(
|
||||
StepLibrary.is_active == True,
|
||||
build_visibility_filter(current_user)
|
||||
build_step_visibility_filter(current_user)
|
||||
).group_by(
|
||||
func.unnest(StepLibrary.tags)
|
||||
).order_by(
|
||||
|
||||
@@ -4,7 +4,7 @@ from uuid import UUID
|
||||
import secrets
|
||||
from fastapi import APIRouter, Depends, HTTPException, status, Query, Request
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select, func, or_, true as sa_true, update
|
||||
from sqlalchemy import select, func, or_, update
|
||||
from sqlalchemy.orm import selectinload
|
||||
|
||||
from app.core.database import get_db
|
||||
@@ -21,6 +21,7 @@ from app.schemas.tree import (
|
||||
)
|
||||
from app.api.deps import get_current_active_user, require_engineer_or_admin, require_admin
|
||||
from app.core.permissions import can_edit_tree, can_access_tree
|
||||
from app.core.filters import build_tree_access_filter
|
||||
from app.core.subscriptions import check_tree_limit
|
||||
from app.core.audit import log_audit
|
||||
from app.core.config import settings
|
||||
@@ -29,28 +30,6 @@ from app.core.tree_validation import can_publish_tree
|
||||
router = APIRouter(prefix="/trees", tags=["trees"])
|
||||
|
||||
|
||||
def build_tree_access_filter(current_user: User):
|
||||
"""Build the access filter for trees based on user permissions.
|
||||
|
||||
Returns trees that are:
|
||||
- All trees (for super admins)
|
||||
- Default/system trees (visible to all)
|
||||
- Public trees
|
||||
- User's own trees
|
||||
- Trees from user's team
|
||||
"""
|
||||
if current_user.is_super_admin:
|
||||
return sa_true()
|
||||
conditions = [
|
||||
Tree.is_default == True,
|
||||
Tree.is_public == True,
|
||||
Tree.author_id == current_user.id,
|
||||
]
|
||||
if current_user.account_id:
|
||||
conditions.append(Tree.account_id == current_user.account_id)
|
||||
return or_(*conditions)
|
||||
|
||||
|
||||
def build_tree_response(tree: Tree) -> TreeListResponse:
|
||||
"""Build TreeListResponse with category_info and tags."""
|
||||
category_info = None
|
||||
|
||||
Reference in New Issue
Block a user