From 31b3012ee133c826ff2f3b0db148aad343f96b1f Mon Sep 17 00:00:00 2001 From: Vo Date: Thu, 11 Jun 2026 11:29:10 -0700 Subject: [PATCH] Add simulation details editing --- backend/app/features/simulation/api.py | 85 +++- backend/app/features/simulation/schemas.py | 67 ++- backend/tests/features/simulation/test_api.py | 208 +++++++++ .../tests/features/simulation/test_schemas.py | 35 +- .../simulations/SimulationDetailsPage.tsx | 118 ++++- frontend/src/features/simulations/api/api.ts | 10 + .../components/SimulationDetailsView.tsx | 433 +++++++++++++++--- frontend/src/features/simulations/routes.tsx | 7 +- frontend/src/types/simulation.ts | 19 + 9 files changed, 886 insertions(+), 96 deletions(-) diff --git a/backend/app/features/simulation/api.py b/backend/app/features/simulation/api.py index a9ce6c39..c4ef3449 100644 --- a/backend/app/features/simulation/api.py +++ b/backend/app/features/simulation/api.py @@ -16,6 +16,7 @@ SimulationOut, SimulationSummaryCapabilitiesOut, SimulationSummaryOut, + SimulationUpdate, ) from app.features.user.manager import current_active_user from app.features.user.models import User @@ -24,6 +25,15 @@ case_router = APIRouter(prefix="/cases", tags=["Cases"]) +def _simulation_detail_query(db: Session): + return db.query(Simulation).options( + joinedload(Simulation.case), + joinedload(Simulation.machine), + selectinload(Simulation.artifacts), + selectinload(Simulation.links), + ) + + @case_router.get( "", response_model=list[CaseOut], @@ -256,15 +266,7 @@ def create_simulation( # Re-query with relationships loaded sim_loaded = ( - db.query(Simulation) - .options( - joinedload(Simulation.case), - joinedload(Simulation.machine), - selectinload(Simulation.artifacts), - selectinload(Simulation.links), - ) - .filter(Simulation.id == sim.id) - .one_or_none() + _simulation_detail_query(db).filter(Simulation.id == sim.id).one_or_none() ) if sim_loaded is None: @@ -336,6 +338,59 @@ def list_simulations( return [_simulation_to_out(s) for s in sims] +@simulation_router.patch( + "/{sim_id}", + response_model=SimulationOut, + responses={ + 200: {"description": "Simulation updated successfully."}, + 401: {"description": "Unauthorized."}, + 404: {"description": "Simulation not found."}, + 422: {"description": "Validation error."}, + 500: {"description": "Internal server error."}, + }, +) +def update_simulation( + sim_id: UUID, + payload: SimulationUpdate, + db: Session = Depends(get_database_session), + user: User = Depends(current_active_user), +) -> SimulationOut: + """Partially update allowed simulation metadata fields.""" + sim = db.query(Simulation).filter(Simulation.id == sim_id).one_or_none() + + if sim is None: + raise HTTPException(status_code=404, detail="Simulation not found") + + now = datetime.now(timezone.utc) + updates = payload.model_dump(by_alias=False, exclude_unset=True) + + if "git_repository_url" in updates and updates["git_repository_url"] is not None: + updates["git_repository_url"] = str(updates["git_repository_url"]) + + for field, value in updates.items(): + setattr(sim, field, value) + + sim.last_updated_by = user.id + sim.updated_at = now + + with transaction(db): + db.add(sim) + db.flush() + + db.expire_all() + sim_loaded = ( + _simulation_detail_query(db).filter(Simulation.id == sim_id).one_or_none() + ) + + if sim_loaded is None: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to load updated simulation.", + ) + + return _simulation_to_out(sim_loaded) + + @simulation_router.get( "/{sim_id}", response_model=SimulationOut, @@ -367,17 +422,7 @@ def get_simulation(sim_id: UUID, db: Session = Depends(get_database_session)): HTTPException If the simulation with the given ID is not found, raises a 404 HTTP exception. """ - sim = ( - db.query(Simulation) - .options( - joinedload(Simulation.case), - joinedload(Simulation.machine), - selectinload(Simulation.artifacts), - selectinload(Simulation.links), - ) - .filter(Simulation.id == sim_id) - .one_or_none() - ) + sim = _simulation_detail_query(db).filter(Simulation.id == sim_id).one_or_none() if not sim: raise HTTPException(status_code=404, detail="Simulation not found") diff --git a/backend/app/features/simulation/schemas.py b/backend/app/features/simulation/schemas.py index 3d1b8fac..2be3ba92 100644 --- a/backend/app/features/simulation/schemas.py +++ b/backend/app/features/simulation/schemas.py @@ -3,7 +3,7 @@ from typing import Annotated, Any from uuid import UUID -from pydantic import Field, HttpUrl, computed_field +from pydantic import ConfigDict, Field, HttpUrl, computed_field from app.common.schemas.base import CamelInBaseModel, CamelOutBaseModel from app.features.machine.schemas import MachineOut @@ -274,6 +274,71 @@ class SimulationCreate(CamelInBaseModel): ] +class SimulationUpdate(CamelInBaseModel): + """Schema for narrow v1 simulation metadata updates.""" + + model_config = ConfigDict(extra="forbid") + + description: Annotated[ + str | None, Field(None, description="Optional description of the simulation") + ] + campaign: Annotated[ + str | None, + Field( + None, description="Campaign or run grouping (e.g. historical, amip, tuning)" + ), + ] + experiment_type: Annotated[ + ExperimentType | str | None, + Field( + None, + description=( + "High-level experiment category (e.g. historical, amip, piControl). " + "Often aligned with CMIP experiment identifiers." + ), + ), + ] + compiler: Annotated[ + str | None, Field(None, description="Optional compiler used for the simulation") + ] + hpc_username: Annotated[ + str | None, + Field( + None, + description="HPC username for provenance (trusted, informational only)", + ), + ] + key_features: Annotated[ + str | None, Field(None, description="Optional key features of the simulation") + ] + known_issues: Annotated[ + str | None, Field(None, description="Optional known issues with the simulation") + ] + notes_markdown: Annotated[ + str | None, + Field(None, description="Optional additional notes in markdown format"), + ] + git_repository_url: Annotated[ + HttpUrl | None, Field(None, description="Optional Git repository URL") + ] + git_branch: Annotated[ + str | None, + Field( + None, description="Optional Git branch name associated with the simulation" + ), + ] + git_tag: Annotated[ + str | None, Field(None, description="Optional Git tag for the simulation") + ] + git_commit_hash: Annotated[ + str | None, + Field( + None, + description="Optional Git commit hash associated with the simulation", + ), + ] + + class SimulationSummaryOut(CamelOutBaseModel): """Lightweight schema for simulation summaries nested inside CaseOut. diff --git a/backend/tests/features/simulation/test_api.py b/backend/tests/features/simulation/test_api.py index 9f730eb9..d37d104a 100644 --- a/backend/tests/features/simulation/test_api.py +++ b/backend/tests/features/simulation/test_api.py @@ -1,4 +1,5 @@ from contextlib import nullcontext +from datetime import datetime, timedelta, timezone from unittest.mock import MagicMock, patch from uuid import uuid4 @@ -48,6 +49,75 @@ def _create_case(db: Session, name: str = "test_case") -> Case: return case +def _create_ingestion( + db: Session, + machine_id, + user_id, + source_reference: str = "test_simulation_ingestion", +) -> Ingestion: + ingestion = Ingestion( + source_type=IngestionSourceType.BROWSER_UPLOAD, + source_reference=source_reference, + machine_id=machine_id, + triggered_by=user_id, + status=IngestionStatus.SUCCESS, + created_count=1, + duplicate_count=0, + error_count=0, + ) + db.add(ingestion) + db.flush() + + return ingestion + + +def _create_simulation_record( + db: Session, + *, + case: Case, + machine_id, + ingestion_id, + created_by, + last_updated_by, + execution_id: str = "patch-test-exec-1", + description: str | None = "Original description", + updated_at: datetime | None = None, +) -> Simulation: + sim = Simulation( + case_id=case.id, + execution_id=execution_id, + description=description, + compset="AQUAPLANET", + compset_alias="QPC4", + grid_name="f19_f19", + grid_resolution="1.9x2.5", + initialization_type="startup", + simulation_type="experimental", + status="created", + campaign="campaign-original", + experiment_type="historical", + machine_id=machine_id, + simulation_start_date="2023-01-01T00:00:00Z", + compiler="gcc", + key_features="Original features", + known_issues="Original issues", + notes_markdown="Original notes", + git_repository_url="https://example.com/original", + git_branch="main", + git_tag="v1.0", + git_commit_hash="abc123", + hpc_username="old-user", + created_by=created_by, + last_updated_by=last_updated_by, + ingestion_id=ingestion_id, + updated_at=updated_at or datetime.now(timezone.utc) - timedelta(days=1), + ) + db.add(sim) + db.flush() + + return sim + + class TestListCases: def test_endpoint_returns_empty_list(self, client): res = client.get(f"{API_BASE}/cases") @@ -794,6 +864,144 @@ def test_endpoint_raises_404_if_simulation_not_found(self, client): assert res.json() == {"detail": "Simulation not found"} +class TestUpdateSimulation: + def test_endpoint_updates_sparse_metadata_and_audit_fields( + self, client, db: Session, normal_user_sync, admin_user_sync + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch", + ) + original_updated_at = datetime.now(timezone.utc) - timedelta(days=2) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=admin_user_sync["id"], + updated_at=original_updated_at, + ) + db.commit() + + payload = { + "description": "Updated description", + "campaign": "campaign-updated", + "gitRepositoryUrl": "https://example.com/updated", + "notesMarkdown": "Updated notes", + } + + res = client.patch(f"{API_BASE}/simulations/{sim.id}", json=payload) + + assert res.status_code == 200 + data = res.json() + assert data["description"] == payload["description"] + assert data["campaign"] == payload["campaign"] + assert data["gitRepositoryUrl"] == payload["gitRepositoryUrl"] + assert data["notesMarkdown"] == payload["notesMarkdown"] + assert data["compiler"] == "gcc" + assert data["gitTag"] == "v1.0" + assert data["hpcUsername"] == "old-user" + assert data["lastUpdatedBy"] == str(normal_user_sync["id"]) + assert data["lastUpdatedByUser"]["email"] == normal_user_sync["email"] + assert data["updatedAt"] != original_updated_at.isoformat() + + db.expire_all() + updated_sim = db.query(Simulation).filter(Simulation.id == sim.id).one() + assert updated_sim.description == payload["description"] + assert updated_sim.campaign == payload["campaign"] + assert updated_sim.git_repository_url == payload["gitRepositoryUrl"] + assert updated_sim.notes_markdown == payload["notesMarkdown"] + assert updated_sim.compiler == "gcc" + assert updated_sim.git_tag == "v1.0" + assert updated_sim.last_updated_by == normal_user_sync["id"] + assert updated_sim.updated_at > original_updated_at + + def test_endpoint_returns_401_without_authentication( + self, client, db: Session, normal_user_sync + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch_unauth") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch_unauth", + ) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=normal_user_sync["id"], + execution_id="patch-test-unauth", + ) + db.commit() + + app.dependency_overrides.pop(current_active_user, None) + + res = client.patch( + f"{API_BASE}/simulations/{sim.id}", + json={"description": "Should fail"}, + ) + + assert res.status_code == 401 + + def test_endpoint_returns_404_when_simulation_not_found(self, client): + res = client.patch( + f"{API_BASE}/simulations/{uuid4()}", + json={"description": "Missing simulation"}, + ) + + assert res.status_code == 404 + assert res.json() == {"detail": "Simulation not found"} + + def test_endpoint_rejects_out_of_scope_fields( + self, client, db: Session, normal_user_sync + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch_scope") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch_scope", + ) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=normal_user_sync["id"], + execution_id="patch-test-scope", + ) + db.commit() + + res = client.patch( + f"{API_BASE}/simulations/{sim.id}", + json={"caseName": "mutated-case-name"}, + ) + + assert res.status_code == 422 + + db.expire_all() + unchanged_sim = db.query(Simulation).filter(Simulation.id == sim.id).one() + assert unchanged_sim.description == "Original description" + assert unchanged_sim.case_id == case.id + + class TestSimulationBrowserIncludesCaseMetadata: def test_simulation_list_includes_case_name_and_id( self, client, db: Session, normal_user_sync, admin_user_sync diff --git a/backend/tests/features/simulation/test_schemas.py b/backend/tests/features/simulation/test_schemas.py index d22ed7ab..b4eace7b 100644 --- a/backend/tests/features/simulation/test_schemas.py +++ b/backend/tests/features/simulation/test_schemas.py @@ -1,7 +1,8 @@ from datetime import datetime from uuid import uuid4 -from pydantic import HttpUrl +import pytest +from pydantic import HttpUrl, ValidationError from app.common.schemas.utils import to_snake_case from app.features.machine.schemas import MachineOut @@ -15,6 +16,7 @@ SimulationOut, SimulationSummaryCapabilitiesOut, SimulationSummaryOut, + SimulationUpdate, ) from app.features.user.schemas import UserPreview @@ -103,6 +105,37 @@ def test_valid_simulation_create_optional_fields(self): assert getattr(simulation_create, snake_case_key) == value +class TestSimulationUpdateSchema: + def test_accepts_allowed_optional_fields(self): + payload = { + "description": "Updated description", + "campaign": "campaign-1", + "experimentType": "historical", + "compiler": "intel", + "hpcUsername": "sim-user", + "keyFeatures": "feature a\nfeature b", + "knownIssues": "issue a", + "notesMarkdown": "## Notes", + "gitRepositoryUrl": HttpUrl("https://example.com/repo"), + "gitBranch": "main", + "gitTag": "v2.0", + "gitCommitHash": "abc123def456", + } + + update = SimulationUpdate(**payload) + + for key, value in payload.items(): + assert getattr(update, to_snake_case(key)) == value + + def test_rejects_invalid_url(self): + with pytest.raises(ValidationError): + SimulationUpdate(gitRepositoryUrl="not-a-url") + + def test_rejects_out_of_scope_field(self): + with pytest.raises(ValidationError): + SimulationUpdate(caseName="new-case") + + class TestSimulationOutSchema: def test_valid_simulation_out_required_fields(self): # Arrange: Define the required fields diff --git a/frontend/src/features/simulations/SimulationDetailsPage.tsx b/frontend/src/features/simulations/SimulationDetailsPage.tsx index 59306ee7..1f2d7806 100644 --- a/frontend/src/features/simulations/SimulationDetailsPage.tsx +++ b/frontend/src/features/simulations/SimulationDetailsPage.tsx @@ -1,17 +1,63 @@ +import axios from 'axios'; import { useEffect, useState } from 'react'; import { useLocation, useParams } from 'react-router-dom'; import { useAuth } from '@/auth/hooks/useAuth'; -import { resolvePaceExecution } from '@/features/simulations/api/api'; +import { resolvePaceExecution, updateSimulation } from '@/features/simulations/api/api'; import { SimulationDetailsView } from '@/features/simulations/components/SimulationDetailsView'; import { useSimulation } from '@/features/simulations/hooks/useSimulation'; import { useSimulationSummary } from '@/features/simulations/hooks/useSimulationSummary'; +import { toast } from '@/hooks/use-toast'; +import type { SimulationOut, SimulationUpdate } from '@/types'; -export const SimulationDetailsPage = () => { +const MAX_COMPARE_SELECTION = 5; + +interface SimulationDetailsPageProps { + selectedSimulationIds: string[]; + setSelectedSimulationIds: (ids: string[]) => void; +} + +const getUpdateErrorMessage = (error: unknown): string => { + if (axios.isAxiosError(error)) { + const detail = error.response?.data?.detail; + + if (typeof detail === 'string') { + return detail; + } + + if (Array.isArray(detail)) { + const messages = detail + .map((item) => { + const field = Array.isArray(item?.loc) ? item.loc[item.loc.length - 1] : null; + const message = typeof item?.msg === 'string' ? item.msg : null; + + if (!message) return null; + if (typeof field !== 'string') return message; + + return `${field}: ${message}`; + }) + .filter((message): message is string => Boolean(message)); + + if (messages.length > 0) { + return messages.join(' '); + } + } + } + + return error instanceof Error ? error.message : 'Failed to update simulation.'; +}; + +export const SimulationDetailsPage = ({ + selectedSimulationIds, + setSelectedSimulationIds, +}: SimulationDetailsPageProps) => { const { id } = useParams<{ id: string }>(); const location = useLocation(); - const { data: simulation, loading, error } = useSimulation(id ?? ''); + const { data: fetchedSimulation, loading, error } = useSimulation(id ?? ''); const { isAuthenticated, loading: authLoading, loginWithGithub } = useAuth(); + const [simulation, setSimulation] = useState(null); + const [isSaving, setIsSaving] = useState(false); + const [saveError, setSaveError] = useState(null); const [paceExperimentId, setPaceExperimentId] = useState(null); const [isResolvingPace, setIsResolvingPace] = useState(false); const [paceResolutionAttempted, setPaceResolutionAttempted] = useState(false); @@ -26,6 +72,7 @@ export const SimulationDetailsPage = () => { : normalizedBackHref.startsWith('/simulations') ? 'Back to Simulations' : 'Back to Runs'; + const canEdit = !authLoading && isAuthenticated; const currentSimulation = simulation?.id === id ? simulation : null; const executionId = currentSimulation?.executionId?.trim() ?? ''; const llmSummaryAvailable = currentSimulation?.summaryCapabilities?.llmAvailable ?? false; @@ -36,6 +83,18 @@ export const SimulationDetailsPage = () => { autoGenerate: shouldAutoGenerateSummary, }); + useEffect(() => { + if (fetchedSimulation?.id === id) { + setSimulation(fetchedSimulation); + setSaveError(null); + return; + } + + if (!loading && !error) { + setSimulation(null); + } + }, [error, fetchedSimulation, id, loading]); + useEffect(() => { if (!executionId) { setPaceExperimentId(null); @@ -72,6 +131,50 @@ export const SimulationDetailsPage = () => { }; }, [executionId]); + const handleSave = async (payload: SimulationUpdate): Promise => { + if (!id) return false; + + setIsSaving(true); + setSaveError(null); + + try { + const updatedSimulation = await updateSimulation(id, payload); + setSimulation(updatedSimulation); + return true; + } catch (saveErr) { + setSaveError(getUpdateErrorMessage(saveErr)); + return false; + } finally { + setIsSaving(false); + } + }; + + const handleToggleCompare = () => { + if (!currentSimulation) return; + + if (selectedSimulationIds.includes(currentSimulation.id)) { + setSelectedSimulationIds( + selectedSimulationIds.filter((simulationId) => simulationId !== currentSimulation.id), + ); + return; + } + + if (selectedSimulationIds.length >= MAX_COMPARE_SELECTION) { + toast({ + title: 'Compare list full', + description: `You can compare up to ${MAX_COMPARE_SELECTION} runs at a time.`, + variant: 'destructive', + }); + return; + } + + setSelectedSimulationIds([...selectedSimulationIds, currentSimulation.id]); + toast({ + title: 'Added to compare', + description: `${currentSimulation.executionId} is now in your compare list.`, + }); + }; + if (!id) { return (
@@ -119,6 +222,9 @@ export const SimulationDetailsPage = () => { return ( { summaryLastDurationMs={summary.lastDurationMs} onGenerateSummary={summary.generate} canGenerateSummary + onToggleCompare={handleToggleCompare} + canEdit={canEdit} + isSaving={isSaving} + saveError={saveError} + onSave={handleSave} + onClearSaveError={() => setSaveError(null)} llmSummaryAvailable={llmSummaryAvailable} autoGenerateDeterministicOnLoad={shouldAutoGenerateSummary} showLoginForAiSummary={showLoginForAiSummary} diff --git a/frontend/src/features/simulations/api/api.ts b/frontend/src/features/simulations/api/api.ts index f00da3de..4c0983fd 100644 --- a/frontend/src/features/simulations/api/api.ts +++ b/frontend/src/features/simulations/api/api.ts @@ -4,6 +4,7 @@ import type { SimulationCreate, SimulationOut, SimulationSummaryResponseOut, + SimulationUpdate, } from '@/types'; export const SIMULATIONS_URL = '/simulations'; @@ -38,6 +39,15 @@ export const getSimulationById = async (id: string): Promise => { return res.data; }; +export const updateSimulation = async ( + id: string, + data: SimulationUpdate, +): Promise => { + const res = await api.patch(`${SIMULATIONS_URL}/${id}`, data); + + return res.data; +}; + export const generateSimulationSummary = async ( id: string, ): Promise => { diff --git a/frontend/src/features/simulations/components/SimulationDetailsView.tsx b/frontend/src/features/simulations/components/SimulationDetailsView.tsx index 754a254b..e8e6a613 100644 --- a/frontend/src/features/simulations/components/SimulationDetailsView.tsx +++ b/frontend/src/features/simulations/components/SimulationDetailsView.tsx @@ -1,5 +1,5 @@ import { ArrowLeft, ChevronDown, CircleHelp } from 'lucide-react'; -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { Link } from 'react-router-dom'; import { SimulationStatusBadge } from '@/components/shared/SimulationStatusBadge'; @@ -21,20 +21,28 @@ import { } from '@/features/simulations/components/SimulationSummaryPanel'; import { SimulationTypeBadge } from '@/features/simulations/components/SimulationTypeBadge'; import { cn } from '@/lib/utils'; -import type { SimulationOut, SimulationSummaryResponseOut } from '@/types'; +import type { SimulationOut, SimulationSummaryResponseOut, SimulationUpdate } from '@/types'; import { getArtifactsByKind } from '@/types/artifact'; import { formatDate, getSimulationDuration } from '@/utils/utils'; // -------------------- Types -------------------- interface SimulationDetailsViewProps { simulation: SimulationOut; + isCompareSelected?: boolean; + compareSelectionCount?: number; + maxCompareSelection?: number; canEdit?: boolean; + isSaving?: boolean; + saveError?: string | null; backHref?: string; backLabel?: string; paceLink?: { href: string; label: string; } | null; + onToggleCompare?: () => void; + onSave?: (payload: SimulationUpdate) => Promise | boolean; + onClearSaveError?: () => void; isResolvingPace?: boolean; showPaceFallbackInfo?: boolean; summary?: SimulationSummaryResponseOut | null; @@ -56,12 +64,26 @@ interface SimulationDetailsViewProps { const FieldRow = ({ label, children }: { label: string; children: React.ReactNode }) => (
-
{children}
+
{children}
); const ReadonlyInput = ({ value, className }: { value?: string | null; className?: string }) => ( - +
+ {value || '—'} +
+); + +const ReadonlyText = ({ value, className }: { value?: string | null; className?: string }) => ( +

+ {value || '—'} +

); const UserDisplay = ({ @@ -87,13 +109,93 @@ const ReadonlyTextBlock = ({ value, className }: { value?: string | null; classN
); +const EDITABLE_FIELDS = [ + 'description', + 'campaign', + 'experimentType', + 'compiler', + 'hpcUsername', + 'keyFeatures', + 'knownIssues', + 'notesMarkdown', + 'gitRepositoryUrl', + 'gitBranch', + 'gitTag', + 'gitCommitHash', +] as const; + +type EditableField = (typeof EDITABLE_FIELDS)[number]; +type EditableFormState = Record; + +const SINGLE_LINE_FIELDS: ReadonlySet = new Set([ + 'campaign', + 'experimentType', + 'compiler', + 'hpcUsername', + 'gitRepositoryUrl', + 'gitBranch', + 'gitTag', + 'gitCommitHash', +]); + +const toEditableFormState = (simulation: SimulationOut): EditableFormState => ({ + description: simulation.description ?? '', + campaign: simulation.campaign ?? '', + experimentType: simulation.experimentType ?? '', + compiler: simulation.compiler ?? '', + hpcUsername: simulation.hpcUsername ?? '', + keyFeatures: simulation.keyFeatures ?? '', + knownIssues: simulation.knownIssues ?? '', + notesMarkdown: simulation.notesMarkdown ?? '', + gitRepositoryUrl: simulation.gitRepositoryUrl ?? '', + gitBranch: simulation.gitBranch ?? '', + gitTag: simulation.gitTag ?? '', + gitCommitHash: simulation.gitCommitHash ?? '', +}); + +const normalizeEditableValue = (field: EditableField, value: string): string | null => { + const trimmed = value.trim(); + + if (!trimmed) { + return null; + } + + return SINGLE_LINE_FIELDS.has(field) ? trimmed : value; +}; + +const buildUpdatePayload = ( + simulation: SimulationOut, + formState: EditableFormState, +): SimulationUpdate => { + const payload: SimulationUpdate = {}; + + for (const field of EDITABLE_FIELDS) { + const nextValue = normalizeEditableValue(field, formState[field]); + const currentValue = simulation[field] ?? null; + + if (nextValue !== currentValue) { + payload[field] = nextValue; + } + } + + return payload; +}; + // -------------------- View Component -------------------- export const SimulationDetailsView = ({ simulation, + isCompareSelected = false, + compareSelectionCount: compareSelectionCountProp = 0, + maxCompareSelection: maxCompareSelectionProp = 5, canEdit = false, + isSaving = false, + saveError = null, backHref = '/browse', backLabel = 'Back to Runs', paceLink = null, + onToggleCompare, + onSave, + onClearSaveError, isResolvingPace = false, showPaceFallbackInfo = false, summary = null, @@ -111,8 +213,11 @@ export const SimulationDetailsView = ({ onLoginForSummary, }: SimulationDetailsViewProps) => { const [activeTab, setActiveTab] = useState('summary'); + const [isEditing, setIsEditing] = useState(false); const [isAdvancedMetadataOpen, setIsAdvancedMetadataOpen] = useState(false); - const [notes, setNotes] = useState(simulation.notesMarkdown || ''); + const [formState, setFormState] = useState(() => + toEditableFormState(simulation), + ); const performanceLinks = simulation.groupedLinks.performance ?? []; const outputArtifacts = getArtifactsByKind( simulation.artifacts, @@ -158,7 +263,63 @@ export const SimulationDetailsView = ({ date: '2024-02-15T13:45:00Z', text: 'The sea-ice diagnostics will be added later.', }, - ]); + ]); + + useEffect(() => { + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + }, [simulation]); + + useEffect(() => { + if (!canEdit) { + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + } + }, [canEdit, simulation]); + + const updateField = (field: EditableField, value: string) => { + onClearSaveError?.(); + setFormState((current) => ({ + ...current, + [field]: value, + })); + }; + + const handleCancelEdit = () => { + onClearSaveError?.(); + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + }; + + const handleSave = async () => { + if (!onSave) return; + + const payload = buildUpdatePayload(simulation, formState); + + if (Object.keys(payload).length === 0) { + onClearSaveError?.(); + setIsEditing(false); + return; + } + + const saved = await onSave(payload); + + if (saved) { + setIsEditing(false); + } + }; + + const hasUnsavedChanges = Object.keys(buildUpdatePayload(simulation, formState)).length > 0; + const compareSelectionCount = compareSelectionCountProp ?? 0; + const maxCompareSelection = maxCompareSelectionProp ?? 5; + const isCompareActionDisabled = + !isCompareSelected && compareSelectionCount >= maxCompareSelection; + const compareActionLabel = isCompareSelected + ? 'Remove from Comparison' + : 'Add to Comparison'; + const compareActionTooltip = isCompareActionDisabled + ? `Compare list is full. Remove one of the ${maxCompareSelection} selected runs first.` + : undefined; const addComment = () => { if (!newComment.trim()) return; @@ -203,12 +364,52 @@ export const SimulationDetailsView = ({
- - + {compareActionTooltip ? ( + + + + + + + + {compareActionTooltip} + + + ) : ( + + )} + {!isEditing && + (canEdit ? ( + + ) : ( + + + + + + + + Login required + + + ))} + {canEdit && isEditing && ( + <> + + + + )}
+ {isEditing && saveError &&
{saveError}
} {/* Tabs */} @@ -275,14 +476,30 @@ export const SimulationDetailsView = ({ - + {isEditing ? ( + updateField('compiler', event.target.value)} + className="h-8 text-sm" + /> + ) : ( + + )} - {simulation.description && ( + {(isEditing || simulation.description) && (
- + {isEditing ? ( +