OpenFn / OpenFn/lightning

Derive `workflow_kind` instead of caching it on the socket

Open
#5,028 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

refactor tech-debt
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:

  1. Join the channel with action: :new for an id with no row.
  2. Push save_and_sync with a commit message (needs a repo connection, or stub
    VersionControl.initiate_sync/2).
  3. Push request_versions.
  4. 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_kind no longer exists.
  • Existing tests for first save via save_workflow still 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.