fix: source reorder (#8496)

packages/nc-gui/components/dashboard/settings/DataSources.vue

@@ -18,8 +18,6 @@ const vReload = useVModel(props, 'reload', emits)
 
 const { $api, $e } = useNuxtApp()
 
-const { t } = useI18n()
-
 const basesStore = useBases()
 const { loadProject } = basesStore
 const { isDataSourceLimitReached } = storeToRefs(basesStore)
@@ -42,6 +40,44 @@ const isReloading = ref(false)
 const isDeleteBaseModalOpen = ref(false)
 const toBeDeletedBase = ref<SourceType | undefined>()
 
+async function updateIfSourceOrderIsNullOrDuplicate() {
+  const sourceOrderSet = new Set()
+  let hasNullOrDuplicates = false
+
+  // Check if sources.value contains null or duplicate order
+  for (const source of sources.value) {
+    if (source.order === null || sourceOrderSet.has(source.order)) {
+      hasNullOrDuplicates = true
+      break
+    }
+    sourceOrderSet.add(source.order)
+  }
+
+  if (!hasNullOrDuplicates) return
+
+  // update the local state
+  sources.value = sources.value.map((source, i) => {
+    return {
+      ...source,
+      order: i + 1,
+    }
+  })
+
+  try {
+    await Promise.all(
+      sources.value.map(async (source) => {
+        await $api.source.update(source.base_id as string, source.id as string, {
+          id: source.id,
+          base_id: source.base_id,
+          order: source.order,
+        })
+      }),
+    )
+  } catch (e: any) {
+    message.error(await extractSdkResponseErrorMsg(e))
+  }
+}
+
 async function loadBases(changed?: boolean) {
   try {
     if (changed) refreshCommandPalette()
@@ -53,6 +89,7 @@ async function loadBases(changed?: boolean) {
     if (baseList.list && baseList.list.length) {
       sources.value = baseList.list
     }
+    await updateIfSourceOrderIsNullOrDuplicate()
   } catch (e) {
     console.error(e)
   } finally {
@@ -90,7 +127,7 @@ const deleteBase = async () => {
     refreshCommandPalette()
   }
 }
-const toggleBase = async (source: BaseType, state: boolean) => {
+const toggleBase = async (source: SourceType, state: boolean) => {
   try {
     if (!state && sources.value.filter((src) => src.enabled).length < 2) {
       message.info('There should be at least one enabled source!')
@@ -116,20 +153,26 @@ const moveBase = async (e: any) => {
     // sources list is mutated so we have to get the new index and mirror it to backend
     const source = sources.value[e.newIndex]
     if (source) {
-      if (!source.order) {
-        // empty update call to reorder sources (migration)
-        await $api.source.update(source.base_id as string, source.id as string, {
-          id: source.id,
-          base_id: source.base_id,
-        })
-        message.info(t('info.basesMigrated'))
+      let nextOrder: number
+
+      // set new order value based on the new order of the items
+      if (sources.value.length - 1 === e.newIndex) {
+        // If moving to the end, set nextOrder greater than the maximum order in the list
+        nextOrder = Math.max(...sources.value.map((item) => item?.order ?? 0)) + 1
       } else {
-        await $api.source.update(source.base_id as string, source.id as string, {
-          id: source.id,
-          base_id: source.base_id,
-          order: e.newIndex + 1,
-        })
+        nextOrder =
+          (parseFloat(String(sources.value[e.newIndex - 1]?.order ?? 0)) +
+            parseFloat(String(sources.value[e.newIndex + 1]?.order ?? 0))) /
+          2
       }
+
+      const _nextOrder = !isNaN(Number(nextOrder)) ? nextOrder : e.oldIndex
+
+      await $api.source.update(source.base_id as string, source.id as string, {
+        id: source.id,
+        base_id: source.base_id,
+        order: _nextOrder,
+      })
     }
     await loadProject(base.value.id as string, true)
     await loadBases()

packages/nocodb/src/models/Source.ts

@@ -76,6 +76,10 @@ export default class Source implements SourceType {
       insertObj.meta = stringifyMetaProp(insertObj);
     }
 
+    insertObj.order = await ncMeta.metaGetNextOrder(MetaTable.BASES, {
+      base_id: source.baseId,
+    });
+
     const { id } = await ncMeta.metaInsert2(
       source.baseId,
       null,
@@ -92,8 +96,6 @@ export default class Source implements SourceType {
       `${CacheScope.BASE}:${id}`,
     );
 
-    await this.reorderBases(source.baseId);
-
     return returnBase;
   }
 
@@ -101,7 +103,6 @@ export default class Source implements SourceType {
     sourceId: string,
     source: SourceType & {
       baseId: string;
-      skipReorder?: boolean;
       meta?: any;
       deleted?: boolean;
       fk_sql_executor_id?: string;
@@ -138,6 +139,36 @@ export default class Source implements SourceType {
       updateObj.type = oldBase.type;
     }
 
+    // if order is missing (possible in old versions), get next order
+    if (!oldBase.order && !updateObj.order) {
+      updateObj.order = await ncMeta.metaGetNextOrder(MetaTable.BASES, {
+        base_id: source.baseId,
+      });
+
+      if (updateObj.order <= 1 && !oldBase.isMeta()) {
+        updateObj.order = 2;
+      }
+    }
+
+    // keep order 1 for default source
+    if (oldBase.isMeta()) {
+      updateObj.order = 1;
+    }
+
+    // keep order 1 for default source
+    if (!oldBase.isMeta()) {
+      if (updateObj.order <= 1) {
+        NcError.badRequest('Cannot change order to 1 or less');
+      }
+
+      // if order is 1 for non-default source, move it to last
+      if (oldBase.order <= 1 && !updateObj.order) {
+        updateObj.order = await ncMeta.metaGetNextOrder(MetaTable.BASES, {
+          base_id: source.baseId,
+        });
+      }
+    }
+
     await ncMeta.metaUpdate(
       source.baseId,
       null,
@@ -154,10 +185,6 @@ export default class Source implements SourceType {
     // call before reorder to update cache
     const returnBase = await this.get(oldBase.id, false, ncMeta);
 
-    if (!source.skipReorder && source.order && source.order !== oldBase.order) {
-      await this.reorderBases(source.baseId, returnBase.id, ncMeta);
-    }
-
     return returnBase;
   }
 
@@ -288,41 +315,6 @@ export default class Source implements SourceType {
     return this.castType(source);
   }
 
-  static async reorderBases(
-    baseId: string,
-    keepBase?: string,
-    ncMeta = Noco.ncMeta,
-  ) {
-    const sources = await this.list({ baseId: baseId }, ncMeta);
-
-    if (keepBase) {
-      const kpBase = sources.splice(
-        sources.indexOf(sources.find((source) => source.id === keepBase)),
-        1,
-      );
-      if (kpBase.length) {
-        sources.splice(kpBase[0].order - 1, 0, kpBase[0]);
-      }
-    }
-
-    // update order for sources
-    for (const [i, b] of Object.entries(sources)) {
-      b.order = parseInt(i) + 1;
-
-      await ncMeta.metaUpdate(
-        b.base_id,
-        null,
-        MetaTable.BASES,
-        {
-          order: b.order,
-        },
-        b.id,
-      );
-
-      await NocoCache.set(`${CacheScope.BASE}:${b.id}`, b);
-    }
-  }
-
   public async getConnectionConfig(): Promise<any> {
     const config = this.getConfig();
 

packages/nocodb/src/version-upgrader/ncProjectConfigUpgrader.ts

@@ -37,7 +37,6 @@ export default async function ({ ncMeta }: NcUpgraderCtx) {
             id: source.id,
             baseId: source.base_id,
             config,
-            skipReorder: true,
           },
           ncMeta,
         ),