Fix histogram removing last spurious bin

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Fix material-ui warnings due to wrong styles in theme [#124](https://github.com/CartoDB/carto-react/pull/124)
 - Add Widgets from @carto/react-widgets to StoryBook [#120](https://github.com/CartoDB/carto-react/pull/120)
 - Improve GoogleMap component [#121](https://github.com/CartoDB/carto-react/pull/121)
+- Fix histogram removing last spurious bin [#123](https://github.com/CartoDB/carto-react/pull/123)
 
 ## 1.0.0-rc.1 (2021-03-11)
 

DEVELOPERS.md

@@ -15,7 +15,7 @@ From now on, use one of the root level commands, that lerna will execute for all
   yarn test
 ```
 
-If you have issues, you can always run `yarn build:full`. It will perform a full clean and then ensure that install, build and test work fine
+If you have issues, you can always run `yarn build:clean`. It will perform a full clean and then ensure that install, build and test work fine
 
 ## Link from carto-react-template
 

package.json

@@ -34,7 +34,7 @@
   "scripts": {
     "start": "lerna run build:watch --stream --parallel",
     "build": "yarn clean:build && NODE_ENV=production lerna run build --stream",
-    "build:full": "yarn clean:all && yarn install && yarn build && yarn test",
+    "build:clean": "yarn clean:all && yarn install && yarn build && yarn test",
     "clean:all": "yarn clean:node_modules && yarn clean:build",
     "clean:node_modules": "rm -rf ./node_modules && lerna exec -- rm -rf ./node_modules ",
     "clean:build": "rm -rf ./coverage ./.nyc_output ./reports && lerna exec -- rm -rf ./dist ./coverage",
@@ -48,7 +48,7 @@
     "storybook:start": "lerna --scope @carto/react-ui exec -- npm run storybook:start",
     "storybook:build": "lerna --scope @carto/react-ui exec -- npm run storybook:build",
     "storybook:publish": "lerna --scope @carto/react-ui exec -- npm run storybook:publish",
-    "prerelease": "yarn run build:full && lerna publish --dist-tag prerelease --force-publish --preid rc"
+    "prerelease": "yarn run build:clean && lerna publish --dist-tag prerelease --force-publish --preid rc"
   },
   "husky": {
     "hooks": {

packages/react-core/__tests__/operations/histogram.test.js

@@ -37,7 +37,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.COUNT
       );
-      expect(h.length).toBe(1 + ticks.length + 1);
+      expect(h.length).toBe(1 + ticks.length);
     });
 
     test(AggregationTypes.COUNT, () => {
@@ -47,7 +47,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.COUNT
       );
-      expect(h).toEqual([0, 1, 2, 3, 2, 1, 0]);
+      expect(h).toEqual([0, 1, 2, 3, 2, 1]);
     });
 
     test(AggregationTypes.AVG, () => {
@@ -57,7 +57,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.AVG
       );
-      expect(h).toEqual([0, 1, 2, 3, 4, 5, 0]);
+      expect(h).toEqual([0, 1, 2, 3, 4, 5]);
     });
 
     test(AggregationTypes.MIN, () => {
@@ -67,7 +67,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.MIN
       );
-      expect(h).toEqual([0, 1, 2, 3, 4, 5, 0]);
+      expect(h).toEqual([0, 1, 2, 3, 4, 5]);
     });
 
     test(AggregationTypes.MAX, () => {
@@ -77,7 +77,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.MAX
       );
-      expect(h).toEqual([0, 1, 2, 3, 4, 5, 0]);
+      expect(h).toEqual([0, 1, 2, 3, 4, 5]);
     });
 
     test(AggregationTypes.SUM, () => {
@@ -87,7 +87,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.SUM
       );
-      expect(h).toEqual([0, 1, 4, 9, 8, 5, 0]);
+      expect(h).toEqual([0, 1, 4, 9, 8, 5]);
     });
   });
 
@@ -99,7 +99,7 @@ describe('histogram', () => {
         ticks,
         AggregationTypes.COUNT
       );
-      expect(h).toEqual([0, 0, 0, 0, 0, 0, 0]);
+      expect(h).toEqual([0, 0, 0, 0, 0, 0]);
     });
   });
 });

packages/react-core/src/operations/histogram.js

@@ -5,12 +5,12 @@ export function histogram(features, columnName, ticks, operation) {
     return [];
   }
 
-  ticks = [Number.MIN_SAFE_INTEGER, ...ticks, Number.MAX_SAFE_INTEGER];
+  ticks = [Number.MIN_SAFE_INTEGER, ...ticks];
 
-  const binsContainer = ticks.map((tick, idx, arr) => ({
-    bin: idx,
+  const binsContainer = ticks.map((tick, index, arr) => ({
+    bin: index,
     start: tick,
-    end: arr[idx + 1],
+    end: index === arr.length - 1 ? Number.MAX_SAFE_INTEGER : arr[index + 1],
     values: []
   }));
 

packages/react-redux/src/slices/cartoSlice.js

@@ -216,9 +216,11 @@ export const setBasemap = (basemap) => ({ type: 'carto/setBasemap', payload: bas
 
 /**
  * Action to add a filter on a given source and column
- * @param {string} id - sourceId of the source to apply the filter
- * @param {string} column - column to filter at the source
+ * @param {string} id - sourceId of the source to apply the filter on
+ * @param {string} column - column to use by the filter at the source
  * @param {FilterType} type - FilterTypes.IN and FilterTypes.BETWEEN
+ * @param {array} values -  Values for the filter (eg: ['a', 'b'] for IN or [10, 20] for BETWEEN)
+ * @param {string} owner - (optional) id of the widget triggering the filter (to be excluded)
  */
 export const addFilter = ({ id, column, type, values, owner }) => ({
   type: 'carto/addFilter',

packages/react-widgets/__tests__/models/HistogramModel.test.js

@@ -112,31 +112,31 @@ describe('getHistogram', () => {
       test(AggregationTypes.COUNT, async () => {
         const params = buildParamsFor(AggregationTypes.COUNT);
         const histogram = await getHistogram(params);
-        expect(histogram).toEqual([0, 1, 2, 1, 0]);
+        expect(histogram).toEqual([0, 1, 2, 1]);
       });
 
       test(AggregationTypes.AVG, async () => {
         const params = buildParamsFor(AggregationTypes.AVG);
         const histogram = await getHistogram(params);
-        expect(histogram).toEqual([0, 0, 1, 2, 0]);
+        expect(histogram).toEqual([0, 0, 1, 2]);
       });
 
       test(AggregationTypes.SUM, async () => {
         const params = buildParamsFor(AggregationTypes.SUM);
         const histogram = await getHistogram(params);
-        expect(histogram).toEqual([0, 0, 2, 2, 0]);
+        expect(histogram).toEqual([0, 0, 2, 2]);
       });
 
       test(AggregationTypes.MIN, async () => {
         const params = buildParamsFor(AggregationTypes.MIN);
         const histogram = await getHistogram(params);
-        expect(histogram).toEqual([0, 0, 1, 2, 0]);
+        expect(histogram).toEqual([0, 0, 1, 2]);
       });
 
       test(AggregationTypes.MAX, async () => {
         const params = buildParamsFor(AggregationTypes.MAX);
         const histogram = await getHistogram(params);
-        expect(histogram).toEqual([0, 0, 1, 2, 0]);
+        expect(histogram).toEqual([0, 0, 1, 2]);
       });
     });
   });
@@ -152,15 +152,15 @@ describe('buildSqlQueryToGetHistogram', () => {
   });
 
   const buildQuery = (params) => `
-    SELECT 
+    SELECT
       tick, ${params.operation}(${params.operationColumn}) as value
-    FROM 
+    FROM
       (
-        SELECT 
-          CASE WHEN revenue < 1200000 THEN 'cat_0' WHEN revenue < 1300000 THEN 'cat_1' WHEN revenue < 1400000 THEN 'cat_2' WHEN revenue < 1500000 THEN 'cat_3' WHEN revenue < 1600000 THEN 'cat_4' WHEN revenue < 1700000 THEN 'cat_5' WHEN revenue < 1800000 THEN 'cat_6' ELSE 'cat_7' END as tick, ${params.operationColumn} 
+        SELECT
+          CASE WHEN revenue < 1200000 THEN 'cat_0' WHEN revenue < 1300000 THEN 'cat_1' WHEN revenue < 1400000 THEN 'cat_2' WHEN revenue < 1500000 THEN 'cat_3' WHEN revenue < 1600000 THEN 'cat_4' WHEN revenue < 1700000 THEN 'cat_5' WHEN revenue < 1800000 THEN 'cat_6' ELSE 'cat_7' END as tick, ${params.operationColumn}
         FROM (
-          SELECT 
-            * 
+          SELECT
+            *
           FROM (${params.data}) as q2
         ) as q1
       ) as q
@@ -208,27 +208,27 @@ describe('filterViewportFeaturesToGetHistogram', () => {
 
   test(AggregationTypes.COUNT, () => {
     const params = buildParamsFor(AggregationTypes.COUNT);
-    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 1, 2, 1, 0]);
+    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 1, 2, 1]);
   });
 
   test(AggregationTypes.AVG, () => {
     const params = buildParamsFor(AggregationTypes.AVG);
-    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2, 0]);
+    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2]);
   });
 
   test(AggregationTypes.SUM, () => {
     const params = buildParamsFor(AggregationTypes.SUM);
-    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 2, 2, 0]);
+    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 2, 2]);
   });
 
   test(AggregationTypes.MIN, () => {
     const params = buildParamsFor(AggregationTypes.MIN);
-    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2, 0]);
+    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2]);
   });
 
   test(AggregationTypes.MAX, () => {
     const params = buildParamsFor(AggregationTypes.MAX);
-    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2, 0]);
+    expect(filterViewportFeaturesToGetHistogram(params)).toEqual([0, 0, 1, 2]);
   });
 
   test('no features', () => {