fix: workflow editor Phase 5/6 sign-off — save guard, conflict detection, dispatch pre-check
- Save button disabled for non-admins (canSave prop threaded through WorkflowEditor → WorkflowCanvas → WorkflowCanvasToolbar); tooltip explains why when hovered - Optimistic concurrency: migration 072 adds updated_at to workflow_definitions; PUT /workflows/:id returns 409 when client sends a stale updated_at; frontend shows a specific reload-prompt toast - _legacy_dispatch now routes through dispatch_order_line_render instead of calling render_order_line_task directly, so cancelled/rejected order lines are skipped before queueing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,28 @@
|
|||||||
|
"""add updated_at to workflow_definitions
|
||||||
|
|
||||||
|
Revision ID: 072
|
||||||
|
Revises: 071
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision = "072"
|
||||||
|
down_revision = "071"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column(
|
||||||
|
"workflow_definitions",
|
||||||
|
sa.Column(
|
||||||
|
"updated_at",
|
||||||
|
sa.DateTime(),
|
||||||
|
nullable=False,
|
||||||
|
server_default=sa.text("now()"),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_column("workflow_definitions", "updated_at")
|
||||||
@@ -705,7 +705,7 @@ def dispatch_render_with_workflow(order_line_id: str) -> dict:
|
|||||||
|
|
||||||
|
|
||||||
def _legacy_dispatch(order_line_id: str) -> dict:
|
def _legacy_dispatch(order_line_id: str) -> dict:
|
||||||
"""Queue render_order_line_task (the working Celery render implementation)."""
|
"""Queue via dispatch_order_line_render so cancelled/rejected pre-checks apply."""
|
||||||
from app.tasks.step_tasks import render_order_line_task
|
from app.tasks.step_tasks import dispatch_order_line_render
|
||||||
render_order_line_task.delay(order_line_id)
|
dispatch_order_line_render.delay(order_line_id)
|
||||||
return {"backend": "celery", "queued": True}
|
return {"backend": "celery", "queued": True}
|
||||||
|
|||||||
@@ -166,6 +166,7 @@ class WorkflowDefinition(Base):
|
|||||||
config: Mapped[dict] = mapped_column(JSONB, nullable=False, default=dict)
|
config: Mapped[dict] = mapped_column(JSONB, nullable=False, default=dict)
|
||||||
is_active: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True)
|
is_active: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True)
|
||||||
created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow, nullable=False)
|
created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow, nullable=False)
|
||||||
|
updated_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False)
|
||||||
|
|
||||||
runs: Mapped[list["WorkflowRun"]] = relationship(
|
runs: Mapped[list["WorkflowRun"]] = relationship(
|
||||||
"WorkflowRun", back_populates="workflow_def", lazy="noload", cascade="all, delete-orphan"
|
"WorkflowRun", back_populates="workflow_def", lazy="noload", cascade="all, delete-orphan"
|
||||||
|
|||||||
@@ -193,6 +193,7 @@ class WorkflowDefinitionUpdate(BaseModel):
|
|||||||
name: str | None = None
|
name: str | None = None
|
||||||
config: dict | None = None
|
config: dict | None = None
|
||||||
is_active: bool | None = None
|
is_active: bool | None = None
|
||||||
|
updated_at: datetime | None = None
|
||||||
|
|
||||||
|
|
||||||
class WorkflowDefinitionOut(BaseModel):
|
class WorkflowDefinitionOut(BaseModel):
|
||||||
@@ -207,6 +208,7 @@ class WorkflowDefinitionOut(BaseModel):
|
|||||||
)
|
)
|
||||||
is_active: bool
|
is_active: bool
|
||||||
created_at: datetime
|
created_at: datetime
|
||||||
|
updated_at: datetime
|
||||||
model_config = {"from_attributes": True}
|
model_config = {"from_attributes": True}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -284,6 +284,7 @@ async def _workflow_to_out(db: AsyncSession, wf: WorkflowDefinition) -> Workflow
|
|||||||
),
|
),
|
||||||
is_active=wf.is_active,
|
is_active=wf.is_active,
|
||||||
created_at=wf.created_at,
|
created_at=wf.created_at,
|
||||||
|
updated_at=wf.updated_at,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -938,6 +939,15 @@ async def update_workflow(
|
|||||||
if not wf:
|
if not wf:
|
||||||
raise HTTPException(status_code=404, detail="Workflow definition not found")
|
raise HTTPException(status_code=404, detail="Workflow definition not found")
|
||||||
|
|
||||||
|
if body.updated_at is not None:
|
||||||
|
stored_ts = wf.updated_at.replace(tzinfo=None) if wf.updated_at.tzinfo else wf.updated_at
|
||||||
|
client_ts = body.updated_at.replace(tzinfo=None) if body.updated_at.tzinfo else body.updated_at
|
||||||
|
if abs((stored_ts - client_ts).total_seconds()) > 1:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=409,
|
||||||
|
detail="Workflow was modified by someone else. Reload and try again.",
|
||||||
|
)
|
||||||
|
|
||||||
if body.name is not None:
|
if body.name is not None:
|
||||||
wf.name = body.name
|
wf.name = body.name
|
||||||
if body.config is not None:
|
if body.config is not None:
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ export interface WorkflowDefinition {
|
|||||||
rollout_summary: WorkflowRolloutSummary
|
rollout_summary: WorkflowRolloutSummary
|
||||||
is_active: boolean
|
is_active: boolean
|
||||||
created_at: string
|
created_at: string
|
||||||
|
updated_at: string
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface WorkflowConfig {
|
export interface WorkflowConfig {
|
||||||
@@ -239,7 +240,11 @@ export const getWorkflow = (id: string): Promise<WorkflowDefinition> =>
|
|||||||
export const createWorkflow = (data: WorkflowCreate): Promise<WorkflowDefinition> =>
|
export const createWorkflow = (data: WorkflowCreate): Promise<WorkflowDefinition> =>
|
||||||
api.post('/workflows', data).then(r => normalizeWorkflowDefinition(r.data))
|
api.post('/workflows', data).then(r => normalizeWorkflowDefinition(r.data))
|
||||||
|
|
||||||
export const updateWorkflow = (id: string, data: Partial<WorkflowCreate>): Promise<WorkflowDefinition> =>
|
export interface WorkflowUpdatePayload extends Partial<WorkflowCreate> {
|
||||||
|
updated_at?: string
|
||||||
|
}
|
||||||
|
|
||||||
|
export const updateWorkflow = (id: string, data: WorkflowUpdatePayload): Promise<WorkflowDefinition> =>
|
||||||
api.put(`/workflows/${id}`, data).then(r => normalizeWorkflowDefinition(r.data))
|
api.put(`/workflows/${id}`, data).then(r => normalizeWorkflowDefinition(r.data))
|
||||||
|
|
||||||
export const deleteWorkflow = (id: string): Promise<void> =>
|
export const deleteWorkflow = (id: string): Promise<void> =>
|
||||||
|
|||||||
@@ -56,9 +56,10 @@ type WorkflowCanvasProps = {
|
|||||||
workflow: WorkflowDefinition
|
workflow: WorkflowDefinition
|
||||||
onSave: (config: WorkflowConfig) => void
|
onSave: (config: WorkflowConfig) => void
|
||||||
isSaving: boolean
|
isSaving: boolean
|
||||||
|
canSave?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
export function WorkflowCanvas({ workflow, onSave, isSaving }: WorkflowCanvasProps) {
|
export function WorkflowCanvas({ workflow, onSave, isSaving, canSave = true }: WorkflowCanvasProps) {
|
||||||
const queryClient = useQueryClient()
|
const queryClient = useQueryClient()
|
||||||
const {
|
const {
|
||||||
reactFlowWrapper,
|
reactFlowWrapper,
|
||||||
@@ -308,6 +309,7 @@ export function WorkflowCanvas({ workflow, onSave, isSaving }: WorkflowCanvasPro
|
|||||||
isDispatchPending={dispatchMutation.isPending}
|
isDispatchPending={dispatchMutation.isPending}
|
||||||
isContextOptionsLoading={isOrderLineContextsLoading}
|
isContextOptionsLoading={isOrderLineContextsLoading}
|
||||||
isSaving={isSaving}
|
isSaving={isSaving}
|
||||||
|
canSave={canSave}
|
||||||
rollbackPendingOutputTypeId={rollbackOutputTypeMutation.variables?.outputTypeId ?? null}
|
rollbackPendingOutputTypeId={rollbackOutputTypeMutation.variables?.outputTypeId ?? null}
|
||||||
preflightState={preflightState}
|
preflightState={preflightState}
|
||||||
authoringActions={authoringActions}
|
authoringActions={authoringActions}
|
||||||
|
|||||||
@@ -125,6 +125,7 @@ interface WorkflowCanvasToolbarProps {
|
|||||||
isDispatchPending: boolean
|
isDispatchPending: boolean
|
||||||
isContextOptionsLoading: boolean
|
isContextOptionsLoading: boolean
|
||||||
isSaving: boolean
|
isSaving: boolean
|
||||||
|
canSave?: boolean
|
||||||
rollbackPendingOutputTypeId?: string | null
|
rollbackPendingOutputTypeId?: string | null
|
||||||
preflightState: 'ready' | 'required' | 'stale' | 'blocked'
|
preflightState: 'ready' | 'required' | 'stale' | 'blocked'
|
||||||
authoringActions: WorkflowAuthoringActions
|
authoringActions: WorkflowAuthoringActions
|
||||||
@@ -175,6 +176,7 @@ export function WorkflowCanvasToolbar({
|
|||||||
isDispatchPending,
|
isDispatchPending,
|
||||||
isContextOptionsLoading,
|
isContextOptionsLoading,
|
||||||
isSaving,
|
isSaving,
|
||||||
|
canSave = true,
|
||||||
rollbackPendingOutputTypeId,
|
rollbackPendingOutputTypeId,
|
||||||
preflightState,
|
preflightState,
|
||||||
authoringActions,
|
authoringActions,
|
||||||
@@ -345,7 +347,8 @@ export function WorkflowCanvasToolbar({
|
|||||||
</ToolbarActionButton>
|
</ToolbarActionButton>
|
||||||
<ToolbarActionButton
|
<ToolbarActionButton
|
||||||
onClick={onSave}
|
onClick={onSave}
|
||||||
disabled={isSaving || hasValidationErrors}
|
disabled={isSaving || hasValidationErrors || !canSave}
|
||||||
|
title={!canSave ? 'Admin access required to save workflows' : undefined}
|
||||||
tone="primary"
|
tone="primary"
|
||||||
>
|
>
|
||||||
<Save size={14} />
|
<Save size={14} />
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { useState, useEffect, useMemo } from 'react'
|
import { useState, useEffect, useMemo } from 'react'
|
||||||
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
|
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
|
||||||
|
import { useAuthStore, isAdmin } from '../store/auth'
|
||||||
import { WorkflowCanvas } from '../components/workflows/WorkflowCanvas'
|
import { WorkflowCanvas } from '../components/workflows/WorkflowCanvas'
|
||||||
import { NewWorkflowModal } from '../components/workflows/NewWorkflowModal'
|
import { NewWorkflowModal } from '../components/workflows/NewWorkflowModal'
|
||||||
import { WorkflowEditorEmptyState } from '../components/workflows/WorkflowEditorEmptyState'
|
import { WorkflowEditorEmptyState } from '../components/workflows/WorkflowEditorEmptyState'
|
||||||
@@ -39,6 +40,8 @@ export default function WorkflowEditor() {
|
|||||||
const queryClient = useQueryClient()
|
const queryClient = useQueryClient()
|
||||||
const [selectedId, setSelectedId] = useState<string | null>(null)
|
const [selectedId, setSelectedId] = useState<string | null>(null)
|
||||||
const [showNewModal, setShowNewModal] = useState(false)
|
const [showNewModal, setShowNewModal] = useState(false)
|
||||||
|
const user = useAuthStore(s => s.user)
|
||||||
|
const canEdit = isAdmin(user)
|
||||||
|
|
||||||
const { data: workflows = [], isLoading } = useQuery({
|
const { data: workflows = [], isLoading } = useQuery({
|
||||||
queryKey: ['workflows'],
|
queryKey: ['workflows'],
|
||||||
@@ -67,13 +70,19 @@ export default function WorkflowEditor() {
|
|||||||
})
|
})
|
||||||
|
|
||||||
const updateMutation = useMutation({
|
const updateMutation = useMutation({
|
||||||
mutationFn: ({ id, config }: { id: string; config: WorkflowConfig }) =>
|
mutationFn: ({ id, config, updated_at }: { id: string; config: WorkflowConfig; updated_at?: string }) =>
|
||||||
updateWorkflow(id, { config }),
|
updateWorkflow(id, { config, updated_at }),
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
queryClient.invalidateQueries({ queryKey: ['workflows'] })
|
queryClient.invalidateQueries({ queryKey: ['workflows'] })
|
||||||
toast.success('Workflow saved')
|
toast.success('Workflow saved')
|
||||||
},
|
},
|
||||||
onError: () => toast.error('Failed to save workflow'),
|
onError: (err: any) => {
|
||||||
|
if (err?.response?.status === 409) {
|
||||||
|
toast.error('Workflow was modified by someone else — please reload the page')
|
||||||
|
} else {
|
||||||
|
toast.error('Failed to save workflow')
|
||||||
|
}
|
||||||
|
},
|
||||||
})
|
})
|
||||||
|
|
||||||
const deleteMutation = useMutation({
|
const deleteMutation = useMutation({
|
||||||
@@ -199,8 +208,9 @@ export default function WorkflowEditor() {
|
|||||||
<WorkflowCanvas
|
<WorkflowCanvas
|
||||||
key={selectedWorkflow.id}
|
key={selectedWorkflow.id}
|
||||||
workflow={selectedWorkflow}
|
workflow={selectedWorkflow}
|
||||||
onSave={config => updateMutation.mutate({ id: selectedWorkflow.id, config })}
|
onSave={config => updateMutation.mutate({ id: selectedWorkflow.id, config, updated_at: selectedWorkflow.updated_at })}
|
||||||
isSaving={updateMutation.isPending}
|
isSaving={updateMutation.isPending}
|
||||||
|
canSave={canEdit}
|
||||||
/>
|
/>
|
||||||
) : (
|
) : (
|
||||||
<WorkflowEditorEmptyState
|
<WorkflowEditorEmptyState
|
||||||
|
|||||||
Reference in New Issue
Block a user