Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app): stable default workflows, workflow saving and loading fixes #7729

Merged
merged 12 commits into from
Mar 5, 2025

Conversation

psychedelicious
Copy link
Collaborator

Summary

  • Default workflows now have stable identity. This means we can have thumbnails for them.
  • Refactor loading and saving of workflows to support this. Also makes a lot of workflows logic simpler.

See commits for detailed messages.

Related Issues / Discussions

offline discussion

QA Instructions

Try it out. The core logic changes here will also be used in the new workflow library UI PR, so we can probably just skip to testing that PR instead of spending time on this one.

Merge Plan

This needs to merge after #7676 but before #7710.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions bot added api python PRs that change python files services PRs that change app services frontend PRs that change frontend files labels Mar 4, 2025
@psychedelicious psychedelicious marked this pull request as draft March 4, 2025 04:13
@psychedelicious
Copy link
Collaborator Author

Marked as draft to prevent premature merge.

@psychedelicious psychedelicious force-pushed the maryhipp/workflow-thumbnails branch from 4000178 to fa179ce Compare March 5, 2025 23:32
Base automatically changed from maryhipp/workflow-thumbnails to main March 5, 2025 23:41
There was a bit of wonk with default workflows. On every app startup, we wiped them all out and recreated them with new IDs. This is a quick-and-dirty way to ensure default workflows are always in sync.

Unfortunately, it also means default workflows are newly-created entities on every app load. Any thumbnails associated to them will be lost (bc they have new IDs), and `updated_at` doesn't work.

This changes makes default workflows stable entities.

The workflows we bundle in the python package in JSON format are still the source of truth for default workflows, but the startup logic that syncs them to the user DB is a bit smarter.

- All bundled workflows have an ID. It is prefixed with "default_" for  clarity.
- Any default workflows in the user's DB that are not in the bundled default workflows are deleted from the DB.
- Any bundled default workflows that are not in the user's DB are added to the DB.
- If a default workflow in the user's DB does not match the content of its corresponding bundled workflow, it is updated in the DB.

The end result is that default workflows are still kept in sync for the user, but they don't change their identity.

We may now add thumbnails to default workflows, and sorting by `updated_at` is now meaningful.
It's only by misunderstanding the pydantic API that this field was is typed as optional. Workflows must _always_ have a category, and indeed they do.

Fixing this allows the generated types in the frontend to be easier to work with..
This big chungus reworks and simplifies much of the logic around loading and saving workflows. It also makes some minor changes to how store the current workflow and determine if it is a draft, user workflow or default workflow.

---

The lower-level hooks to save a workflow have been revised:
- `useSaveLibraryWorkflow`: Saves a user or project workflow that has had changes made to it.
- `useCreateNewWorkflow`: Saves a workflow as a new entity.

A new higher-level hook `useSaveOrSaveAsWorkflow` is intended to be used by components. It returns a single function that:
- Constructs the workflow payload to be sent to the server
- Checks if the workflow is an existing user workflow. If so, it immediately saves (updates) that workflow.
- If it's not an existing user workflow, it opens the save as dialog so the user can choose a name for it and create a new workflow. This occurs for both draft workflows and loaded default workflows.

---

The logic to build the current redux state into a workflow - either to be saved as JSON, to update an existing user workflow, or save as - was a bit convoluted.

Changes to redux state triggered a debounced function to build the workflow, setting it in a global nanostores atom. Then, all of the functions that consumed the "built workflow" referenced this atom.

Now, this logic is strictly imperative. When a consumer wants to save a workflow, we build it on the spot. This removes a layer of indirection.

The logic is in the `useBuildWorkflowFast` hook.

---

The logic for loading a workflow is also revised. Previously, it happened in an RTK listener. You'd need to dispatch an action to load a workflow, and wouldn't know if it succeeded or not (though the listener would make a toast if the load failed).

This is now done in a callback, outside redux middleware. The callback is returned from the `useLoadWorkflow` hook.

---

Previously, we stripped the id from default workflows when loading them. Then, when saving the workflow, we built a workflow object from redux state and hit the API with it.

This has two issues:
- It relies on redux state never having an ID set when a default workflow is loaded. If we somehow ended up with a default workflow's ID in redux, when we go to save the workflow, we'd get and error or it wouldn't work, because you cannot save a default workflow. You can only save-as it.
- We do not know the default workflow from which the current workflow was loaded. And be cause we don't know the default workflow, we cannot show a thumbnail image.

The responsibilities have been shifted around a bit.

Now, when we load a workflow, we load it as-is. The default workflow IDs are saved in redux state. We can render the thumbnail, and if the user goes to save the workflow, we detect that it is a default workflow and save-as it.

---

In `App.tsx`, the long list of modals are moved into their own "isolator" component to ensure any re-renders there do not affect the rest of the app.

---

The save-workflow-as modal is restructured to be a bit simpler. Still works the same. On commercial, "save to project" will be enabled by default.

---

The workflow JSON tab uses a debounced version of "buildWorkflow" to build the workflow as JSON.

---

`buildWorkflowFast` is updated to deep-copy its _whole_ output, preventing issues where field types could accidentally get mutated. I don't think this has ever happened but we may as well be safe.

---

Fixed an issue where the edit button in the workflow list didn't open the workflow in edit mode.
@psychedelicious psychedelicious force-pushed the psyche/feat/app/stable-default-workflows branch from f20ab50 to 43aae93 Compare March 5, 2025 23:43
@psychedelicious psychedelicious marked this pull request as ready for review March 5, 2025 23:43
@psychedelicious psychedelicious enabled auto-merge (rebase) March 5, 2025 23:44
@github-actions github-actions bot added the invocations PRs that change invocations label Mar 5, 2025
@github-actions github-actions bot added backend PRs that change backend files python-tests PRs that change python tests labels Mar 5, 2025
@psychedelicious psychedelicious merged commit cf0cbaf into main Mar 5, 2025
15 checks passed
@psychedelicious psychedelicious deleted the psyche/feat/app/stable-default-workflows branch March 5, 2025 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api backend PRs that change backend files frontend PRs that change frontend files invocations PRs that change invocations python PRs that change python files python-tests PRs that change python tests services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants