Edit in sandbox: create or join a sandbox from a live workflow#4901
Edit in sandbox: create or join a sandbox from a live workflow#4901elias-ba wants to merge 1 commit into
Conversation
From a live workflow, "Edit in sandbox" opens a picker to either branch a new sandbox from the current live version or join an active one, then lands the user in the sandbox editor with that workflow's trigger live and its own endpoint. The parent stays untouched until a change is promoted back (a later slice of the epic). - New channel events: list a parent's active sandboxes (with collaborators and the joinable workflow, sorted by last edited) and create-and-open a sandbox. - Creating clones the parent project, then promotes the edited workflow to live inside the sandbox; the other cloned workflows stay draft so state and triggers remain coherent. The sandbox copy stays editable because the read-only lock only applies to live workflows outside a sandbox. - A sandbox badge in the editor header. Server-side errors (permission, usage limit) surface their real message in the picker rather than a generic one.
Security Review ✅
|
elias-ba
requested review from
doc-han and
lmac-1
and removed request for
doc-han
|
@elias-ba I noticed that you haven't checked the box that you've done an AI self review of the code here. Have you done a Claude review? |
There was a problem hiding this comment.
This is looking exciting @elias-ba - it's a big review so strap in 🤩! I've left a few comments in the code that I think are worth addressing within this PR. One final code comment:
assets/js/collaborative-editor/components/EditInSandboxPicker.tsx:1 and test files — multi-paragraph block comments violate CLAUDE.md
CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max."
Four blocks violate this:
- EditInSandboxPicker.tsx lines 1–14 (14-line file header with 3 paragraph breaks)
- EditInSandboxPicker.tsx lines 36–39 (serverMessage function comment)
- EditInSandboxPicker.test.tsx lines 1–12
- Header.sandbox.test.tsx lines 1–12
My feedback from a UX/UI perspective on "Edit in sandbox" experience (not blocking but observational):
- It was a bit confusing seeing "Join an active sandbox" and then both sandboxes having disabled "Join" buttons. Perhaps it shouldn't show this section if there is no sandbox to join?
-
When hovering over the disabled "Join" button in the above scenario, there was no explanatory tooltip message to explain why. Sometimes it appears but after a long pause, not instantly.
-
The "Join an active sandbox" wording was a bit confusing.. it made me feel like I wasn't a part of that sandbox before. Not sure what would be better wording here.
-
What exactly are the collaborator circles for? What do they represent?
-
If entering a sandbox name is optional, our styling in the header should support long workflow/sandbox names more elegantly. Suggestions:
(1) Make the Save button grow
(2) Add ellipsis for the sandbox/workflow name when it's a certain length:
-
I think we should improve the toast message when you try to add a sandbox with a name that already exists rather than the generic toast message. It should tell the user that a sandbox already exists with that name so they know how to fix it.
-
It is confusing for me when I switch to sandbox and see "Live" -> Switch to draft. I know what it means because of the extra context from meetings. But it's a bit strange to come from somewhere where I can't edit a live workflow and then it is marked as "Live" in the sandbox.
Some general comments outside of the scope of this PR of things I noticed for this epic while I'm here:
-
Switch to draft modal - should Esc close this modal?
-
Switch to draft modal: I'm not sure the message is clear enough that it will turn off the production workflow. Not sure of the wording but perhaps needs to warn more explicitly.
-
At one point I forgot I was reviewing this PR, went to rename my workflow and got confused why I couldn't edit anything. Perhaps this tooltip should be extended to mention why you don't have permission to edit and how to edit? (however, you might not be able to mention sandboxes as the experience is different for free vs paid users)
- The box shadows of the new buttons feel very different stylistically compared to the other parts of the application. Are they part of the new design system?
-
You cannot copy or see the full webhook URL for a live workflow
-
Go live button is the wrong colour when disabled (should be the same as Save)
-
I see the old trigger picker.. you should merge from main to avoid any nasty conflicts while you can 🤭
-
The top bar feels very hectic.. there is a LOT of things going on up there now. I'm sure this will get better as other epics come in but it feels like a lot.
| {overflow > 0 && ( | ||
| <span | ||
| className="inline-flex h-6 w-6 items-center justify-center rounded-full | ||
| bg-gray-100 text-[10px] font-medium text-gray-500 ring-2 ring-white" |
There was a problem hiding this comment.
I don't love text-[10px], but this is more for us to tidy up these sorts of sizes and design tokens when we generalise the design system more comprehensively later on. No block to merging.
| setShowEditInSandboxPicker(true); | ||
| }} | ||
| className={cn( | ||
| 'inline-flex items-center gap-1 rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 disabled:cursor-not-allowed disabled:opacity-50' |
There was a problem hiding this comment.
here we can just use className as it is one long static string
| {:reply, {:ok, %{project_id: sandbox.id, workflow_id: cloned_workflow.id}}, | ||
| socket} | ||
| else | ||
| error -> workflow_error_reply(socket, error) |
There was a problem hiding this comment.
workflow_error_reply in workflow_channel.ex doesn't have a clause for {:error, :nesting_too_deep}, which is one of the return values from Projects.provision_sandbox (see sandboxes.ex:585). If that error reaches workflow_error_reply, the channel process will crash and drop the user's WebSocket connection. Needs a handler clause added, similar to the existing ones at line 1045-1069.
| promote_cloned_workflow(sandbox, workflow.name, user) do | ||
| {:reply, {:ok, %{project_id: sandbox.id, workflow_id: cloned_workflow.id}}, | ||
| socket} | ||
| else |
There was a problem hiding this comment.
If provision_sandbox succeeds but promote_cloned_workflow fails, the sandbox is already committed to the database and this else branch doesn't do anything to clean it up.
The user gets an error but the sandbox persists and consume quota. Not blocking but could add a compensating delete of the sandbox here, or both steps wrapped in a single transaction to make it a better user experience.
| end | ||
| end | ||
|
|
||
| defp promote_cloned_workflow(sandbox, workflow_name, user) do |
There was a problem hiding this comment.
This function queries the database directly instead of going through the Workflows context. It would be better to extract a Workflows.get_workflow_by_name(project_id, name) function in lib/lightning/workflows.ex
| "#{base}-sandbox" | ||
| end | ||
|
|
||
| defp random_sandbox_color do |
There was a problem hiding this comment.
This is a duplicate of get_random_color/0 in sandbox_live/form_component_ex.
Since color_palette_hex_colors/0 already lives in LightningWeb.SandboxLive.Components we could extract this into a shared public random_color/0 function there and call it from both places.
| } | ||
| end | ||
|
|
||
| defp collaborator_name(user) do |
There was a problem hiding this comment.
Non-blocking. Claude mentioned how this function is duplicating the first/last name logic that already appears in mailer.ex, workflow_json.ex and elsewhere. There isn't a canonical helper yet. But we could extract this out to a shared Accounts.display_name/1 function (not sure if that's the right place) and replace all the copies. This is out of scope of this PR but flagging for follow-up.
| joinable_workflow_ids = | ||
| from(w in Workflow, | ||
| where: | ||
| w.project_id in ^sandbox_ids and w.name == ^workflow_name and |
There was a problem hiding this comment.
If the parent project has no sandboxes, sandbox_ids will be [] and the workflow lookup query still runs against the database unnecessarily, always returning nothing. Suggest adding an early return guard here: if sandbox_ids == [], do: []
2 participants
Description
This PR adds "Edit in sandbox". From a live workflow, the header gains an "Edit in sandbox" action that opens a picker to either branch a new sandbox from the current live version or join an active one, then lands the user in the sandbox editor with that workflow's trigger live and its own endpoint. The parent project is untouched; changes come back through promotion in a later slice of the epic.
Creating clones the parent project (the existing full-clone provisioning), then promotes the edited workflow to live inside the sandbox. The other cloned workflows stay draft, so a workflow's state stays coherent with its triggers. The sandbox copy remains editable because the live read-only lock only applies outside a sandbox. The picker lists the parent's active sandboxes (last edited first, with collaborators), and join is disabled for a sandbox that does not contain this workflow. A sandbox badge is shown in the editor header. Server-side errors (permission, usage limit) surface their real message in the picker, including the limit upsell, rather than a generic toast.
Closes #4859
Validation steps
Additional notes for the reviewer
:provision_sandboxpermission (owner/admin/editor) and the:new_sandboxusage limit.AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)integration branch; the changelog is updated when the epic merges to main.)