fix: Call onCardDragEnd and onColumnDragEnd only when the movement is…

… to another position (#261) Co-authored-by: MatheusPoliCamilo <matheuspolicamilo@gmail.com> Co-authored-by: sousajunior <cdesousajunior@gmail.com>
asseinfo · Feb 13, 2020 · a119d3e · a119d3e
1 parent 8122c94
commit a119d3e
Show file tree

Hide file tree

Showing 4 changed files with 145 additions and 5 deletions.
diff --git a/src/components/Board/index.js b/src/components/Board/index.js
@@ -7,7 +7,13 @@ import withDroppable from '../withDroppable'
 import { when, partialRight } from '@services/utils'
 import DefaultColumnHeader from './components/DefaultColumnHeader'
 import DefaultCard from './components/DefaultCard'
-import { getCard, getCoordinates, isAColumnMove } from './services'
+import {
+  getCard,
+  getCoordinates,
+  isAColumnMove,
+  isMovingAColumnToAnotherPosition,
+  isMovingACardToAnotherPosition
+} from './services'
 import { moveCard, moveColumn, addColumn, removeColumn, changeColumn, addCard, removeCard } from '@services/helpers'
 
 const StyledBoard = styled.div`
@@ -216,8 +222,10 @@ function BoardContainer({
     if (!coordinates.source) return
 
     isAColumnMove(event.type)
-      ? onColumnDragEnd({ ...coordinates, subject: board.columns[coordinates.source.fromPosition] })
-      : onCardDragEnd({ ...coordinates, subject: getCard(board, coordinates.source) })
+      ? isMovingAColumnToAnotherPosition(coordinates) &&
+        onColumnDragEnd({ ...coordinates, subject: board.columns[coordinates.source.fromPosition] })
+      : isMovingACardToAnotherPosition(coordinates) &&
+        onCardDragEnd({ ...coordinates, subject: getCard(board, coordinates.source) })
   }
 
   return (

diff --git a/src/components/Board/index.spec.js b/src/components/Board/index.spec.js
@@ -99,6 +99,21 @@ describe('<Board />', () => {
           })
         })
 
+        describe('whe the user moves a card to the same position', () => {
+          beforeEach(() => {
+            act(() => {
+              callbacks.onDragEnd({
+                source: { droppableId: '1', index: 0 },
+                destination: { droppableId: '1', index: 0 }
+              })
+            })
+          })
+
+          it('does not call onCardDragEnd callback', () => {
+            expect(onCardDragEnd).not.toHaveBeenCalled()
+          })
+        })
+
         describe('when the user moves a card to another position', () => {
           beforeEach(() => {
             act(() => {
@@ -138,6 +153,18 @@ describe('<Board />', () => {
           })
         })
 
+        describe('when the user moves a column to same position', () => {
+          beforeEach(() => {
+            act(() => {
+              callbacks.onDragEnd({ source: { index: 0 }, destination: { index: 0 }, type: 'BOARD' })
+            })
+          })
+
+          it('does not call onColumnDragEnd callback', () => {
+            expect(onColumnDragEnd).not.toHaveBeenCalled()
+          })
+        })
+
         describe('when the user moves a column to another position', () => {
           beforeEach(() => {
             act(() => {
@@ -624,6 +651,21 @@ describe('<Board />', () => {
           })
         })
 
+        describe('whe the user moves a card to the same position', () => {
+          beforeEach(() => {
+            act(() => {
+              callbacks.onDragEnd({
+                source: { droppableId: '1', index: 0 },
+                destination: { droppableId: '1', index: 0 }
+              })
+            })
+          })
+
+          it('does not call onCardDragEnd callback', () => {
+            expect(onCardDragEnd).not.toHaveBeenCalled()
+          })
+        })
+
         describe('when the user moves a card to another position', () => {
           beforeEach(() => {
             act(() => {
@@ -695,6 +737,18 @@ describe('<Board />', () => {
           })
         })
 
+        describe('when the user moves a column to same position', () => {
+          beforeEach(() => {
+            act(() => {
+              callbacks.onDragEnd({ source: { index: 0 }, destination: { index: 0 }, type: 'BOARD' })
+            })
+          })
+
+          it('does not call onColumnDragEnd callback', () => {
+            expect(onColumnDragEnd).not.toHaveBeenCalled()
+          })
+        })
+
         describe('when the user moves a column to another position', () => {
           beforeEach(() => {
             act(() => {

diff --git a/src/components/Board/services/index.js b/src/components/Board/services/index.js
@@ -27,4 +27,15 @@ function getColumn(board, droppableId) {
   return board.columns.find(({ id }) => String(id) === droppableId)
 }
 
-export { getCard, getCoordinates, isAColumnMove }
+function isMovingAColumnToAnotherPosition(coordinates) {
+  return coordinates.source.fromPosition !== coordinates.destination.toPosition
+}
+
+function isMovingACardToAnotherPosition(coordinates) {
+  return !(
+    coordinates.source.fromPosition === coordinates.destination.toPosition &&
+    coordinates.source.fromColumnId === coordinates.destination.toColumnId
+  )
+}
+
+export { getCard, getCoordinates, isAColumnMove, isMovingAColumnToAnotherPosition, isMovingACardToAnotherPosition }
diff --git a/src/components/Board/services/index.spec.js b/src/components/Board/services/index.spec.js
@@ -1,4 +1,10 @@
-import { getCard, isAColumnMove, getCoordinates } from './'
+import {
+  getCard,
+  isAColumnMove,
+  getCoordinates,
+  isMovingAColumnToAnotherPosition,
+  isMovingACardToAnotherPosition
+} from './'
 
 describe('#getCoordinates', () => {
   describe('when the event does not have destination', () => {
@@ -65,3 +71,64 @@ describe('#getCard', () => {
     expect(getCard(board, { fromColumnId: 1, fromPosition: 1 })).toEqual({ id: 2 })
   })
 })
+
+describe('#isMovingAColumnToAnotherPosition', () => {
+  describe('when coordinates does not have same source and destination', () => {
+    const validColumnCoordinates = {
+      source: { fromPosition: 0 },
+      destination: { toPosition: 1 }
+    }
+
+    it('returns true', () => {
+      expect(isMovingAColumnToAnotherPosition(validColumnCoordinates)).toEqual(true)
+    })
+  })
+
+  describe('when coordinates has same source and destination', () => {
+    const invalidColumnCoordinates = {
+      source: { fromPosition: 0 },
+      destination: { toPosition: 0 }
+    }
+
+    it('returns false', () => {
+      expect(isMovingAColumnToAnotherPosition(invalidColumnCoordinates)).toEqual(false)
+    })
+  })
+})
+
+describe('#isMovingACardToAnotherPosition', () => {
+  describe('when coordinates does not have same source and destination', () => {
+    describe('when the source column is different from the destination column', () => {
+      const validCardCoordinates = {
+        source: { fromPosition: 0, fromColumnId: 0 },
+        destination: { toPosition: 0, toColumnId: 1 }
+      }
+
+      it('returns true', () => {
+        expect(isMovingACardToAnotherPosition(validCardCoordinates)).toEqual(true)
+      })
+    })
+
+    describe('when the source position is different from the destination position', () => {
+      const validCardCoordinates = {
+        source: { fromPosition: 0, fromColumnId: 0 },
+        destination: { toPosition: 1, toColumnId: 0 }
+      }
+
+      it('returns true', () => {
+        expect(isMovingACardToAnotherPosition(validCardCoordinates)).toEqual(true)
+      })
+    })
+  })
+
+  describe('when coordinates has same source and destination', () => {
+    const validCardCoordinates = {
+      source: { fromPosition: 0, fromColumnId: 0 },
+      destination: { toPosition: 0, toColumnId: 0 }
+    }
+
+    it('returns false', () => {
+      expect(isMovingACardToAnotherPosition(validCardCoordinates)).toEqual(false)
+    })
+  })
+})