fix(queries): Ignore duplicate recent queries COMPASS-2237

packages/compass-query-history/src/stores/favorite-list-store.js

@@ -1,8 +1,9 @@
+import _ from 'lodash';
 import Reflux from 'reflux';
 import StateMixin from 'reflux-state-mixin';
 import { FavoriteQuery, FavoriteQueryCollection } from '../models';
 import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging';
-import { isDeepStrictEqual } from 'util';
+import { comparableQuery } from '../utils';
 const { track } = createLoggerAndTelemetry('COMPASS-QUERY-HISTORY-UI');
 
 /**
@@ -60,18 +61,12 @@ const configureStore = (options = {}) => {
     },
 
     runQuery(query) {
-      // Loosely match queries against known history entries, because
-      // currently we do not distinguish between favorites and recents
-      // when running queries. This way, we do track some queries twice
-      // (because there are more options than just .filter), but that
-      // is probably fine as a temporary measure.
-      // https://jira.mongodb.org/browse/COMPASS-5243
-      const item = this.state.items
-        .map((_item) => _item.serialize())
-        .find((_item) => {
-          return isDeepStrictEqual(_item.filter, query.filter);
+      const existingQuery = this.state.items
+        .find((item) => {
+          return _.isEqual(comparableQuery(item), query);
         });
-      if (item) {
+      if (existingQuery) {
+        const item = existingQuery.serialize();
         track('Query History Favorite Used', {
           id: item._id,
           screen: 'documents'

packages/compass-query-history/src/stores/recent-list-store.js

@@ -2,16 +2,15 @@ import Reflux from 'reflux';
 import StateMixin from 'reflux-state-mixin';
 import { remote } from 'electron';
 import _ from 'lodash';
-import { isDeepStrictEqual } from 'util';
-
-import { formatQuery } from '../utils';
+import { formatQuery, comparableQuery } from '../utils';
 import { RecentQuery, RecentQueryCollection } from '../models';
 import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging';
 const { track } = createLoggerAndTelemetry('COMPASS-QUERY-HISTORY-UI');
 
 const TOTAL_RECENTS = 30;
 const ALLOWED = ['filter', 'project', 'sort', 'skip', 'limit', 'collation'];
 
+
 /**
  * Query History Recent List store.
  */
@@ -25,7 +24,9 @@ const configureStore = (options = {}) => {
      * Filter attributes that aren't query fields or have default/empty values.
      * @param {object} attributes
      */
-    _filterDefaults(attributes) {
+    _filterDefaults(_attributes) {
+      // don't mutate the passed in parameter
+      const attributes = _.clone(_attributes);
       for (const key in attributes) {
         if (attributes.hasOwnProperty(key)) {
           if (!attributes[key] || ALLOWED.indexOf(key) === -1) {
@@ -35,6 +36,7 @@ const configureStore = (options = {}) => {
           }
         }
       }
+      return attributes;
     },
 
     onConnected() {
@@ -58,7 +60,7 @@ const configureStore = (options = {}) => {
       const ns = recent.ns;
 
       /* Ignore empty or default queries */
-      this._filterDefaults(recent);
+      recent = this._filterDefaults(recent);
       if (_.isEmpty(recent)) {
         return;
       }
@@ -67,6 +69,15 @@ const configureStore = (options = {}) => {
         return r._ns === ns;
       });
 
+      /* Ignore duplicate queries */
+      const existingQuery = this.state.items.find((item) => _.isEqual(comparableQuery(item), recent));
+      if (existingQuery) {
+        // update the existing query's lastExecuted to move it to the top
+        existingQuery._lastExecuted = Date.now();
+        existingQuery.save();
+        return;
+      }
+
       /* Keep length of each recent list to TOTAL_RECENTS */
       if (filtered.length >= TOTAL_RECENTS) {
         const lastRecent = filtered[TOTAL_RECENTS - 1];
@@ -92,14 +103,8 @@ const configureStore = (options = {}) => {
     },
 
     runQuery(query) {
-      // Loosely match queries against known history entries, because
-      // currently we do not distinguish between favorites and recents
-      // when running queries. This way, we do track some queries twice
-      // (because there are more options than just .filter), but that
-      // is probably fine as a temporary measure.
-      // https://jira.mongodb.org/browse/COMPASS-5243
       if (this.state.items.map(item => item.serialize()).some(item => {
-        return isDeepStrictEqual(item.filter, query.filter);
+        return _.isEqual(comparableQuery(item), query);
       })) {
         track('Query History Recent Used');
       }

packages/compass-query-history/test/renderer/recent-list-store.spec.js → packages/compass-query-history/src/stores/recent-list-store.spec.js

@@ -89,4 +89,26 @@ describe('RecentListStore [Store]', () => {
       });
     });
   });
+
+  describe('#addRecent', () => {
+    it('ignores duplicate queries', () => {
+      expect(store.state.items.length).to.equal(0);
+
+      const recent = { ns: 'foo', filter: { foo: 1 } };
+
+      store.addRecent(recent);
+      expect(store.state.items.length).to.equal(1);
+
+      // set _lastExecuted to the epoch so we can check that it gets increased
+      store.state.items.at(0)._lastExecuted = 0;
+      store.state.items.at(0).save();
+      expect(store.state.items.at(0)._lastExecuted.getTime()).to.equal(0);
+
+      store.addRecent(recent);
+      expect(store.state.items.length).to.equal(1);
+
+      // didn't add a duplicate, but did move it to the top
+      expect(store.state.items.at(0)._lastExecuted.getTime()).to.be.gt(0);
+    });
+  });
 });

packages/compass-query-history/src/utils/comparable-query.js

@@ -0,0 +1,13 @@
+function comparableQuery(item) {
+  const query = {};
+  for (const [k, v] of Object.entries(item.serialize())) {
+    if (k.startsWith('_')) {
+      continue;
+    }
+    query[k] = v;
+  }
+  return query;
+}
+
+export default comparableQuery;
+export { comparableQuery };

packages/compass-query-history/src/utils/comparable-query.spec.js

@@ -0,0 +1,32 @@
+import AppRegistry from 'hadron-app-registry';
+import configureStore from '../../src/stores/recent-list-store';
+import { comparableQuery } from './';
+
+describe('comparableQuery', () => {
+  let store;
+  let appRegistry;
+
+  beforeEach(() => {
+    appRegistry = new AppRegistry();
+    store = configureStore({ localAppRegistry: appRegistry });
+  });
+
+  it('strips ampersand properties', () => {
+    const recent = { ns: 'foo', filter: { foo: 1 } };
+
+    store.addRecent(recent);
+    expect(store.state.items.length).to.equal(1);
+
+    const query = store.state.items.at(0);
+
+    // make sure it has the things we're going to strip
+    const serialized = query.serialize();
+    expect(serialized).to.haveOwnProperty('_id');
+    expect(serialized).to.haveOwnProperty('_lastExecuted');
+    expect(serialized).to.haveOwnProperty('_ns');
+
+    expect(comparableQuery(query)).to.deep.equal({
+      filter: { foo: 1 }
+    });
+  });
+});

packages/compass-query-history/src/utils/index.js

@@ -1,4 +1,5 @@
 import formatQuery from './format-query';
+import comparableQuery from './comparable-query';
 import { FavoriteQueryStorage } from './favorite-query-storage';
 
-export { formatQuery, FavoriteQueryStorage };
+export { formatQuery, comparableQuery, FavoriteQueryStorage };