diff --git a/apps/studio/src/components/sidebar/ConnectionSidebar.vue b/apps/studio/src/components/sidebar/ConnectionSidebar.vue index 70084991f..c665542dd 100644 --- a/apps/studio/src/components/sidebar/ConnectionSidebar.vue +++ b/apps/studio/src/components/sidebar/ConnectionSidebar.vue @@ -673,23 +673,12 @@ export default { async onConnectionFolderHeaderDrop(folder) { if (!this.draggingConnection) return try { - if (this.isCloud) { - await this.$store.dispatch('data/connections/save', { - ...this.draggingConnection, - connectionFolderId: folder.id, - position: { before: null } - }) - } else { - const folderItems = this.filteredConnections.filter(c => c.connectionFolderId === folder.id) - const newList = [this.draggingConnection, ...folderItems.filter(c => c.id !== this.draggingConnection.id)] - await this.$store.dispatch('data/connections/saveMany', - newList.map((item, idx) => ({ - ...item, - connectionFolderId: folder.id, - position: idx + 1 - })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/connections/reorder', { + item: this.draggingConnection, + connectionFolderId: folder.id, + position: { before: null } + }) } catch (ex) { this.$noty.error(`Move error: ${ex.message}`) } @@ -698,33 +687,19 @@ export default { try { if (event.added) { const { element: item, newIndex } = event.added - if (this.isCloud) { - await this.$store.dispatch('data/connections/save', { - ...item, - connectionFolderId: folder?.id ?? null, - position: this.cloudRelativePosition(currentList, newIndex) - }) - } else { - await this.$store.dispatch('data/connections/saveMany', - currentList.map((item, idx) => ({ - ...item, - connectionFolderId: folder?.id ?? null, - position: idx + 1 - })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/connections/reorder', { + item, + connectionFolderId: folder?.id ?? null, + position: this.cloudRelativePosition(currentList, newIndex) + }) } else if (event.moved) { const { element: item, newIndex } = event.moved - if (this.isCloud) { - await this.$store.dispatch('data/connections/save', { - ...item, - position: this.cloudRelativePosition(currentList, newIndex) - }) - } else { - await this.$store.dispatch('data/connections/saveMany', - currentList.map((item, idx) => ({ ...item, position: idx + 1 })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/connections/reorder', { + item, + position: this.cloudRelativePosition(currentList, newIndex) + }) } } catch (ex) { this.$noty.error(`Move error: ${ex.message}`) diff --git a/apps/studio/src/components/sidebar/core/FavoriteList.vue b/apps/studio/src/components/sidebar/core/FavoriteList.vue index 60284ba41..fa8fa4a8f 100644 --- a/apps/studio/src/components/sidebar/core/FavoriteList.vue +++ b/apps/studio/src/components/sidebar/core/FavoriteList.vue @@ -465,23 +465,12 @@ export default { async onQueryFolderHeaderDrop(folder) { if (!this.draggingQuery) return try { - if (this.isCloud) { - await this.$store.dispatch('data/queries/save', { - ...this.draggingQuery, - queryFolderId: folder.id, - position: { before: null } - }) - } else { - const folderItems = this.filteredQueries.filter(q => q.queryFolderId === folder.id) - const newList = [this.draggingQuery, ...folderItems.filter(q => q.id !== this.draggingQuery.id)] - await this.$store.dispatch('data/queries/saveMany', - newList.map((item, idx) => ({ - ...item, - queryFolderId: folder.id, - position: idx + 1 - })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/queries/reorder', { + item: this.draggingQuery, + queryFolderId: folder.id, + position: { before: null } + }) } catch (ex) { this.$noty.error(`Move error: ${ex.message}`) } @@ -490,33 +479,19 @@ export default { try { if (event.added) { const { element: item, newIndex } = event.added - if (this.isCloud) { - await this.$store.dispatch('data/queries/save', { - ...item, - queryFolderId: folder?.id ?? null, - position: this.cloudRelativePosition(currentList, newIndex) - }) - } else { - await this.$store.dispatch('data/queries/saveMany', - currentList.map((item, idx) => ({ - ...item, - queryFolderId: folder?.id ?? null, - position: idx + 1 - })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/queries/reorder', { + item, + queryFolderId: folder?.id ?? null, + position: this.cloudRelativePosition(currentList, newIndex) + }) } else if (event.moved) { const { element: item, newIndex } = event.moved - if (this.isCloud) { - await this.$store.dispatch('data/queries/save', { - ...item, - position: this.cloudRelativePosition(currentList, newIndex) - }) - } else { - await this.$store.dispatch('data/queries/saveMany', - currentList.map((item, idx) => ({ ...item, position: idx + 1 })) - ) - } + // Use reorder action for both local and cloud workspaces + await this.$store.dispatch('data/queries/reorder', { + item, + position: this.cloudRelativePosition(currentList, newIndex) + }) } } catch (ex) { this.$noty.error(`Move error: ${ex.message}`) diff --git a/apps/studio/src/lib/cloud/controllers/ConnectionsController.ts b/apps/studio/src/lib/cloud/controllers/ConnectionsController.ts index 5a65bc8d8..8c13543e0 100644 --- a/apps/studio/src/lib/cloud/controllers/ConnectionsController.ts +++ b/apps/studio/src/lib/cloud/controllers/ConnectionsController.ts @@ -1,8 +1,23 @@ import { ICloudSavedConnection } from "@/common/interfaces/IConnection"; import { GenericController } from "@/lib/cloud/controllers/GenericController"; +import { res, url } from "@/lib/cloud/ClientHelpers"; + +export interface ReorderResult { + id: number; + position: number; + updatedAt: number; +} export class ConnectionsController extends GenericController { name = 'connection' plural = 'connections' path = '/connections' + + async reorder(id: number, position: { before?: number | null; after?: number } | number, connectionFolderId?: number | null): Promise { + const response = await this.axios.patch(url(this.path, id, 'reorder'), { + position, + connectionFolderId + }) + return res(response, 'connections') + } } \ No newline at end of file diff --git a/apps/studio/src/lib/cloud/controllers/QueriesController.ts b/apps/studio/src/lib/cloud/controllers/QueriesController.ts index 00369d7bb..2a22bd9c9 100644 --- a/apps/studio/src/lib/cloud/controllers/QueriesController.ts +++ b/apps/studio/src/lib/cloud/controllers/QueriesController.ts @@ -1,10 +1,23 @@ - import ISavedQuery from '@/common/interfaces/ISavedQuery' import { GenericController } from '@/lib/cloud/controllers/GenericController' +import { res, url } from "@/lib/cloud/ClientHelpers"; +export interface ReorderResult { + id: number; + position: number; + updatedAt: number; +} export class QueriesController extends GenericController { name = 'query' plural = 'queries' path = '/queries' + + async reorder(id: number, position: { before?: number | null; after?: number } | number, queryFolderId?: number | null): Promise { + const response = await this.axios.patch(url(this.path, id, 'reorder'), { + position, + queryFolderId + }) + return res(response, 'queries') + } } \ No newline at end of file diff --git a/apps/studio/src/store/modules/data/DataModuleBase.ts b/apps/studio/src/store/modules/data/DataModuleBase.ts index 983fe2e6c..24d3cb682 100644 --- a/apps/studio/src/store/modules/data/DataModuleBase.ts +++ b/apps/studio/src/store/modules/data/DataModuleBase.ts @@ -25,6 +25,7 @@ export interface DataState { error: ClientError pollError: ClientError filter?: string + pendingSaveIds?: number[] } @@ -72,6 +73,17 @@ const buildBasicMutations = (sortBy?: SortSpec) => ({ pollError(state, error: Error | null) { state.pollError = error }, + addPendingSave(state, id: number) { + if (!state.pendingSaveIds) state.pendingSaveIds = [] + if (!state.pendingSaveIds.includes(id)) { + state.pendingSaveIds.push(id) + } + }, + removePendingSave(state, id: number) { + if (state.pendingSaveIds) { + state.pendingSaveIds = state.pendingSaveIds.filter((i) => i !== id) + } + }, set(state, items: T[] | T) { items = _.isArray(items) ? items : [items]; const sorted = sortBy ? _.sortBy(items, sortBy.field) : items; @@ -87,13 +99,18 @@ const buildBasicMutations = (sortBy?: SortSpec) => ({ state.items = sortBy?.direction === 'desc' ? sorted.reverse() : sorted }, replace(state, items: T[]) { + const pendingIds = state.pendingSaveIds || [] const itemIds = items.map((i) => i.id) const stateIds = state.items.map((i) => i.id) - const toUpdate = items.filter((i) => stateIds.includes(i.id)) + // Don't update items that have pending saves - keep local optimistic version + const toUpdate = items.filter((i) => stateIds.includes(i.id) && !pendingIds.includes(i.id)) const toInsert = items.filter((i) => !stateIds.includes(i.id)) - const stateItems = _.reject(state.items, (item) => !itemIds.includes(item.id)) + // Don't remove items that have pending saves + const stateItems = _.reject(state.items, (item) => + !itemIds.includes(item.id) && !pendingIds.includes(item.id) + ) const upsertable = [...toUpdate, ...toInsert] upsertable.forEach((i) => upsert(stateItems, i)) const sorted = sortBy ? _.sortBy(stateItems, sortBy.field) : stateItems diff --git a/apps/studio/src/store/modules/data/connection/CloudConnectionModule.ts b/apps/studio/src/store/modules/data/connection/CloudConnectionModule.ts index 731d6824b..4f6d9e2be 100644 --- a/apps/studio/src/store/modules/data/connection/CloudConnectionModule.ts +++ b/apps/studio/src/store/modules/data/connection/CloudConnectionModule.ts @@ -12,7 +12,8 @@ export const CloudConnectionModule: DataStore = { loading: false, error: null, pollError: null, - filter: undefined + filter: undefined, + pendingSaveIds: [] }, mutations: mutationsFor({ connectionFilter(state: State, str: string) { @@ -24,11 +25,78 @@ export const CloudConnectionModule: DataStore = { context.commit('connectionFilter', filter); }, 500), async saveMany(context, items: ICloudSavedConnection[]) { + // Mark items as pending to protect from poll overwrites + items.forEach(item => context.commit('addPendingSave', item.id)) context.commit('upsert', items) - return havingCli(context, async (cli) => { - const saved = await Promise.all(items.map(item => cli.connections.upsert(item))) - context.commit('upsert', saved) + try { + return await havingCli(context, async (cli) => { + const saved = await Promise.all(items.map(item => cli.connections.upsert(item))) + context.commit('upsert', saved) + }) + } finally { + // Clear pending status + items.forEach(item => context.commit('removePendingSave', item.id)) + } + }, + // Reorder action for drag/drop - uses dedicated reorder API that returns all affected positions + async reorder(context, { item, position, connectionFolderId }) { + // Get the full item from state for optimistic update + const existing = context.state.items.find(c => c.id === item.id) + if (!existing) return + + // Calculate optimistic numeric position + let optimisticPosition = 1 + if (typeof position === 'object') { + const siblings = context.state.items.filter( + c => c.connectionFolderId === (connectionFolderId ?? existing.connectionFolderId) + ) + if (position.after) { + const afterItem = siblings.find(c => c.id === position.after) + optimisticPosition = afterItem ? (afterItem.position ?? 0) + 0.5 : 1 + } else if (position.before) { + const beforeItem = siblings.find(c => c.id === position.before) + optimisticPosition = beforeItem ? Math.max(0, (beforeItem.position ?? 1) - 0.5) : 1 + } else { + // { before: null } means first position + const minPos = Math.min(...siblings.filter(c => c.id !== item.id).map(c => c.position ?? 1)) + optimisticPosition = Math.max(0, minPos - 1) + } + } + + // Mark as pending to protect from poll overwrites + context.commit('addPendingSave', item.id) + + // Optimistic commit with numeric position + context.commit('upsert', { + ...existing, + connectionFolderId: connectionFolderId ?? existing.connectionFolderId, + position: optimisticPosition }) + + // Use dedicated reorder API that returns all affected positions + try { + return await havingCli(context, async (cli) => { + const affectedItems = await cli.connections.reorder( + item.id, + position, + connectionFolderId + ) + // Update all affected items with their new positions + affectedItems.forEach(affected => { + const existing = context.state.items.find(c => c.id === affected.id) + if (existing) { + context.commit('upsert', { + ...existing, + position: affected.position + }) + } + }) + return item.id + }) + } finally { + // Clear pending status + context.commit('removePendingSave', item.id) + } } }), getters: { diff --git a/apps/studio/src/store/modules/data/connection/UtilityConnectionModule.ts b/apps/studio/src/store/modules/data/connection/UtilityConnectionModule.ts index 8cd2cb84a..fb79baea8 100644 --- a/apps/studio/src/store/modules/data/connection/UtilityConnectionModule.ts +++ b/apps/studio/src/store/modules/data/connection/UtilityConnectionModule.ts @@ -1,6 +1,7 @@ import { IConnection } from "@/common/interfaces/IConnection"; import { DataState, DataStore, mutationsFor, utilActionsFor } from "@/store/modules/data/DataModuleBase"; import _ from "lodash"; +import Vue from "vue"; type State = DataState @@ -11,7 +12,8 @@ export const UtilConnectionModule: DataStore = { loading: false, error: null, pollError: null, - filter: undefined + filter: undefined, + pendingSaveIds: [] }, mutations: mutationsFor({ connectionFilter(state: DataState, str: string) { @@ -21,7 +23,66 @@ export const UtilConnectionModule: DataStore = { actions: utilActionsFor('saved', { setConnectionFilter: _.debounce(function (context, filter) { context.commit('connectionFilter', filter); - }, 500) + }, 500), + + // Reorder action for drag/drop - matches cloud module interface + async reorder(context, { item, position, connectionFolderId }) { + const existing = context.state.items.find(c => c.id === item.id) + if (!existing) return + + const targetFolderId = connectionFolderId ?? existing.connectionFolderId + + // Get all siblings in the target folder, sorted by position + const siblings = context.state.items + .filter(c => c.connectionFolderId === targetFolderId) + .sort((a, b) => (a.position ?? 0) - (b.position ?? 0)) + + // Build the new ordered list + let newOrder: IConnection[] + const itemWithoutSelf = siblings.filter(c => c.id !== item.id) + + if (typeof position === 'object') { + if (position.after) { + const afterIndex = itemWithoutSelf.findIndex(c => c.id === position.after) + newOrder = [ + ...itemWithoutSelf.slice(0, afterIndex + 1), + { ...existing, connectionFolderId: targetFolderId }, + ...itemWithoutSelf.slice(afterIndex + 1) + ] + } else if (position.before) { + const beforeIndex = itemWithoutSelf.findIndex(c => c.id === position.before) + newOrder = [ + ...itemWithoutSelf.slice(0, beforeIndex), + { ...existing, connectionFolderId: targetFolderId }, + ...itemWithoutSelf.slice(beforeIndex) + ] + } else { + // { before: null } means first position + newOrder = [{ ...existing, connectionFolderId: targetFolderId }, ...itemWithoutSelf] + } + } else { + // Numeric position - insert at that index + newOrder = [...itemWithoutSelf] + newOrder.splice(position, 0, { ...existing, connectionFolderId: targetFolderId }) + } + + // Assign sequential positions + const updates = newOrder.map((c, idx) => ({ + ...c, + position: idx + 1 + })) + + // Optimistic update + context.commit('upsert', updates) + + // Save all items + const saved = await Promise.all( + updates.map(c => Vue.prototype.$util.send('appdb/saved/save', { obj: c })) + ) + context.commit('upsert', saved) + + return item.id + } }), getters: { filteredConnections(state) { diff --git a/apps/studio/src/store/modules/data/query/CloudQueryModule.ts b/apps/studio/src/store/modules/data/query/CloudQueryModule.ts index e18ab1d11..c8db5ebde 100644 --- a/apps/studio/src/store/modules/data/query/CloudQueryModule.ts +++ b/apps/studio/src/store/modules/data/query/CloudQueryModule.ts @@ -13,7 +13,8 @@ export const CloudQueryModule: DataStore = { loading: false, error: null, pollError: null, - filter: undefined + filter: undefined, + pendingSaveIds: [] }, mutations: mutationsFor({ // more mutations go here @@ -26,11 +27,78 @@ export const CloudQueryModule: DataStore = { context.commit('savedQueryFilter', filter); }, 500), async saveMany(context, items: ISavedQuery[]) { + // Mark items as pending to protect from poll overwrites + items.forEach(item => context.commit('addPendingSave', item.id)) context.commit('upsert', items) - return havingCli(context, async (cli) => { - const saved = await Promise.all(items.map(item => cli.queries.upsert(item))) - context.commit('upsert', saved) + try { + return await havingCli(context, async (cli) => { + const saved = await Promise.all(items.map(item => cli.queries.upsert(item))) + context.commit('upsert', saved) + }) + } finally { + // Clear pending status + items.forEach(item => context.commit('removePendingSave', item.id)) + } + }, + // Reorder action for drag/drop - uses dedicated reorder API that returns all affected positions + async reorder(context, { item, position, queryFolderId }) { + // Get the full item from state for optimistic update + const existing = context.state.items.find(q => q.id === item.id) + if (!existing) return + + // Calculate optimistic numeric position + let optimisticPosition = 1 + if (typeof position === 'object') { + const siblings = context.state.items.filter( + q => q.queryFolderId === (queryFolderId ?? existing.queryFolderId) + ) + if (position.after) { + const afterItem = siblings.find(q => q.id === position.after) + optimisticPosition = afterItem ? (afterItem.position ?? 0) + 0.5 : 1 + } else if (position.before) { + const beforeItem = siblings.find(q => q.id === position.before) + optimisticPosition = beforeItem ? Math.max(0, (beforeItem.position ?? 1) - 0.5) : 1 + } else { + // { before: null } means first position + const minPos = Math.min(...siblings.filter(q => q.id !== item.id).map(q => q.position ?? 1)) + optimisticPosition = Math.max(0, minPos - 1) + } + } + + // Mark as pending to protect from poll overwrites + context.commit('addPendingSave', item.id) + + // Optimistic commit with numeric position + context.commit('upsert', { + ...existing, + queryFolderId: queryFolderId ?? existing.queryFolderId, + position: optimisticPosition }) + + // Use dedicated reorder API that returns all affected positions + try { + return await havingCli(context, async (cli) => { + const affectedItems = await cli.queries.reorder( + item.id, + position, + queryFolderId + ) + // Update all affected items with their new positions + affectedItems.forEach(affected => { + const existing = context.state.items.find(q => q.id === affected.id) + if (existing) { + context.commit('upsert', { + ...existing, + position: affected.position + }) + } + }) + return item.id + }) + } finally { + // Clear pending status + context.commit('removePendingSave', item.id) + } } }), getters: { diff --git a/apps/studio/src/store/modules/data/query/UtilityQueryModule.ts b/apps/studio/src/store/modules/data/query/UtilityQueryModule.ts index 0530f0aa7..4df760b52 100644 --- a/apps/studio/src/store/modules/data/query/UtilityQueryModule.ts +++ b/apps/studio/src/store/modules/data/query/UtilityQueryModule.ts @@ -1,5 +1,6 @@ import { TransportFavoriteQuery } from '@/common/transport'; import _ from 'lodash' +import Vue from 'vue' import { mutationsFor, DataState, DataStore, utilActionsFor } from '../DataModuleBase' export const UtilQueryModule: DataStore> = { @@ -9,7 +10,8 @@ export const UtilQueryModule: DataStore({ // more mutations go here @@ -20,7 +22,66 @@ export const UtilQueryModule: DataStore('query', { setSavedQueryFilter: _.debounce(function (context, filter) { context.commit('savedQueryFilter', filter); - }, 500) + }, 500), + + // Reorder action for drag/drop - matches cloud module interface + async reorder(context, { item, position, queryFolderId }) { + const existing = context.state.items.find(q => q.id === item.id) + if (!existing) return + + const targetFolderId = queryFolderId ?? existing.queryFolderId + + // Get all siblings in the target folder, sorted by position + const siblings = context.state.items + .filter(q => q.queryFolderId === targetFolderId) + .sort((a, b) => (a.position ?? 0) - (b.position ?? 0)) + + // Build the new ordered list + let newOrder: TransportFavoriteQuery[] + const itemWithoutSelf = siblings.filter(q => q.id !== item.id) + + if (typeof position === 'object') { + if (position.after) { + const afterIndex = itemWithoutSelf.findIndex(q => q.id === position.after) + newOrder = [ + ...itemWithoutSelf.slice(0, afterIndex + 1), + { ...existing, queryFolderId: targetFolderId }, + ...itemWithoutSelf.slice(afterIndex + 1) + ] + } else if (position.before) { + const beforeIndex = itemWithoutSelf.findIndex(q => q.id === position.before) + newOrder = [ + ...itemWithoutSelf.slice(0, beforeIndex), + { ...existing, queryFolderId: targetFolderId }, + ...itemWithoutSelf.slice(beforeIndex) + ] + } else { + // { before: null } means first position + newOrder = [{ ...existing, queryFolderId: targetFolderId }, ...itemWithoutSelf] + } + } else { + // Numeric position - insert at that index + newOrder = [...itemWithoutSelf] + newOrder.splice(position, 0, { ...existing, queryFolderId: targetFolderId }) + } + + // Assign sequential positions + const updates = newOrder.map((q, idx) => ({ + ...q, + position: idx + 1 + })) + + // Optimistic update + context.commit('upsert', updates) + + // Save all items + const saved = await Promise.all( + updates.map(q => Vue.prototype.$util.send('appdb/query/save', { obj: q })) + ) + context.commit('upsert', saved) + + return item.id + } }, {}, { text: true, title: true, database: true, excerpt: true, id: true }), getters: { filteredQueries(state) { diff --git a/apps/studio/tests/unit/components/sidebar/FolderHeaderDrop.spec.ts b/apps/studio/tests/unit/components/sidebar/FolderHeaderDrop.spec.ts index 8273a99f7..0bd848523 100644 --- a/apps/studio/tests/unit/components/sidebar/FolderHeaderDrop.spec.ts +++ b/apps/studio/tests/unit/components/sidebar/FolderHeaderDrop.spec.ts @@ -5,6 +5,7 @@ import Vuex from "vuex"; Vue.use(Vuex); // Minimal stub components for ConnectionSidebar and FavoriteList +// Both now use unified reorder action for local and cloud workspaces const ConnectionSidebarStub = { template: "
", data() { @@ -12,41 +13,16 @@ const ConnectionSidebarStub = { draggingConnection: null, }; }, - computed: { - isCloud() { - return this.$store.state.workspaceId > 0; - }, - filteredConnections() { - return this.$store.getters["data/connections/filteredConnections"]; - }, - }, methods: { async onConnectionFolderHeaderDrop(folder) { if (!this.draggingConnection) return; try { - if (this.isCloud) { - await this.$store.dispatch("data/connections/save", { - ...this.draggingConnection, - connectionFolderId: folder.id, - position: { before: null }, - }); - } else { - const folderItems = this.filteredConnections.filter( - (c) => c.connectionFolderId === folder.id - ); - const newList = [ - this.draggingConnection, - ...folderItems.filter((c) => c.id !== this.draggingConnection.id), - ]; - await this.$store.dispatch( - "data/connections/saveMany", - newList.map((item, idx) => ({ - ...item, - connectionFolderId: folder.id, - position: idx + 1, - })) - ); - } + // Unified reorder action for both local and cloud + await this.$store.dispatch("data/connections/reorder", { + item: this.draggingConnection, + connectionFolderId: folder.id, + position: { before: null }, + }); } catch (ex) { this.$noty.error(`Move error: ${ex.message}`); } @@ -61,41 +37,16 @@ const FavoriteListStub = { draggingQuery: null, }; }, - computed: { - isCloud() { - return this.$store.state.workspaceId > 0; - }, - filteredQueries() { - return this.$store.getters["data/queries/filteredQueries"]; - }, - }, methods: { async onQueryFolderHeaderDrop(folder) { if (!this.draggingQuery) return; try { - if (this.isCloud) { - await this.$store.dispatch("data/queries/save", { - ...this.draggingQuery, - queryFolderId: folder.id, - position: { before: null }, - }); - } else { - const folderItems = this.filteredQueries.filter( - (q) => q.queryFolderId === folder.id - ); - const newList = [ - this.draggingQuery, - ...folderItems.filter((q) => q.id !== this.draggingQuery.id), - ]; - await this.$store.dispatch( - "data/queries/saveMany", - newList.map((item, idx) => ({ - ...item, - queryFolderId: folder.id, - position: idx + 1, - })) - ); - } + // Unified reorder action for both local and cloud + await this.$store.dispatch("data/queries/reorder", { + item: this.draggingQuery, + queryFolderId: folder.id, + position: { before: null }, + }); } catch (ex) { this.$noty.error(`Move error: ${ex.message}`); } @@ -109,18 +60,14 @@ describe("Folder Header Drop - Connection Sidebar", () => { let mockDispatch; let mockNoty; - function createStore(isCloud: boolean, connections: any[] = []) { + function createStore() { mockDispatch = jest.fn().mockResolvedValue({}); return new Vuex.Store({ state: { - workspaceId: isCloud ? 1 : -1, - }, - getters: { - "data/connections/filteredConnections": () => connections, + workspaceId: 1, }, actions: { - "data/connections/save": mockDispatch, - "data/connections/saveMany": mockDispatch, + "data/connections/reorder": mockDispatch, }, }); } @@ -131,7 +78,7 @@ describe("Folder Header Drop - Connection Sidebar", () => { describe("onConnectionFolderHeaderDrop", () => { it("does nothing when draggingConnection is null", async () => { - store = createStore(false); + store = createStore(); wrapper = shallowMount(ConnectionSidebarStub, { store, mocks: { $noty: mockNoty }, @@ -142,8 +89,8 @@ describe("Folder Header Drop - Connection Sidebar", () => { expect(mockDispatch).not.toHaveBeenCalled(); }); - it("cloud workspace: dispatches save with object-based position", async () => { - store = createStore(true); + it("dispatches reorder with object-based position", async () => { + store = createStore(); wrapper = shallowMount(ConnectionSidebarStub, { store, mocks: { $noty: mockNoty }, @@ -157,20 +104,16 @@ describe("Folder Header Drop - Connection Sidebar", () => { expect(mockDispatch).toHaveBeenCalledWith( expect.anything(), { - id: 10, - name: "My DB", + item: connection, connectionFolderId: 5, position: { before: null }, } ); }); - it("local workspace: dispatches saveMany with numeric positions", async () => { - const existingConnections = [ - { id: 20, name: "Existing1", connectionFolderId: 5 }, - { id: 21, name: "Existing2", connectionFolderId: 5 }, - ]; - store = createStore(false, existingConnections); + it("uses unified reorder action for both local and cloud workspaces", async () => { + // Both local (-1) and cloud (1) workspaces now use the same reorder action + store = createStore(); wrapper = shallowMount(ConnectionSidebarStub, { store, mocks: { $noty: mockNoty }, @@ -181,57 +124,14 @@ describe("Folder Header Drop - Connection Sidebar", () => { await wrapper.vm.onConnectionFolderHeaderDrop({ id: 5, name: "Work" }); + // Always uses reorder action with relative position expect(mockDispatch).toHaveBeenCalledWith( expect.anything(), - [ - { id: 10, name: "My DB", connectionFolderId: 5, position: 1 }, - { id: 20, name: "Existing1", connectionFolderId: 5, position: 2 }, - { id: 21, name: "Existing2", connectionFolderId: 5, position: 3 }, - ] - ); - }); - - it("local workspace: prepends dragged item and excludes duplicate", async () => { - const existingConnections = [ - { id: 10, name: "My DB", connectionFolderId: 5 }, - { id: 20, name: "Other", connectionFolderId: 5 }, - ]; - store = createStore(false, existingConnections); - wrapper = shallowMount(ConnectionSidebarStub, { - store, - mocks: { $noty: mockNoty }, - }); - - // Dragging an item that's already in the folder (reordering to top) - const connection = { id: 10, name: "My DB", connectionFolderId: 5 }; - wrapper.setData({ draggingConnection: connection }); - - await wrapper.vm.onConnectionFolderHeaderDrop({ id: 5, name: "Work" }); - - expect(mockDispatch).toHaveBeenCalledWith( - expect.anything(), - [ - { id: 10, name: "My DB", connectionFolderId: 5, position: 1 }, - { id: 20, name: "Other", connectionFolderId: 5, position: 2 }, - ] - ); - }); - - it("local workspace: handles empty folder", async () => { - store = createStore(false, []); - wrapper = shallowMount(ConnectionSidebarStub, { - store, - mocks: { $noty: mockNoty }, - }); - - const connection = { id: 10, name: "My DB", connectionFolderId: null }; - wrapper.setData({ draggingConnection: connection }); - - await wrapper.vm.onConnectionFolderHeaderDrop({ id: 5, name: "Work" }); - - expect(mockDispatch).toHaveBeenCalledWith( - expect.anything(), - [{ id: 10, name: "My DB", connectionFolderId: 5, position: 1 }] + { + item: connection, + connectionFolderId: 5, + position: { before: null }, + } ); }); }); @@ -243,18 +143,14 @@ describe("Folder Header Drop - Favorite List", () => { let mockDispatch; let mockNoty; - function createStore(isCloud: boolean, queries: any[] = []) { + function createStore() { mockDispatch = jest.fn().mockResolvedValue({}); return new Vuex.Store({ state: { - workspaceId: isCloud ? 1 : -1, - }, - getters: { - "data/queries/filteredQueries": () => queries, + workspaceId: 1, }, actions: { - "data/queries/save": mockDispatch, - "data/queries/saveMany": mockDispatch, + "data/queries/reorder": mockDispatch, }, }); } @@ -265,7 +161,7 @@ describe("Folder Header Drop - Favorite List", () => { describe("onQueryFolderHeaderDrop", () => { it("does nothing when draggingQuery is null", async () => { - store = createStore(false); + store = createStore(); wrapper = shallowMount(FavoriteListStub, { store, mocks: { $noty: mockNoty }, @@ -276,8 +172,8 @@ describe("Folder Header Drop - Favorite List", () => { expect(mockDispatch).not.toHaveBeenCalled(); }); - it("cloud workspace: dispatches save with object-based position", async () => { - store = createStore(true); + it("dispatches reorder with object-based position", async () => { + store = createStore(); wrapper = shallowMount(FavoriteListStub, { store, mocks: { $noty: mockNoty }, @@ -291,20 +187,16 @@ describe("Folder Header Drop - Favorite List", () => { expect(mockDispatch).toHaveBeenCalledWith( expect.anything(), { - id: 10, - title: "My Query", + item: query, queryFolderId: 5, position: { before: null }, } ); }); - it("local workspace: dispatches saveMany with numeric positions", async () => { - const existingQueries = [ - { id: 20, title: "Existing1", queryFolderId: 5 }, - { id: 21, title: "Existing2", queryFolderId: 5 }, - ]; - store = createStore(false, existingQueries); + it("uses unified reorder action for both local and cloud workspaces", async () => { + // Both local (-1) and cloud (1) workspaces now use the same reorder action + store = createStore(); wrapper = shallowMount(FavoriteListStub, { store, mocks: { $noty: mockNoty }, @@ -315,56 +207,14 @@ describe("Folder Header Drop - Favorite List", () => { await wrapper.vm.onQueryFolderHeaderDrop({ id: 5, name: "Work" }); + // Always uses reorder action with relative position expect(mockDispatch).toHaveBeenCalledWith( expect.anything(), - [ - { id: 10, title: "My Query", queryFolderId: 5, position: 1 }, - { id: 20, title: "Existing1", queryFolderId: 5, position: 2 }, - { id: 21, title: "Existing2", queryFolderId: 5, position: 3 }, - ] - ); - }); - - it("local workspace: prepends dragged item and excludes duplicate", async () => { - const existingQueries = [ - { id: 10, title: "My Query", queryFolderId: 5 }, - { id: 20, title: "Other", queryFolderId: 5 }, - ]; - store = createStore(false, existingQueries); - wrapper = shallowMount(FavoriteListStub, { - store, - mocks: { $noty: mockNoty }, - }); - - const query = { id: 10, title: "My Query", queryFolderId: 5 }; - wrapper.setData({ draggingQuery: query }); - - await wrapper.vm.onQueryFolderHeaderDrop({ id: 5, name: "Work" }); - - expect(mockDispatch).toHaveBeenCalledWith( - expect.anything(), - [ - { id: 10, title: "My Query", queryFolderId: 5, position: 1 }, - { id: 20, title: "Other", queryFolderId: 5, position: 2 }, - ] - ); - }); - - it("local workspace: handles empty folder", async () => { - store = createStore(false, []); - wrapper = shallowMount(FavoriteListStub, { - store, - mocks: { $noty: mockNoty }, - }); - - const query = { id: 10, title: "My Query", queryFolderId: null }; - wrapper.setData({ draggingQuery: query }); - - await wrapper.vm.onQueryFolderHeaderDrop({ id: 5, name: "Work" }); - - expect(mockDispatch).toHaveBeenCalledWith( - expect.anything(), - [{ id: 10, title: "My Query", queryFolderId: 5, position: 1 }] + { + item: query, + queryFolderId: 5, + position: { before: null }, + } ); }); }); diff --git a/apps/studio/tests/unit/store/CloudConnectionReorder.spec.ts b/apps/studio/tests/unit/store/CloudConnectionReorder.spec.ts new file mode 100644 index 000000000..e8007cb49 --- /dev/null +++ b/apps/studio/tests/unit/store/CloudConnectionReorder.spec.ts @@ -0,0 +1,919 @@ +import Vuex, { Store } from "vuex"; +import Vue from "vue"; +import _ from "lodash"; +import { CloudConnectionModule } from "@/store/modules/data/connection/CloudConnectionModule"; +import { ICloudSavedConnection } from "@/common/interfaces/IConnection"; + +Vue.use(Vuex); + +type TestConnection = ICloudSavedConnection; + +interface MockCloudClient { + connections: { + reorder: jest.Mock; + list: jest.Mock; + }; +} + +function createMockConnection( + id: number, + name: string, + position: number, + connectionFolderId: number | null = null +): TestConnection { + return { + id, + name, + position, + connectionFolderId, + workspaceId: 1, + connectionType: "postgresql", + defaultDatabase: "test", + host: "localhost", + port: 5432, + createdAt: new Date(), + updatedAt: new Date(), + } as TestConnection; +} + +function createMockCloudClient(latencyMs: number = 0, siblingPositions?: { id: number; position: number }[]): MockCloudClient { + return { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + if (latencyMs > 0) { + await new Promise((r) => setTimeout(r, latencyMs)); + } + // Return all sibling positions (simulating server response) + // If siblingPositions provided, use those, otherwise return sensible defaults + if (siblingPositions) { + return siblingPositions.map(s => ({ + id: s.id, + position: s.position, + updatedAt: Date.now(), + })); + } + // Default: return just the moved item with position 1 + return [{ + id, + position: 1, + updatedAt: Date.now(), + }]; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; +} + +function createStore( + mockClient: MockCloudClient, + initialConnections: TestConnection[] = [] +): Store { + // Deep clone the module to avoid shared state between tests + const moduleClone = _.cloneDeep(CloudConnectionModule); + + return new Vuex.Store({ + state: { + workspaceId: 1, + }, + getters: { + cloudClient: () => mockClient, + }, + modules: { + data: { + namespaced: true, + modules: { + connections: { + ...moduleClone, + state: { + items: initialConnections, + loading: false, + error: null, + pollError: null, + filter: undefined, + pendingSaveIds: [], + }, + }, + }, + }, + }, + }); +} + +function getConnectionsInOrder(store: Store): TestConnection[] { + const items = store.state.data.connections.items as TestConnection[]; + return _.sortBy(items, "position"); +} + +function getConnectionById( + store: Store, + id: number +): TestConnection | undefined { + return (store.state.data.connections.items as TestConnection[]).find( + (c) => c.id === id + ); +} + +describe("CloudConnectionModule reorder", () => { + let store: Store; + let mockClient: MockCloudClient; + + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.clearAllMocks(); + }); + + describe("API response scenarios (potential snap-back causes)", () => { + it("reorder API returns all siblings with correct positions", async () => { + // The reorder API now returns ALL siblings with updated positions + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + ]; + + mockClient = { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + await new Promise(r => setTimeout(r, 50)); + // Server returns ALL siblings with correct positions + return [ + { id: 2, position: 1, updatedAt: Date.now() }, + { id: 1, position: 2, updatedAt: Date.now() }, + ]; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 2 }, + position: { before: 1 }, // Move to first + connectionFolderId: null, + }); + + // Optimistic update should put item 2 first + let item2 = getConnectionById(store, 2); + const optimisticPos = item2!.position; + expect(optimisticPos).toBeLessThan(1); + console.log("Optimistic position:", optimisticPos); + + // API returns + jest.advanceTimersByTime(100); + await reorderPromise; + + // After API, all positions should be updated from server response + item2 = getConnectionById(store, 2); + const item1 = getConnectionById(store, 1); + console.log("After API - item2 position:", item2!.position); + console.log("After API - item1 position:", item1!.position); + + // Positions should match server response + expect(item2!.position).toBe(1); + expect(item1!.position).toBe(2); + }); + + it("reorder API prevents position collisions by returning all siblings", async () => { + // The reorder API returns ALL siblings, preventing collisions + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + + mockClient = { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + await new Promise(r => setTimeout(r, 50)); + // Server returns ALL siblings with renumbered positions - no collisions + return [ + { id: 3, position: 1, updatedAt: Date.now() }, + { id: 1, position: 2, updatedAt: Date.now() }, + { id: 2, position: 3, updatedAt: Date.now() }, + ]; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + jest.advanceTimersByTime(100); + await reorderPromise; + + // All items should have unique positions from server + const item1 = getConnectionById(store, 1); + const item2 = getConnectionById(store, 2); + const item3 = getConnectionById(store, 3); + console.log("Item 1 position:", item1!.position); + console.log("Item 2 position:", item2!.position); + console.log("Item 3 position:", item3!.position); + + expect(item3!.position).toBe(1); + expect(item1!.position).toBe(2); + expect(item2!.position).toBe(3); + + // No collision - all positions are unique + const positions = [item1!.position, item2!.position, item3!.position]; + const uniquePositions = new Set(positions); + expect(uniquePositions.size).toBe(3); + + // Sort order should be stable + const sorted = getConnectionsInOrder(store); + console.log("Sorted order:", sorted.map(c => ({ id: c.id, pos: c.position }))); + expect(sorted[0].id).toBe(3); + expect(sorted[1].id).toBe(1); + expect(sorted[2].id).toBe(2); + }); + + it("reorder API always returns position for all siblings", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + ]; + + mockClient = { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + await new Promise(r => setTimeout(r, 50)); + // Server always returns position for all siblings + return [ + { id: 2, position: 1, updatedAt: Date.now() }, + { id: 1, position: 2, updatedAt: Date.now() }, + ]; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 2 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Optimistic position + let item2 = getConnectionById(store, 2); + const optimisticPos = item2!.position; + console.log("Optimistic position:", optimisticPos); + + jest.advanceTimersByTime(100); + await reorderPromise; + + // After API, positions should be from server + item2 = getConnectionById(store, 2); + const item1 = getConnectionById(store, 1); + console.log("After API - item2:", item2!.position); + console.log("After API - item1:", item1!.position); + + expect(item2!.position).toBe(1); + expect(item1!.position).toBe(2); + }); + + it("simulates drag from position 2 to position 1 - exact user scenario", async () => { + // Recreate the exact scenario: drag item from pos 2 to pos 1 + const connections = [ + createMockConnection(1, "ConnectionA", 1), + createMockConnection(2, "ConnectionB", 2), + createMockConnection(3, "ConnectionC", 3), + ]; + + // Server returns ALL siblings with their new positions (the fix!) + mockClient = { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + console.log("API reorder call:", { id, position, folderId }); + await new Promise(r => setTimeout(r, 100)); + + // Server renumbers ALL siblings - no collisions + const response = [ + { id: 2, position: 1, updatedAt: Date.now() }, // Moved item + { id: 1, position: 2, updatedAt: Date.now() }, // Shifted down + { id: 3, position: 3, updatedAt: Date.now() }, // Unchanged + ]; + console.log("API response (all siblings):", response); + return response; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; + store = createStore(mockClient, connections); + + console.log("\n=== Starting drag from position 2 to position 1 ==="); + console.log("Initial state:", getConnectionsInOrder(store).map(c => ({ id: c.id, name: c.name, pos: c.position }))); + + // User drags ConnectionB (pos 2) to before ConnectionA (pos 1) + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 2 }, // ConnectionB + position: { before: 1 }, // Before ConnectionA + connectionFolderId: null, + }); + + // Check optimistic state immediately + const optimisticOrder = getConnectionsInOrder(store); + console.log("After optimistic update:", optimisticOrder.map(c => ({ id: c.id, name: c.name, pos: c.position }))); + + // ConnectionB should be first (position 0.5) + expect(optimisticOrder[0].id).toBe(2); + expect(optimisticOrder[0].position).toBeLessThan(1); + + // Wait for API + jest.advanceTimersByTime(150); + await reorderPromise; + + const finalOrder = getConnectionsInOrder(store); + console.log("After API response:", finalOrder.map(c => ({ id: c.id, name: c.name, pos: c.position }))); + + // FIX VERIFICATION: All items have correct, non-colliding positions + expect(finalOrder[0].id).toBe(2); // ConnectionB is first + expect(finalOrder[0].position).toBe(1); + expect(finalOrder[1].id).toBe(1); // ConnectionA is second + expect(finalOrder[1].position).toBe(2); + expect(finalOrder[2].id).toBe(3); // ConnectionC is third + expect(finalOrder[2].position).toBe(3); + + // No position collisions + const positions = finalOrder.map(c => c.position); + const uniquePositions = new Set(positions); + expect(uniquePositions.size).toBe(positions.length); + }); + }); + + describe("optimistic update behavior", () => { + it("immediately updates position in store before API response", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + // Start the reorder (move "Third" before "First") + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Immediately after dispatch (before API returns), check the store + // The position should be optimistically updated + const thirdConn = getConnectionById(store, 3); + expect(thirdConn).toBeDefined(); + // Optimistic position should be calculated (minPos - 1 or similar) + expect(thirdConn!.position).toBeLessThan(1); + + // Fast-forward past the API latency + jest.advanceTimersByTime(150); + await reorderPromise; + }); + + it("maintains position with 0ms latency", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(0); + store = createStore(mockClient, connections); + + await store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { after: 1 }, + connectionFolderId: null, + }); + + // API should have been called + expect(mockClient.connections.reorder).toHaveBeenCalledWith( + 3, + { after: 1 }, + null + ); + + // Store should have the updated position + const thirdConn = getConnectionById(store, 3); + expect(thirdConn).toBeDefined(); + expect(thirdConn!.position).toBeDefined(); + }); + + it("maintains position with 100ms latency", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { after: 1 }, + connectionFolderId: null, + }); + + // Check optimistic update happened + const beforeApiConn = getConnectionById(store, 3); + expect(beforeApiConn!.position).toBeCloseTo(1.5, 1); // after item with position 1 + + jest.advanceTimersByTime(150); + await reorderPromise; + + // After API response, position should be server's value + const afterApiConn = getConnectionById(store, 3); + expect(afterApiConn!.position).toBe(1); // Server returned position: 1 + }); + + it("maintains position with 500ms latency", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(500); + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { after: 2 }, + connectionFolderId: null, + }); + + // Optimistic update should have the item between positions 2 and 3 + const midwayConn = getConnectionById(store, 3); + expect(midwayConn!.position).toBeCloseTo(2.5, 1); + + jest.advanceTimersByTime(600); + await reorderPromise; + + const finalConn = getConnectionById(store, 3); + expect(finalConn).toBeDefined(); + }); + }); + + describe("poll during save scenarios", () => { + it("maintains position when poll runs during save", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(200); + + // Set up poll to return old positions + mockClient.connections.list.mockResolvedValue([ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), // Old position! + ]); + + store = createStore(mockClient, connections); + + // Start reorder + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Optimistic update should place item 3 first + const optimisticConn = getConnectionById(store, 3); + expect(optimisticConn!.position).toBeLessThan(1); + + // Simulate poll running mid-save (at 100ms, before API returns at 200ms) + jest.advanceTimersByTime(100); + await store.dispatch("data/connections/poll"); + + // After poll, check if position was reverted (THIS IS THE BUG) + const afterPollConn = getConnectionById(store, 3); + + // Record what happens - this test documents the current behavior + // If afterPollConn.position === 3, the poll overwrote the optimistic update (BUG) + // If afterPollConn.position < 1, the optimistic update was preserved (CORRECT) + console.log( + "Position after poll during save:", + afterPollConn!.position + ); + + // EXPECTED: Optimistic position should be preserved during pending save + // This will FAIL until the bug is fixed + expect(afterPollConn!.position).toBeLessThan(1); + + // Complete the API call + jest.advanceTimersByTime(150); + await reorderPromise; + + const finalConn = getConnectionById(store, 3); + console.log("Final position after API response:", finalConn!.position); + }); + + it("should preserve optimistic position when poll runs during save", async () => { + // This test will FAIL until the bug is fixed + // It asserts the CORRECT expected behavior + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(200); + + // Poll returns stale data (server hasn't processed the reorder yet) + mockClient.connections.list.mockResolvedValue([ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]); + + store = createStore(mockClient, connections); + + // User drags item 3 to first position + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Verify optimistic update worked + let item3 = getConnectionById(store, 3); + const optimisticPosition = item3!.position; + expect(optimisticPosition).toBeLessThan(1); // User sees item at top + + // Poll runs during the save (this happens in production with 5s interval) + jest.advanceTimersByTime(50); + await store.dispatch("data/connections/poll"); + + // EXPECTED: Poll should NOT overwrite the optimistic position + // The item should remain at the optimistic position until API completes + item3 = getConnectionById(store, 3); + expect(item3!.position).toBeLessThan(1); // Should still be at top, not snapped back + + // Complete the API call + jest.advanceTimersByTime(200); + await reorderPromise; + + // Final position from server + item3 = getConnectionById(store, 3); + expect(item3!.position).toBe(1); + }); + + it("handles poll returning updated positions during save", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + ]; + mockClient = createMockCloudClient(200); + + store = createStore(mockClient, connections); + + // Start reorder + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { after: 1 }, + connectionFolderId: null, + }); + + // Wait a bit, then update poll to return new positions + jest.advanceTimersByTime(50); + + // Poll now returns the NEW positions (as if server already processed) + mockClient.connections.list.mockResolvedValue([ + createMockConnection(1, "First", 1), + createMockConnection(3, "Third", 2), // New position + createMockConnection(2, "Second", 3), + ]); + + // Run poll + await store.dispatch("data/connections/poll"); + + // Complete the API call + jest.advanceTimersByTime(200); + await reorderPromise; + + const finalOrder = getConnectionsInOrder(store); + console.log( + "Final order:", + finalOrder.map((c) => ({ id: c.id, pos: c.position })) + ); + }); + }); + + describe("rapid reorder scenarios", () => { + it("handles multiple rapid reorders", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + createMockConnection(3, "Third", 3), + createMockConnection(4, "Fourth", 4), + ]; + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + // User drags multiple times quickly + const reorder1 = store.dispatch("data/connections/reorder", { + item: { id: 4 }, + position: { after: 1 }, + connectionFolderId: null, + }); + + jest.advanceTimersByTime(20); + + const reorder2 = store.dispatch("data/connections/reorder", { + item: { id: 4 }, + position: { after: 2 }, + connectionFolderId: null, + }); + + jest.advanceTimersByTime(20); + + const reorder3 = store.dispatch("data/connections/reorder", { + item: { id: 4 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Check state during rapid updates + const midConn = getConnectionById(store, 4); + console.log("Position during rapid reorders:", midConn!.position); + + // Complete all API calls + jest.advanceTimersByTime(200); + await Promise.all([reorder1, reorder2, reorder3]); + + // Check how many times API was called + expect(mockClient.connections.reorder).toHaveBeenCalledTimes(3); + + const finalConn = getConnectionById(store, 4); + console.log("Final position after rapid reorders:", finalConn!.position); + }); + + it("maintains correct order after competing updates", async () => { + const connections = [ + createMockConnection(1, "A", 1), + createMockConnection(2, "B", 2), + createMockConnection(3, "C", 3), + ]; + + // API returns with delays that arrive out of order + let callCount = 0; + mockClient = { + connections: { + reorder: jest.fn().mockImplementation(async (id, position, folderId) => { + callCount++; + const thisCall = callCount; + // First call returns last, second returns first + const delay = thisCall === 1 ? 200 : 50; + await new Promise((r) => setTimeout(r, delay)); + return [{ + id, + position: thisCall, + updatedAt: Date.now(), + }]; + }), + list: jest.fn().mockResolvedValue([]), + }, + }; + store = createStore(mockClient, connections); + + // Two reorders - API responses will arrive out of order + const reorder1 = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + jest.advanceTimersByTime(10); + + const reorder2 = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { after: 2 }, + connectionFolderId: null, + }); + + // Second API call returns first (at 60ms) + jest.advanceTimersByTime(60); + await Promise.resolve(); // Let second API resolve + + const afterSecondApi = getConnectionById(store, 3); + console.log( + "Position after second API (arrived first):", + afterSecondApi!.position + ); + + // First API call returns (at 210ms total) + jest.advanceTimersByTime(200); + await Promise.all([reorder1, reorder2]); + + const afterFirstApi = getConnectionById(store, 3); + console.log( + "Position after first API (arrived last):", + afterFirstApi!.position + ); + + // The position might be wrong if we don't handle out-of-order responses + }); + }); + + describe("folder move with reorder", () => { + it("moves item to new folder at first position", async () => { + const connections = [ + createMockConnection(1, "FolderAItem1", 1, 100), + createMockConnection(2, "FolderAItem2", 2, 100), + createMockConnection(3, "FolderBItem1", 1, 200), + ]; + // Use latency to capture the optimistic state before API response + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 1 }, + position: { before: null }, + connectionFolderId: 200, // Move to folder B + }); + + // Check optimistic update + const optimisticConn = getConnectionById(store, 1); + expect(optimisticConn!.connectionFolderId).toBe(200); + // Should be before existing item in folder B (position 1) + expect(optimisticConn!.position).toBeLessThanOrEqual(0); + + // Complete API + jest.advanceTimersByTime(150); + await reorderPromise; + + // After API, position is server-returned value + const finalConn = getConnectionById(store, 1); + expect(finalConn!.connectionFolderId).toBe(200); + }); + + it("moves item to new folder after specific item", async () => { + const connections = [ + createMockConnection(1, "FolderAItem1", 1, 100), + createMockConnection(2, "FolderBItem1", 1, 200), + createMockConnection(3, "FolderBItem2", 2, 200), + ]; + // Use latency to capture the optimistic state before API response + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 1 }, + position: { after: 2 }, // After FolderBItem1 + connectionFolderId: 200, + }); + + // Check optimistic update + const optimisticConn = getConnectionById(store, 1); + expect(optimisticConn!.connectionFolderId).toBe(200); + // Should be after position 1 (FolderBItem1's position) + expect(optimisticConn!.position).toBeCloseTo(1.5, 1); + + // Complete API + jest.advanceTimersByTime(150); + await reorderPromise; + }); + }); + + describe("edge cases", () => { + it("handles reorder of non-existent item gracefully", async () => { + const connections = [createMockConnection(1, "Only", 1)]; + mockClient = createMockCloudClient(0); + store = createStore(mockClient, connections); + + const result = await store.dispatch("data/connections/reorder", { + item: { id: 999 }, // Does not exist + position: { before: 1 }, + connectionFolderId: null, + }); + + expect(result).toBeUndefined(); + expect(mockClient.connections.reorder).not.toHaveBeenCalled(); + }); + + it("handles empty folder positioning", async () => { + const connections = [createMockConnection(1, "Item", 1, null)]; + mockClient = createMockCloudClient(0); + store = createStore(mockClient, connections); + + await store.dispatch("data/connections/reorder", { + item: { id: 1 }, + position: { before: null }, // First in empty folder + connectionFolderId: 999, // New folder with no items + }); + + const conn = getConnectionById(store, 1); + expect(conn!.connectionFolderId).toBe(999); + // With no siblings, position calculation should handle gracefully + expect(typeof conn!.position).toBe("number"); + }); + + it("handles position object with null before (first position)", async () => { + const connections = [ + createMockConnection(1, "First", 1), + createMockConnection(2, "Second", 2), + ]; + // Use latency to capture the optimistic state before API response + mockClient = createMockCloudClient(100); + store = createStore(mockClient, connections); + + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 2 }, + position: { before: null }, // Move to first + connectionFolderId: null, + }); + + // Check optimistic update + const optimisticConn = getConnectionById(store, 2); + // Should calculate position before the minimum existing position + expect(optimisticConn!.position).toBeLessThan(1); + + // Complete API + jest.advanceTimersByTime(150); + await reorderPromise; + }); + }); + + describe("state consistency checks", () => { + it("verifies upsert mutation preserves other item properties", async () => { + const connections = [ + createMockConnection(1, "Test", 1), + ]; + mockClient = createMockCloudClient(0); + store = createStore(mockClient, connections); + + const originalConn = getConnectionById(store, 1); + const originalHost = originalConn!.host; + const originalPort = originalConn!.port; + + await store.dispatch("data/connections/reorder", { + item: { id: 1 }, + position: { before: null }, + connectionFolderId: 100, + }); + + const updatedConn = getConnectionById(store, 1); + expect(updatedConn!.host).toBe(originalHost); + expect(updatedConn!.port).toBe(originalPort); + expect(updatedConn!.connectionFolderId).toBe(100); + }); + + it("tracks position values through full reorder cycle", async () => { + const connections = [ + createMockConnection(1, "A", 1), + createMockConnection(2, "B", 2), + createMockConnection(3, "C", 3), + ]; + mockClient = createMockCloudClient(50); + store = createStore(mockClient, connections); + + const positions: { stage: string; id: number; position: number }[] = []; + + // Record initial + connections.forEach((c) => { + positions.push({ stage: "initial", id: c.id, position: c.position }); + }); + + // Start reorder + const reorderPromise = store.dispatch("data/connections/reorder", { + item: { id: 3 }, + position: { before: 1 }, + connectionFolderId: null, + }); + + // Record after optimistic + const afterOptimistic = getConnectionById(store, 3); + positions.push({ + stage: "optimistic", + id: 3, + position: afterOptimistic!.position, + }); + + // Complete API + jest.advanceTimersByTime(100); + await reorderPromise; + + // Record final + const afterApi = getConnectionById(store, 3); + positions.push({ + stage: "final", + id: 3, + position: afterApi!.position, + }); + + console.log("Position tracking:", positions); + + // Verify the flow + expect(positions[3].stage).toBe("optimistic"); + expect(positions[3].position).toBeLessThan(1); // Moved before first + expect(positions[4].stage).toBe("final"); + }); + }); +}); diff --git a/apps/studio/tests/unit/store/LocalConnectionReorder.spec.ts b/apps/studio/tests/unit/store/LocalConnectionReorder.spec.ts new file mode 100644 index 000000000..12a372de3 --- /dev/null +++ b/apps/studio/tests/unit/store/LocalConnectionReorder.spec.ts @@ -0,0 +1,172 @@ +import Vuex from 'vuex' +import Vue from 'vue' +import { UtilConnectionModule } from '@/store/modules/data/connection/UtilityConnectionModule' + +Vue.use(Vuex) + +// Mock Vue.prototype.$util.send +const mockSend = jest.fn().mockImplementation(async (channel, { obj }) => { + // Return the object as-is (simulating successful save) + return obj +}) + +Vue.prototype.$util = { + send: mockSend +} + +describe('UtilConnectionModule reorder', () => { + let store: any + + beforeEach(() => { + mockSend.mockClear() + + store = new Vuex.Store({ + modules: { + connections: { + ...UtilConnectionModule, + namespaced: true + } + } + }) + }) + + describe('position sorting bug fix', () => { + it('correctly reorders when state items are not sorted by position', async () => { + // This test reproduces the bug where moving from position 5 to position 2 + // caused the item at position 3 to jump to position 5 instead of position 4. + // + // The bug occurred because siblings were filtered from state without sorting, + // and state order doesn't guarantee position order. + + // Set up items in state in a DIFFERENT order than their positions + // State order: [id:3, id:1, id:5, id:2, id:4] (arbitrary) + // Position order: id:1=1, id:2=2, id:3=3, id:4=4, id:5=5 + const items = [ + { id: 3, name: 'C3', position: 3, connectionFolderId: 1 }, + { id: 1, name: 'C1', position: 1, connectionFolderId: 1 }, + { id: 5, name: 'C5', position: 5, connectionFolderId: 1 }, + { id: 2, name: 'C2', position: 2, connectionFolderId: 1 }, + { id: 4, name: 'C4', position: 4, connectionFolderId: 1 }, + ] + + store.commit('connections/replace', items) + + // Move item at position 5 (id:5) to position 2 (before id:2) + const itemToMove = items.find(i => i.id === 5)! + await store.dispatch('connections/reorder', { + item: itemToMove, + position: { before: 2 }, // Insert before item with id:2 + connectionFolderId: 1 + }) + + // Get the final state sorted by position + const finalItems = [...store.state.connections.items].sort( + (a, b) => a.position - b.position + ) + + // Expected order: C1(1), C5(2), C2(3), C3(4), C4(5) + expect(finalItems.map(i => ({ id: i.id, position: i.position }))).toEqual([ + { id: 1, position: 1 }, + { id: 5, position: 2 }, + { id: 2, position: 3 }, + { id: 3, position: 4 }, + { id: 4, position: 5 }, + ]) + }) + + it('correctly reorders to first position when state is unsorted', async () => { + // Items in state in random order + const items = [ + { id: 2, name: 'C2', position: 2, connectionFolderId: 1 }, + { id: 3, name: 'C3', position: 3, connectionFolderId: 1 }, + { id: 1, name: 'C1', position: 1, connectionFolderId: 1 }, + ] + + store.commit('connections/replace', items) + + // Move C3 to first position + const itemToMove = items.find(i => i.id === 3)! + await store.dispatch('connections/reorder', { + item: itemToMove, + position: { before: null }, // First position + connectionFolderId: 1 + }) + + const finalItems = [...store.state.connections.items].sort( + (a, b) => a.position - b.position + ) + + // Expected: C3(1), C1(2), C2(3) + expect(finalItems.map(i => ({ id: i.id, position: i.position }))).toEqual([ + { id: 3, position: 1 }, + { id: 1, position: 2 }, + { id: 2, position: 3 }, + ]) + }) + + it('correctly reorders after an item when state is unsorted', async () => { + // Items in state in random order + const items = [ + { id: 3, name: 'C3', position: 3, connectionFolderId: 1 }, + { id: 1, name: 'C1', position: 1, connectionFolderId: 1 }, + { id: 2, name: 'C2', position: 2, connectionFolderId: 1 }, + ] + + store.commit('connections/replace', items) + + // Move C3 after C1 (should become position 2) + const itemToMove = items.find(i => i.id === 3)! + await store.dispatch('connections/reorder', { + item: itemToMove, + position: { after: 1 }, // After item with id:1 + connectionFolderId: 1 + }) + + const finalItems = [...store.state.connections.items].sort( + (a, b) => a.position - b.position + ) + + // Expected: C1(1), C3(2), C2(3) + expect(finalItems.map(i => ({ id: i.id, position: i.position }))).toEqual([ + { id: 1, position: 1 }, + { id: 3, position: 2 }, + { id: 2, position: 3 }, + ]) + }) + + it('maintains correct positions when moving within unsorted state', async () => { + // Simulate real-world scenario: items loaded in arbitrary order + const items = [ + { id: 4, name: 'D', position: 4, connectionFolderId: null }, + { id: 1, name: 'A', position: 1, connectionFolderId: null }, + { id: 3, name: 'C', position: 3, connectionFolderId: null }, + { id: 2, name: 'B', position: 2, connectionFolderId: null }, + ] + + store.commit('connections/replace', items) + + // Move D (position 4) to position 2 + await store.dispatch('connections/reorder', { + item: { id: 4, name: 'D', position: 4, connectionFolderId: null }, + position: { before: 2 }, + connectionFolderId: null + }) + + const finalItems = [...store.state.connections.items] + .filter(i => i.connectionFolderId === null) + .sort((a, b) => a.position - b.position) + + // All items should have sequential positions with no gaps + const positions = finalItems.map(i => i.position) + expect(positions).toEqual([1, 2, 3, 4]) + + // D should now be at position 2 + const itemD = finalItems.find(i => i.id === 4) + expect(itemD?.position).toBe(2) + + // Items that were after the insertion point should shift down + // A stays at 1, D is now 2, B shifts to 3, C shifts to 4 + expect(finalItems.map(i => i.id)).toEqual([1, 4, 2, 3]) + }) + }) +})