fix: prevent empty entries when dragging items past last row (#5384)

Co-authored-by: nivedin <nivedinp@gmail.com>
Co-authored-by: jamesgeorge007 <25279263+jamesgeorge007@users.noreply.github.com>

packages/hoppscotch-common/src/components/graphql/Headers.vue

@@ -59,6 +59,10 @@
       ghost-class="cursor-move"
       chosen-class="bg-primaryLight"
       drag-class="cursor-grabbing"
+      :move="
+        (event: DragDropEvent) =>
+          isDragDropAllowed(event, workingHeaders.length)
+      "
     >
       <template #item="{ element: header, index }">
         <HttpKeyValue
@@ -253,6 +257,7 @@ import { throwError } from "~/helpers/functional/error"
 import { objRemoveKey } from "~/helpers/functional/object"
 import { commonHeaders } from "~/helpers/headers"
 import { HoppInheritedProperty } from "~/helpers/types/HoppInheritedProperties"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 import { toggleNestedSetting } from "~/newstore/settings"
 import IconArrowUpRight from "~icons/lucide/arrow-up-right"
 import IconEdit from "~icons/lucide/edit"

packages/hoppscotch-common/src/components/http/BodyParameters.vue

@@ -68,6 +68,9 @@
       ghost-class="cursor-move"
       chosen-class="bg-primaryLight"
       drag-class="cursor-grabbing"
+      :move="
+        (event: DragDropEvent) => isDragDropAllowed(event, workingParams.length)
+      "
     >
       <template #item="{ element: { entry }, index }">
         <div
@@ -267,6 +270,7 @@ import { useNestedSetting } from "~/composables/settings"
 import { toggleNestedSetting } from "~/newstore/settings"
 import * as E from "fp-ts/Either"
 import linter from "~/helpers/editor/linting/rawKeyValue"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 
 type Body = HoppRESTReqBody & { contentType: "multipart/form-data" }
 

packages/hoppscotch-common/src/components/http/Headers.vue

@@ -68,6 +68,10 @@
         ghost-class="cursor-move"
         chosen-class="bg-primaryLight"
         drag-class="cursor-grabbing"
+        :move="
+          (event: DragDropEvent): boolean =>
+            isDragDropAllowed(event, workingHeaders.length)
+        "
       >
         <template #item="{ element: header, index }">
           <HttpKeyValue
@@ -274,6 +278,7 @@ import {
   getComputedAuthHeaders,
   getComputedHeaders,
 } from "~/helpers/utils/EffectiveURL"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 import {
   AggregateEnvironment,
   aggregateEnvs$,

packages/hoppscotch-common/src/components/http/Parameters.vue

@@ -57,6 +57,10 @@
         ghost-class="cursor-move"
         chosen-class="bg-primaryLight"
         drag-class="cursor-grabbing"
+        :move="
+          (event: DragDropEvent) =>
+            isDragDropAllowed(event, workingParams.length)
+        "
       >
         <template #item="{ element: param, index }">
           <HttpKeyValue
@@ -120,6 +124,7 @@ import {
 import { isEqual, cloneDeep } from "lodash-es"
 import draggable from "vuedraggable-es"
 import linter from "~/helpers/editor/linting/rawKeyValue"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 import { useCodemirror } from "@composables/codemirror"
 import { useColorMode } from "@composables/theming"
 import { useI18n } from "@composables/i18n"

packages/hoppscotch-common/src/components/http/RequestVariables.vue

@@ -59,6 +59,10 @@
         ghost-class="cursor-move"
         chosen-class="bg-primaryLight"
         drag-class="cursor-grabbing"
+        :move="
+          (event: DragDropEvent) =>
+            isDragDropAllowed(event, workingRequestVariables.length)
+        "
       >
         <template #item="{ element: variable, index }">
           <div
@@ -187,6 +191,7 @@ import { useI18n } from "~/composables/i18n"
 import { useNestedSetting } from "~/composables/settings"
 import { useColorMode } from "~/composables/theming"
 import { useToast } from "~/composables/toast"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 import linter from "~/helpers/editor/linting/rawKeyValue"
 import { objRemoveKey } from "~/helpers/functional/object"
 import { toggleNestedSetting } from "~/newstore/settings"

packages/hoppscotch-common/src/components/http/URLEncodedParams.vue

@@ -60,6 +60,10 @@
         ghost-class="cursor-move"
         chosen-class="bg-primaryLight"
         drag-class="cursor-grabbing"
+        :move="
+          (event: DragDropEvent) =>
+            isDragDropAllowed(event, workingUrlEncodedParams.length)
+        "
       >
         <template #item="{ element: param, index }">
           <div
@@ -209,6 +213,7 @@ import { useVModel } from "@vueuse/core"
 import { useNestedSetting } from "~/composables/settings"
 import { toggleNestedSetting } from "~/newstore/settings"
 import { AggregateEnvironment } from "~/newstore/environments"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 
 type Body = HoppRESTReqBody & {
   contentType: "application/x-www-form-urlencoded"

packages/hoppscotch-common/src/helpers/__tests__/dragDropValidation.spec.ts

@@ -0,0 +1,244 @@
+import { describe, test, expect } from "vitest"
+import { isDragDropAllowed, DragDropEvent } from "../dragDropValidation"
+
+describe("isDragDropAllowed", () => {
+  describe("Valid drag operations", () => {
+    test("allows dropping at the beginning of a list", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 0,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("allows dropping in the middle of a list", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 2,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("allows dropping at second-to-last position", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 3,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("allows dropping in a list with only one empty item", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 0,
+        },
+      }
+      const totalItems = 2
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+  })
+
+  describe("Invalid drag operations", () => {
+    test("prevents dropping at the last position (reserved for empty entry)", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 4,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+
+    test("prevents dropping beyond the last position", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 10,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+
+    test("prevents dropping at the last position in a minimal list", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 1,
+        },
+      }
+      const totalItems = 2
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+  })
+
+  describe("Edge cases and invalid input", () => {
+    test("returns false when dragEvent is null", () => {
+      expect(isDragDropAllowed(null as any, 5)).toBe(false)
+    })
+
+    test("returns false when dragEvent is undefined", () => {
+      expect(isDragDropAllowed(undefined as any, 5)).toBe(false)
+    })
+
+    test("returns false when draggedContext is missing", () => {
+      const dragEvent = {} as DragDropEvent
+      expect(isDragDropAllowed(dragEvent, 5)).toBe(false)
+    })
+
+    test("returns false when futureIndex is missing", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {},
+      }
+      expect(isDragDropAllowed(dragEvent, 5)).toBe(false)
+    })
+
+    test("returns false when futureIndex is null", () => {
+      const dragEvent = {
+        draggedContext: {
+          futureIndex: null as any,
+        },
+      } as DragDropEvent
+      expect(isDragDropAllowed(dragEvent, 5)).toBe(false)
+    })
+
+    test("returns false when futureIndex is not a number", () => {
+      const dragEvent = {
+        draggedContext: {
+          futureIndex: "invalid" as any,
+        },
+      } as DragDropEvent
+      expect(isDragDropAllowed(dragEvent, 5)).toBe(false)
+    })
+
+    test("returns false when futureIndex is NaN", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: NaN,
+        },
+      }
+      expect(isDragDropAllowed(dragEvent, 5)).toBe(false)
+    })
+
+    test("returns false when totalItems is null", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 2,
+        },
+      }
+      expect(isDragDropAllowed(dragEvent, null as any)).toBe(false)
+    })
+
+    test("returns false when totalItems is not a number", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 2,
+        },
+      }
+      expect(isDragDropAllowed(dragEvent, "invalid" as any)).toBe(false)
+    })
+
+    test("returns false when totalItems is NaN", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 2,
+        },
+      }
+      expect(isDragDropAllowed(dragEvent, NaN)).toBe(false)
+    })
+
+    test("handles negative futureIndex correctly", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: -1,
+        },
+      }
+      const totalItems = 5
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("handles zero totalItems correctly", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 0,
+        },
+      }
+      const totalItems = 0
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+
+    test("handles minimal valid case (totalItems = 1)", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 0,
+        },
+      }
+      const totalItems = 1
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+  })
+
+  describe("Real-world scenarios", () => {
+    test("simulates dragging a header to position 0 in a list of 3 items", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 0,
+        },
+      }
+      const totalItems = 3 // 2 real headers + 1 empty
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("simulates dragging a header to position 1 in a list of 3 items", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 1,
+        },
+      }
+      const totalItems = 3 // 2 real headers + 1 empty
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(true)
+    })
+
+    test("prevents dragging a header to the empty position (position 2) in a list of 3 items", () => {
+      const dragEvent: DragDropEvent = {
+        draggedContext: {
+          futureIndex: 2,
+        },
+      }
+      const totalItems = 3 // 2 real headers + 1 empty
+
+      expect(isDragDropAllowed(dragEvent, totalItems)).toBe(false)
+    })
+
+    test("simulates a large list with many items", () => {
+      const totalItems = 100 // 99 real items + 1 empty
+
+      // Should allow dropping anywhere except the last position
+      const dragEvent0: DragDropEvent = { draggedContext: { futureIndex: 0 } }
+      const dragEvent50: DragDropEvent = { draggedContext: { futureIndex: 50 } }
+      const dragEvent98: DragDropEvent = { draggedContext: { futureIndex: 98 } }
+      const dragEvent99: DragDropEvent = { draggedContext: { futureIndex: 99 } }
+
+      expect(isDragDropAllowed(dragEvent0, totalItems)).toBe(true)
+      expect(isDragDropAllowed(dragEvent50, totalItems)).toBe(true)
+      expect(isDragDropAllowed(dragEvent98, totalItems)).toBe(true)
+      expect(isDragDropAllowed(dragEvent99, totalItems)).toBe(false)
+    })
+  })
+})

packages/hoppscotch-common/src/helpers/dragDropValidation.ts

@@ -0,0 +1,38 @@
+export type DragDropEvent = {
+  draggedContext?: {
+    futureIndex?: number
+  }
+}
+
+/**
+ * Validates if a drag-and-drop operation should be allowed.
+ *
+ * Prevents items from being dropped at the last position, which is reserved
+ * for the "add new item" empty entry in lists like headers and parameters.
+ *
+ * @param dragEvent - The drag event containing position information
+ * @param totalItems - The current length of the items array
+ * @returns true if the drop is allowed, false otherwise
+ */
+export function isDragDropAllowed(
+  dragEvent: DragDropEvent,
+  totalItems: number
+): boolean {
+  const targetIndex = dragEvent?.draggedContext?.futureIndex
+
+  // Validate input parameters
+  if (
+    typeof targetIndex !== "number" ||
+    isNaN(targetIndex) ||
+    isNaN(totalItems)
+  ) {
+    return false
+  }
+
+  // Prevent dropping at the last position (reserved for empty entry)
+  if (targetIndex >= totalItems - 1) {
+    return false
+  }
+
+  return true
+}

packages/hoppscotch-common/src/pages/realtime/websocket.vue

@@ -81,6 +81,10 @@
             ghost-class="cursor-move"
             chosen-class="bg-primaryLight"
             drag-class="cursor-grabbing"
+            :move="
+              (event: DragDropEvent) =>
+                isDragDropAllowed(event, protocols?.length)
+            "
           >
             <template #item="{ element: { protocol }, index }">
               <div
@@ -212,6 +216,7 @@ import { useColorMode } from "@composables/theming"
 import { WSConnection, WSErrorMessage } from "@helpers/realtime/WSConnection"
 import RegexWorker from "@workers/regex?worker"
 import { LogEntryData } from "~/components/realtime/Log.vue"
+import { isDragDropAllowed, DragDropEvent } from "~/helpers/dragDropValidation"
 
 const t = useI18n()
 const toast = useToast()