From 48145d971af7b8bfb0be3b684a1967145c1560e0 Mon Sep 17 00:00:00 2001
From: George Gelashvili and Tira Odhner <pair+ggelashvili+aodhner@pivotal.io>
Date: Mon, 3 Apr 2017 11:23:50 -0400
Subject: [PATCH 10/11] Extract the generic selection methods to
 rangeSelectionHelper

- now just business logic and rendering logic are in row/column selectors.
---
 web/pgadmin/static/js/selection/column_selector.js | 58 ++-----------
 .../static/js/selection/range_selection_helper.js  | 60 ++++++++++++++
 web/pgadmin/static/js/selection/row_selector.js    | 53 +++---------
 .../javascript/selection/column_selector_spec.js   | 95 ++++++++++++++--------
 4 files changed, 141 insertions(+), 125 deletions(-)
 create mode 100644 web/pgadmin/static/js/selection/range_selection_helper.js

diff --git a/web/pgadmin/static/js/selection/column_selector.js b/web/pgadmin/static/js/selection/column_selector.js
index 15267d2c..930ec45f 100644
--- a/web/pgadmin/static/js/selection/column_selector.js
+++ b/web/pgadmin/static/js/selection/column_selector.js
@@ -1,7 +1,5 @@
-define(['jquery', 'slickgrid'], function ($) {
+define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], function ($, rangeSelectionHelper) {
   var ColumnSelector = function () {
-    var Slick = window.Slick;
-
     var init = function (grid) {
       grid.onHeaderClick.subscribe(function (event, eventArgument) {
           var column = eventArgument.column;
@@ -21,74 +19,36 @@ define(['jquery', 'slickgrid'], function ($) {
         .subscribe(handleSelectedRangesChanged.bind(null, grid));
     };
 
-    function rangeForColumn(grid, columnIndex) {
-      return new Slick.Range(0, columnIndex, grid.getDataLength() - 1, columnIndex)
-    }
-
-    function handleSelectedRangesChanged(grid, event, ranges) {
+    var handleSelectedRangesChanged = function (grid, event, ranges) {
       $('[data-cell-type="column-header-row"] input:checked')
         .each(function (index, checkbox) {
           var $checkbox = $(checkbox);
           var columnIndex = grid.getColumnIndex($checkbox.data('column-id'));
-          var isStillSelected = isRangeSelected(ranges, rangeForColumn(grid, columnIndex));
+          var isStillSelected = rangeSelectionHelper.isRangeSelected(ranges, rangeSelectionHelper.rangeForColumn(grid, columnIndex));
           if (!isStillSelected) {
             toggleCheckbox($checkbox);
           }
         });
-    }
-
-    var isRangeSelected = function (selectedRanges, range) {
-      return _.any(selectedRanges, function (selectedRange) {
-        return isSameRange(selectedRange, range)
-      })
     };
 
-    function updateRanges(grid, columnId) {
+    var updateRanges = function (grid, columnId) {
       var selectionModel = grid.getSelectionModel();
       var ranges = selectionModel.getSelectedRanges();
 
       var columnIndex = grid.getColumnIndex(columnId);
 
-      var columnRange = rangeForColumn(grid, columnIndex);
+      var columnRange = rangeSelectionHelper.rangeForColumn(grid, columnIndex);
       var newRanges;
-      if (isColumnSelected(ranges, columnRange)) {
-        newRanges = removeColumn(ranges, columnRange);
+      if (rangeSelectionHelper.isRangeSelected(ranges, columnRange)) {
+        newRanges = rangeSelectionHelper.removeRange(ranges, columnRange);
       } else {
-        if (allRangesAreColumns(ranges, grid)) {
-          newRanges = addColumn(ranges, columnRange);
+        if (rangeSelectionHelper.areAllRangesColumns(ranges, grid)) {
+          newRanges = rangeSelectionHelper.addRange(ranges, columnRange);
         } else {
           newRanges = [columnRange];
         }
       }
       selectionModel.setSelectedRanges(newRanges);
-    }
-
-    var isColumnSelected = function (ranges, columnRange) {
-      return _.any(ranges, function (range) {
-        return isSameRange(range, columnRange)
-      })
-    };
-
-    var isSameRange = function (range, otherRange) {
-      return range.fromCell == otherRange.fromCell && range.toCell == otherRange.toCell &&
-        range.fromRow == otherRange.fromRow && range.toRow == otherRange.toRow;
-    };
-
-    var removeColumn = function (ranges, columnRange) {
-      return _.filter(ranges, function (range) {
-        return !(isSameRange(range, columnRange))
-      })
-    };
-    var addColumn = function (ranges, column) {
-      ranges.push(column);
-      return ranges;
-    };
-
-    var allRangesAreColumns = function (ranges, grid) {
-      return _.every(ranges, function (range) {
-        return range.fromCell == range.toCell &&
-          range.fromRow == 0 && range.toRow == grid.getDataLength() - 1
-      })
     };
 
     var clickedCheckbox = function (e) {
diff --git a/web/pgadmin/static/js/selection/range_selection_helper.js b/web/pgadmin/static/js/selection/range_selection_helper.js
new file mode 100644
index 00000000..c219fcf1
--- /dev/null
+++ b/web/pgadmin/static/js/selection/range_selection_helper.js
@@ -0,0 +1,60 @@
+define(['slickgrid'], function () {
+  var Slick = window.Slick;
+
+  var isSameRange = function (range, otherRange) {
+
+    return range.fromCell == otherRange.fromCell && range.toCell == otherRange.toCell &&
+      range.fromRow == otherRange.fromRow && range.toRow == otherRange.toRow;
+  };
+
+  var isRangeSelected = function (selectedRanges, range) {
+    return _.any(selectedRanges, function (selectedRange) {
+      return isSameRange(selectedRange, range)
+    })
+  };
+
+
+  var removeRange = function (selectedRanges, range) {
+    return _.filter(selectedRanges, function (selectedRange) {
+      return !(isSameRange(selectedRange, range))
+    })
+  };
+
+  var addRange = function (ranges, range) {
+    ranges.push(range);
+    return ranges;
+  };
+
+
+  var areAllRangesRows = function (ranges, grid) {
+    return _.every(ranges, function (range) {
+      return range.fromRow == range.toRow &&
+        range.fromCell == 1 && range.toCell == grid.getColumns().length - 1
+    })
+  };
+
+  var areAllRangesColumns = function (ranges, grid) {
+    return _.every(ranges, function (range) {
+      return range.fromCell == range.toCell &&
+        range.fromRow == 0 && range.toRow == grid.getDataLength() - 1
+    })
+  };
+
+  var rangeForRow = function (grid, rowId) {
+    return new Slick.Range(rowId, 1, rowId, grid.getColumns().length - 1);
+  };
+
+  function rangeForColumn(grid, columnIndex) {
+    return new Slick.Range(0, columnIndex, grid.getDataLength() - 1, columnIndex)
+  }
+
+  return {
+    addRange: addRange,
+    removeRange: removeRange,
+    isRangeSelected: isRangeSelected,
+    areAllRangesRows: areAllRangesRows,
+    areAllRangesColumns: areAllRangesColumns,
+    rangeForRow: rangeForRow,
+    rangeForColumn: rangeForColumn
+  }
+});
\ No newline at end of file
diff --git a/web/pgadmin/static/js/selection/row_selector.js b/web/pgadmin/static/js/selection/row_selector.js
index 09e4aa78..76a8c1a7 100644
--- a/web/pgadmin/static/js/selection/row_selector.js
+++ b/web/pgadmin/static/js/selection/row_selector.js
@@ -1,4 +1,4 @@
-define(['jquery', 'slickgrid'], function ($) {
+define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], function ($, rangeSelectionHelper) {
   var RowSelector = function () {
     var Slick = window.Slick;
 
@@ -11,7 +11,7 @@ define(['jquery', 'slickgrid'], function ($) {
         .subscribe(grid.onClick, handleClick.bind(null, grid))
     };
 
-    function handleClick(grid, event, args) {
+    var handleClick = function (grid, event, args) {
       if (grid.getColumns()[args.cell].id === 'row-header-column') {
         if (event.target.type != "checkbox") {
           var checkbox = $(event.target).find('input[type="checkbox"]');
@@ -21,34 +21,31 @@ define(['jquery', 'slickgrid'], function ($) {
       }
     }
 
-    function handleSelectedRangesChanged(grid, event, ranges) {
+    var handleSelectedRangesChanged = function (grid, event, ranges) {
       $('[data-cell-type="row-header-checkbox"]:checked')
         .each(function (index, checkbox) {
           var $checkbox = $(checkbox);
           var row = parseInt($checkbox.data('row'));
-          var isStillSelected = isRangeSelected(ranges, rangeForRow(grid, row));
+          var isStillSelected = rangeSelectionHelper.isRangeSelected(ranges,
+            rangeSelectionHelper.rangeForRow(grid, row));
           if (!isStillSelected) {
             toggleCheckbox($checkbox);
           }
         });
     }
 
-    function rangeForRow(grid, rowId) {
-      return new Slick.Range(rowId, 1, rowId, grid.getColumns().length - 1);
-    }
-
-    function updateRanges(grid, rowId) {
+    var updateRanges = function (grid, rowId) {
       var selectionModel = grid.getSelectionModel();
       var ranges = selectionModel.getSelectedRanges();
 
-      var rowRange = rangeForRow(grid, rowId);
+      var rowRange = rangeSelectionHelper.rangeForRow(grid, rowId);
 
       var newRanges;
-      if (isRangeSelected(ranges, rowRange)) {
-        newRanges = removeRange(ranges, rowRange);
+      if (rangeSelectionHelper.isRangeSelected(ranges, rowRange)) {
+        newRanges = rangeSelectionHelper.removeRange(ranges, rowRange);
       } else {
-        if (allRangesAreRows(ranges, grid)) {
-          newRanges = addRange(ranges, rowRange);
+        if (rangeSelectionHelper.areAllRangesRows(ranges, grid)) {
+          newRanges = rangeSelectionHelper.addRange(ranges, rowRange);
         } else {
           newRanges = [rowRange];
         }
@@ -56,34 +53,6 @@ define(['jquery', 'slickgrid'], function ($) {
       selectionModel.setSelectedRanges(newRanges);
     }
 
-    var isRangeSelected = function (selectedRanges, range) {
-      return _.any(selectedRanges, function (selectedRange) {
-        return isSameRange(selectedRange, range)
-      })
-    };
-
-    var isSameRange = function (range, otherRange) {
-      return range.fromCell == otherRange.fromCell && range.toCell == otherRange.toCell &&
-        range.fromRow == otherRange.fromRow && range.toRow == otherRange.toRow;
-    };
-
-    var removeRange = function (selectedRanges, range) {
-      return _.filter(selectedRanges, function (selectedRange) {
-        return !(isSameRange(selectedRange, range))
-      })
-    };
-    var addRange = function (ranges, range) {
-      ranges.push(range);
-      return ranges;
-    };
-
-    var allRangesAreRows = function (ranges, grid) {
-      return _.every(ranges, function (range) {
-        return range.fromRow == range.toRow &&
-          range.fromCell == 1 && range.toCell == grid.getColumns().length - 1
-      })
-    };
-
     var toggleCheckbox = function (checkbox) {
       if (checkbox.prop("checked")) {
         checkbox.prop("checked", false)
diff --git a/web/regression/javascript/selection/column_selector_spec.js b/web/regression/javascript/selection/column_selector_spec.js
index 7c7a2127..3209befe 100644
--- a/web/regression/javascript/selection/column_selector_spec.js
+++ b/web/regression/javascript/selection/column_selector_spec.js
@@ -65,7 +65,7 @@ define(
         expect(selectableColumns[0].id).toBe('1');
       });
 
-      describe("when the user clicks on a column header", function () {
+      describe("selecting columns", function () {
         var grid, rowSelectionModel;
         beforeEach(function () {
           var columnSelector = new ColumnSelector();
@@ -88,45 +88,48 @@ define(
           $("body").find(container).remove();
         });
 
-        it("selects the column", function () {
-          container.find('.slick-header-column')[1].click();
-          var selectedRanges = rowSelectionModel.getSelectedRanges();
-          var column = selectedRanges[0];
-
-          expect(selectedRanges.length).toEqual(1);
-          expect(column.fromCell).toBe(1);
-          expect(column.toCell).toBe(1);
-          expect(column.fromRow).toBe(0);
-          expect(column.toRow).toBe(9);
+        describe("when the user clicks a column header", function () {
+          it("selects the column", function () {
+            container.find('.slick-header-column')[0].click();
+            var selectedRanges = rowSelectionModel.getSelectedRanges();
+            expectOnlyTheFirstColumnToBeSelected(selectedRanges);
+          });
         });
 
-        it("selects another column", function () {
-          container.find('.slick-header-column')[1].click();
-          container.find('.slick-header-column')[0].click();
+        describe("when the user clicks additional column headers", function () {
+          beforeEach(function () {
+            container.find('.slick-header-column')[1].click();
+          });
 
-          var selectedRanges = rowSelectionModel.getSelectedRanges();
-          var column1 = selectedRanges[0];
+          it("selects additional columns", function () {
+            container.find('.slick-header-column')[0].click();
 
-          expect(selectedRanges.length).toEqual(2);
-          expect(column1.fromCell).toBe(1);
-          expect(column1.toCell).toBe(1);
+            var selectedRanges = rowSelectionModel.getSelectedRanges();
+            var column1 = selectedRanges[0];
 
-          var column2 = selectedRanges[1];
+            expect(selectedRanges.length).toEqual(2);
+            expect(column1.fromCell).toBe(1);
+            expect(column1.toCell).toBe(1);
 
-          expect(column2.fromCell).toBe(0);
-          expect(column2.toCell).toBe(0);
+            var column2 = selectedRanges[1];
+
+            expect(column2.fromCell).toBe(0);
+            expect(column2.toCell).toBe(0);
+          });
         });
 
-        it("allows clicking on the checkbox", function () {
-          container.find('.slick-header-column')[1].click();
-          container.find('.slick-header-columns input')[1].click();
+        describe("when the user clicks a column header checkbox", function () {
+          it("selects the column", function () {
+            container.find('.slick-header-columns input')[0].click();
 
-          expect($(container.find('.slick-header-columns input')[1]).is(':checked')).toBeFalsy();
-        });
+            var selectedRanges = rowSelectionModel.getSelectedRanges();
+            expectOnlyTheFirstColumnToBeSelected(selectedRanges);
+          });
 
-        it("checks the checkbox", function () {
-          container.find('.slick-header-column')[1].click();
-          expect($(container.find('.slick-header-columns input')[1]).is(':checked')).toBeTruthy();
+          it("checks the checkbox", function () {
+            container.find('.slick-header-column')[1].click();
+            expect($(container.find('.slick-header-columns input')[1]).is(':checked')).toBeTruthy();
+          });
         });
 
         describe("when a row is selected", function () {
@@ -150,7 +153,7 @@ define(
           })
         });
 
-        describe("click a second time", function () {
+        describe("clicking a second time", function () {
           beforeEach(function () {
             container.find('.slick-header-column')[1].click();
           });
@@ -160,7 +163,7 @@ define(
             expect($(container.find('.slick-header-columns input')[1]).is(':checked')).toBeFalsy();
           });
 
-          it("unselects the column", function () {
+          it("deselects the column", function () {
             container.find('.slick-header-column')[1].click();
             var selectedRanges = rowSelectionModel.getSelectedRanges();
 
@@ -168,7 +171,7 @@ define(
           })
         });
 
-        describe("and the column is not selectable", function () {
+        describe("when the column is not selectable", function () {
           it("does not select the column", function () {
             $(container.find('.slick-header-column:contains(some-non-selectable-column)')).click();
             var selectedRanges = rowSelectionModel.getSelectedRanges();
@@ -182,13 +185,27 @@ define(
             container.find('.slick-header-column')[1].click();
           });
 
-          it("should uncheck the checkbox", function () {
+          it("unchecks the checkbox", function () {
             rowSelectionModel.setSelectedRanges([]);
 
             expect($(container.find('.slick-header-columns input')[1])
               .is(':checked')).toBeFalsy();
           });
         });
+
+        describe("when a non-column range was already selected", function () {
+          beforeEach(function () {
+            var selectedRanges = [new Slick.Range(0, 0, 1, 0)];
+            rowSelectionModel.setSelectedRanges(selectedRanges);
+          });
+
+          it("deselects the non-column range", function () {
+            container.find('.slick-header-column')[0].click();
+
+            var selectedRanges = rowSelectionModel.getSelectedRanges();
+            expectOnlyTheFirstColumnToBeSelected(selectedRanges);
+          })
+        });
       });
 
       var setupGrid = function (columns) {
@@ -202,5 +219,15 @@ define(
         grid.registerPlugin(columnSelector);
         grid.invalidate();
       };
+
+      function expectOnlyTheFirstColumnToBeSelected(selectedRanges) {
+        var row = selectedRanges[0];
+
+        expect(selectedRanges.length).toEqual(1);
+        expect(row.fromCell).toBe(0);
+        expect(row.toCell).toBe(0);
+        expect(row.fromRow).toBe(0);
+        expect(row.toRow).toBe(9);
+      }
     });
   });
-- 
2.12.0

