Derive `workflow_kind` instead of caching it on the socket
Nobody has claimed this yet.
- Dominant language
- Elixir
- Stars
- 296
- Forks
- 86
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 50
Description
Depends on #4848
User story
As an engineer working on the workflow channel, I want "has this workflow been
saved yet?" to be a question I can ask, instead of a stored answer I have to
remember to keep up to date.
Details
When you open the editor, the server writes down whether the workflow already
exists in the database. It never checks again. Saving is what makes that note
wrong, and only one of the two save paths updates it.
The note is workflow_kind. Set on connect (workflow_channel.ex:46, stored at
:84), corrected in one place — the success branch of save_workflow (:366).
Save & Sync saves through the same code (:398) and doesn't correct it.
Two handlers read it, and both use it to skip a database read:
# get_context, :212
if workflow_kind == :new do
{workflow, nil} # in-memory copy, no version
else
fresh = Lightning.Workflows.get_workflow(workflow.id, include: [...])
{fresh, (fresh && fresh.lock_version) || workflow.lock_version}
end
# request_versions, :476
if workflow_kind == :new do
%{versions: []} # no history
else
...
end
So once the note is stale, the editor is told the workflow has no version
history and is handed a stale copy of the workflow, for the rest of the session.
Only a refresh clears it.
You can't hit this today. Save & Sync lives in the header, and the header
isn't rendered until the workflow exists, so Save & Sync can never be your first
save. That's why it was disclosed and marked WON'T FIX, and it still holds.
Why file it. The only thing making that true is a comment in frontend code
(useWorkflow.tsx:601-604). The server is relying on a UI rule it can't see or
enforce. Anything else holding the connection can call Save & Sync directly, and
whoever next changes when the header renders won't read that comment.
The note isn't earning its keep. Both readers only ask it "is this new?", and
when the answer is no they read the database anyway. The save code doesn't trust
it either — it re-checks on every save (session.ex:331).
How to see it
No UI repro exists, by design. Use a channel test:
- Join the channel with
action: :newfor an id with no row. - Push
save_and_syncwith a commit message (needs a repo connection, or stub
VersionControl.initiate_sync/2). - Push
request_versions. - It replies
%{versions: []}. The workflow now has a row and a snapshot.
Swapping step 2 for save_workflow returns the versions correctly. That
difference is the bug.
Implementation notes
Delete the workflow_kind assign and ask the database directly. Both readers
already make the call that answers it.
# get_context
{fresh_workflow, latest_lock_version} =
case Lightning.Workflows.get_workflow(workflow.id,
include: [:edges, :jobs, :triggers]
) do
nil -> {workflow, nil}
fresh -> {fresh, fresh.lock_version}
end
# request_versions
case Lightning.Workflows.get_workflow(workflow.id) do
nil -> %{versions: []}
fresh_workflow -> ...
end
That removes the assign at :84, the correction at :366, and the Save & Sync
gap together. Net effect is one extra SELECT on a brand-new workflow's first
get_context, which is a query that currently returns nil anyway.
Keep :new itself. It's doing real work in WorkflowResolver.resolve/3
(workflow_resolver.ex:82-107): a :new action with no row builds a struct in
Ecto's :built state so the save routes to an INSERT, and a :new action that
does find a row resolves to the row so the save routes to an UPDATE instead
(that's the #4830 fix). Stop storing the answer, don't delete the concept.
Not the same as #5009. validate_workflow_name (:456) is a third handler
with a similar smell, but it reads neither the note nor the workflow id — it
works off the project alone. Its fix is where: w.id != ^id. Neither issue fixes
the other.
Frontend item worth doing in the same sitting. The read-only rule for unsaved
workflows still says Click "Create" to edit this workflow, naming a button that
no longer exists. Rewording it needs a product answer first: should the canvas be
read-only while the AI is generating? If yes, that rule inverts rather than
disappears — so leave the dead || isNewWorkflow in ReadOnlyWarning.tsx:30
alone until it's decided.
User acceptance criteria
- The channel test above passes: first save via
save_and_sync, then
request_versions, returns the workflow's versions. socket.assigns.workflow_kindno longer exists.- Existing tests for first save via
save_workflowstill pass unchanged. - A second save still routes to an UPDATE, not a duplicate INSERT (#4830's test
still passes).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in workflow_channel.ex at the workflow_kind assign and the get_context, save_workflow, and request_versions handlers; use the channel test scenario described in the issue to reproduce the stale versions response. Check workflow_resolver.ex:82-107 and the existing #4830 test to preserve new-versus-existing save behavior. Done means the first save_and_sync is followed by correct versions, workflow_kind is absent, and existing save tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elixir
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100