Skip to content

fix(sidebar): unify workflow and folder insertion ordering#3250

Merged
waleedlatif1 merged 7 commits intostagingfrom
fix/sidebar
Feb 18, 2026
Merged

fix(sidebar): unify workflow and folder insertion ordering#3250
waleedlatif1 merged 7 commits intostagingfrom
fix/sidebar

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • unify workflow and folder insertion ordering

Type of Change

  • Bug fix

Testing

Tested manually, added a bunch of unit tests

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Feb 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Feb 18, 2026 10:40pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 18, 2026

Greptile Summary

Unifies the sort order insertion logic across all workflow and folder creation/duplication paths. Previously, new items were placed at the top based only on siblings of the same type (e.g., workflow creation only checked other workflows). Now, all four code paths (workflow create, workflow duplicate, folder create, folder duplicate) compute the minimum sort order across both sibling workflows and sibling folders in the same parent scope, then insert at min - 1. A shared client-side utility getTopInsertionSortOrder replaces the divergent inline implementations.

  • Server-side routes (workflows/route.ts, folders/route.ts, folders/[id]/duplicate/route.ts, duplicate.ts) now query both the workflow and workflowFolder tables in parallel to find the global minimum sort order among siblings
  • Client-side optimistic updates in folders.ts and workflows.ts hooks now use the shared getTopInsertionSortOrder utility that considers both workflows and folders
  • The old getNextSortOrder function (which only looked at sibling folders and appended to the bottom) is removed
  • New unit tests cover the ordering logic for workflow creation, workflow duplication, folder creation/duplication, and the client-side optimistic sort computation

Confidence Score: 4/5

  • This PR is safe to merge — it consistently applies the same top-insertion sort order logic across all creation and duplication paths.
  • The changes are well-structured and consistent across all four code paths (workflow create/duplicate, folder create/duplicate) on both server and client. The logic is straightforward (find min sort order across sibling workflows and folders, subtract 1). New tests cover the key scenarios. The sort order reduce pattern is duplicated in four server-side locations rather than extracted into a shared helper, but this is a minor style concern, not a correctness issue.
  • No files require special attention.

Important Files Changed

Filename Overview
apps/sim/hooks/queries/utils/top-insertion-sort-order.ts New shared utility that computes top-insertion sort order from both workflows and folders. Clean implementation with correct filtering by workspace and parent.
apps/sim/hooks/queries/folders.ts Replaced old getNextSortOrder (bottom-insertion, folders only) with getTopInsertionSortOrder (top-insertion, mixed folders+workflows) in both useCreateFolder and useDuplicateFolderMutation. Arguments are passed in the correct order.
apps/sim/hooks/queries/workflows.ts Updated useCreateWorkflow and useDuplicateWorkflowMutation to use shared getTopInsertionSortOrder with both workflows and folders, replacing inline workflow-only sort logic.
apps/sim/app/api/workflows/route.ts POST handler now queries both workflow and folder tables for min sort order. Consistent with the unified insertion ordering approach. Default sort order 0 when no siblings matches the client-side utility.
apps/sim/app/api/folders/route.ts POST handler now uses min() aggregate on both workflow and folder tables instead of orderBy(desc).limit(1) on folders only. Consistent with the workflow route approach.
apps/sim/app/api/folders/[id]/duplicate/route.ts Folder duplication now computes sort order from both siblings instead of copying sourceFolder.sortOrder. Consistent with other routes.
apps/sim/lib/workflows/persistence/duplicate.ts Workflow duplication now queries both workflow and folder tables for min sort order, matching the unified approach. Default sort order changed from (null ?? 1) - 1 = 0 to explicit 0.
apps/sim/app/api/folders/route.test.ts Test mocks updated to support two select queries (folders + workflows). createMockTransaction refactored to use sequential mockReturnValueOnce. Sort order test validates actual computed value via onInsertValues callback.
apps/sim/app/api/workflows/route.test.ts New test file covering workflow POST ordering: mixed-sibling top insertion and no-siblings default. Tests verify both the response sort order and the actually inserted database values.
apps/sim/hooks/queries/folders.test.ts New test file covering client-side optimistic sort order for folder creation and duplication, verifying that mixed siblings (workflows + folders) are considered.
apps/sim/lib/workflows/persistence/duplicate.test.ts New test file covering workflow duplication ordering with mixed siblings and no-siblings scenarios. Tests verify both the return value sort order and the values passed to tx.insert.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Create/Duplicate Request] --> B{Server or Client?}
    B -->|Server API| C[Query min sort order from<br/>sibling workflows]
    B -->|Server API| D[Query min sort order from<br/>sibling folders]
    B -->|Client Optimistic| E[getTopInsertionSortOrder]
    C --> F[Promise.all]
    D --> F
    F --> G[Reduce to global minimum]
    E --> H[Filter workflows by<br/>workspaceId + folderId]
    E --> I[Filter folders by<br/>workspaceId + parentId]
    H --> J[Compute Math.min<br/>of all sibling sort orders]
    I --> J
    G --> K{min found?}
    J --> L{siblings exist?}
    K -->|Yes| M[sortOrder = min - 1]
    K -->|No| N[sortOrder = 0]
    L -->|Yes| O[sortOrder = min - 1]
    L -->|No| P[sortOrder = 0]
    M --> Q[Insert at top of list]
    N --> Q
    O --> Q
    P --> Q
Loading

Last reviewed commit: 8662179

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

@waleedlatif1 waleedlatif1 merged commit 2979269 into staging Feb 18, 2026
6 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/sidebar branch February 18, 2026 22:41
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

}
}

// Helper to recursively duplicate folder structure
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Response parentId diverges from inserted targetParentId

Low Severity

The DB insert uses targetParentId (computed with ?? on line 65), but the JSON response on line 160 still uses the old expression parentId || sourceFolder.parentId (with ||). The response parentId can differ from the value actually stored in the database — for instance, if parentId were an empty string, ?? would keep it while || would fall through to sourceFolder.parentId. The response parentId feeds the client store via mapFolder, so any mismatch means the client briefly shows the folder under the wrong parent until refetch.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments