feat: 🎸 Add active session filter (#2052)

* feat: 🎸 Add active session filter

* fix: 🐛 Fix tests and verify client daemon calls are correct

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11803

addons/api/addon-test-support/handlers/client-daemon-search.js

@@ -1,4 +1,5 @@
 import sinon from 'sinon';
+import { typeOf } from '@ember/utils';
 
 /**
  * This test helper can be used to help setup your sinon stubs in your tests.
@@ -23,6 +24,8 @@ import sinon from 'sinon';
  * @public
  */
 export default function setupStubs(hooks) {
+  let stubTypes = [];
+
   hooks.beforeEach(function () {
     const ipcService = this.owner.lookup('service:ipc');
     this.ipcStub = sinon.stub(ipcService, 'invoke');
@@ -42,12 +45,24 @@ export default function setupStubs(hooks) {
      * @param types
      */
     this.stubClientDaemonSearch = (...types) => {
+      stubTypes = types;
+
       types.forEach((type, i) => {
+        let models;
+        let resourceName;
+        if (typeOf(type) === 'object') {
+          models = type.func();
+          resourceName = type.resource;
+        } else {
+          models = this.server.schema[type].all().models;
+          resourceName = type;
+        }
+
         this.ipcStub
           .withArgs('searchClientDaemon')
           .onCall(i)
           .returns({
-            [type]: this.server.schema[type]?.all().models.map((model) => {
+            [resourceName]: models.map((model) => {
               // Use internal serializer to serialize the model correctly
               // according to our mirage serializers
               const modelData =
@@ -63,6 +78,25 @@ export default function setupStubs(hooks) {
   });
 
   hooks.afterEach(function () {
+    // Verify the number of mocks we set up match with how
+    // many times the client daemon was called with
+    sinon.assert.callCount(
+      this.ipcStub.withArgs('searchClientDaemon'),
+      stubTypes.length,
+    );
+
+    stubTypes.forEach((type, i) => {
+      // Assert that the stub was called with the resource we're trying to mock
+      // so we don't mistakenly mock the wrong resource
+      const ipcCall = this.ipcStub.withArgs('searchClientDaemon').getCall(i);
+      let resource = type;
+      if (typeOf(type) === 'object') {
+        resource = type.resource;
+      }
+
+      sinon.assert.match(ipcCall.args[1], { resource });
+    });
+
     sinon.restore();
   });
 }

addons/api/addon/utils/mqlQuery.js

@@ -22,38 +22,45 @@ export function generateMQLFilterExpression(filterObj) {
 
   const expressions = Object.entries(filterObj)
     .map(([key, filterObjValue]) => {
-      const filterObjValueArray = Array.isArray(filterObjValue)
+      const filterValueArray = Array.isArray(filterObjValue)
         ? filterObjValue
-        : [filterObjValue];
+        : filterObjValue.values;
 
-      const orClauses = or(
-        filterObjValueArray.map((filterObjValue) => {
-          if (!filterObjValue) {
-            return null;
-          }
+      let clauses;
+      const filterValues = filterValueArray.map((filterObjValue) => {
+        if (!filterObjValue) {
+          return null;
+        }
 
-          const [operation, value] = Object.entries(filterObjValue)[0];
+        const [operation, value] = Object.entries(filterObjValue)[0];
 
-          switch (operation) {
-            case 'contains':
-              return contains(key, value);
-            case 'gt':
-              return gt(key, value);
-            case 'gte':
-              return gte(key, value);
-            case 'lt':
-              return lt(key, value);
-            case 'lte':
-              return lte(key, value);
-            case 'notEquals':
-              return notEquals(key, value);
-            default:
-              return equals(key, value);
-          }
-        }),
-      );
+        switch (operation) {
+          case 'contains':
+            return contains(key, value);
+          case 'gt':
+            return gt(key, value);
+          case 'gte':
+            return gte(key, value);
+          case 'lt':
+            return lt(key, value);
+          case 'lte':
+            return lte(key, value);
+          case 'notEquals':
+            return notEquals(key, value);
+          default:
+            return equals(key, value);
+        }
+      });
 
-      return parenthetical(orClauses);
+      // Default to `or` if we don't have a logical operator
+      const { logicalOperator } = filterObjValue;
+      if (logicalOperator === 'and') {
+        clauses = and(filterValues);
+      } else {
+        clauses = or(filterValues);
+      }
+
+      return parenthetical(clauses);
     })
     .filter((item) => item);
   return and(expressions);

addons/core/translations/resources/en-us.yaml

@@ -567,6 +567,8 @@ target:
     no-search-results:
       title: No results found
       description: Sorry, we weren't able to find any targets for "{query}".
+    no-filter-results:
+      description: Sorry, we weren’t able to find any targets within the filtered parameters.
   titles:
     new: New Target
     active-sessions: Active sessions

ui/desktop/app/controllers/scopes/scope/projects/targets/index.js

@@ -21,33 +21,53 @@ export default class ScopesScopeProjectsTargetsIndexController extends Controlle
   @service session;
   @service store;
   @service can;
+  @service intl;
 
   // =attributes
 
-  queryParams = ['search', { scopes: { type: 'array' } }, 'page', 'pageSize'];
+  queryParams = [
+    'search',
+    { scopes: { type: 'array' } },
+    { availableSessions: { type: 'array' } },
+    'page',
+    'pageSize',
+  ];
 
   @tracked search;
   @tracked scopes = [];
+  @tracked availableSessions = [];
   @tracked page = 1;
   @tracked pageSize = 10;
   @tracked selectedTarget;
 
   // =methods
 
-  /**
-   * Returns true if model is empty but we have a search term
-   * @returns {boolean}
-   */
-  get noResults() {
-    return this.model.targets.length === 0 && this.search;
+  get showFilters() {
+    return (
+      this.model.targets.length || this.search || this.availableSessions.length
+    );
   }
 
   /**
-   * Returns true if model is empty and we have no search term
+   * Returns true if model is empty but we have a search term or filters
+   * @returns {boolean}
+   */
+  get noResults() {
+    return (
+      this.model.targets.length === 0 &&
+      (this.search || this.availableSessions.length)
+    );
+  }
+
+  /**
+   * Returns true if model is empty and we have no search term or filters
    * @returns {boolean}
    */
   get noTargets() {
-    return this.model.targets.length === 0 && !this.search;
+    return (
+      this.model.targets.length === 0 &&
+      !(this.search || this.availableSessions.length)
+    );
   }
 
   /**
@@ -64,11 +84,19 @@ export default class ScopesScopeProjectsTargetsIndexController extends Controlle
     );
   }
 
+  get availableSessionOptions() {
+    return [
+      { id: 'yes', displayName: this.intl.t('actions.yes') },
+      { id: 'no', displayName: this.intl.t('actions.no') },
+    ];
+  }
+
   /**
-   * Returns active and pending sessions associated with a target.
+   * Returns active and pending sessions associated with a target
+   * sorted descending by created time.
    * @returns {object}
    */
-  get availableSessions() {
+  get sortedTargetSessions() {
     const sessions = this.selectedTarget.sessions.filter(
       (session) => session.isAvailable,
     );
@@ -180,8 +208,9 @@ export default class ScopesScopeProjectsTargetsIndexController extends Controlle
    * @param {function} close
    */
   @action
-  applyFilter(selectedItems) {
-    this.scopes = [...selectedItems];
+  applyFilter(paramKey, selectedItems) {
+    this[paramKey] = [...selectedItems];
+    this.page = 1;
   }
 
   /**

ui/desktop/app/routes/scopes/scope/projects/targets/index.js

@@ -31,6 +31,10 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route {
       refreshModel: true,
       replace: true,
     },
+    availableSessions: {
+      refreshModel: true,
+      replace: true,
+    },
     page: {
       refreshModel: true,
     },
@@ -56,36 +60,23 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route {
    *
    * @returns {Promise<{totalItems: number, targets: [TargetModel], projects: [ScopeModel], allTargets: [TargetModel] }> }
    */
-  async model({ search, scopes, page, pageSize }, transition) {
-    const from = transition.from?.name;
-    // TODO: Filter targets by scope we're in manually
-    // const { id: scope_id } = this.modelFor('scopes.scope');
-    const filters = { scope_id: [] };
+  async model(
+    { search, scopes, availableSessions, page, pageSize },
+    transition,
+  ) {
+    await this.getAllTargets(transition);
+
+    const filters = { scope_id: [], id: { values: [] } };
     scopes.forEach((scope) => {
       filters.scope_id.push({ equals: scope });
     });
 
-    const queryOptions = {
-      query: { search, filters },
-      page,
-      pageSize,
-    };
-    const projects = this.modelFor('scopes.scope.projects');
-
-    let targets = await this.store.query('target', queryOptions);
-    const totalItems = targets.meta?.totalItems;
-
-    // Filter out targets to which users do not have the connect ability
-    targets = targets.filter((target) =>
-      this.can.can('connect target', target),
-    );
-
     // Retrieve all sessions so that the session and activeSessions getters
     // in the target model always retrieve the most up-to-date sessions.
-    await this.store.query('session', {
+    const sessions = await this.store.query('session', {
       query: {
         filters: {
-          user_id: { equals: this.session.data.authenticated.user_id },
+          user_id: [{ equals: this.session.data.authenticated.user_id }],
           status: [
             { equals: STATUS_SESSION_ACTIVE },
             { equals: STATUS_SESSION_PENDING },
@@ -95,6 +86,41 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route {
       force_refresh: true,
     });
 
+    this.addActiveSessionFilters(filters, availableSessions, sessions);
+
+    let targets = await this.store.query('target', {
+      query: { search, filters },
+      page,
+      pageSize,
+      force_refresh: true,
+    });
+    const totalItems = targets.meta?.totalItems;
+
+    // TODO: Filter targets by scope we're in manually
+    // const { id: scope_id } = this.modelFor('scopes.scope');
+    // Filter out targets to which users do not have the connect ability
+    targets = targets.filter((target) =>
+      this.can.can('connect target', target),
+    );
+    const projects = this.modelFor('scopes.scope.projects');
+
+    return {
+      targets,
+      projects,
+      allTargets: this.allTargets,
+      totalItems,
+    };
+  }
+
+  /**
+   * Get all the targets but only load them once when entering the route. Ideally we would lazy load it when needed
+   * but this can be revisited in the future.
+   * @param transition
+   * @returns {Promise<void>}
+   */
+  async getAllTargets(transition) {
+    const from = transition.from?.name;
+
     // Query all targets for defining filtering values if entering route for first time
     if (from !== 'scopes.scope.projects.targets.index') {
       const options = { pushToStore: false };
@@ -105,12 +131,45 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route {
         target.authorized_actions.includes('authorize-session'),
       );
     }
-
-    return {
-      targets,
-      projects,
-      allTargets: this.allTargets,
-      totalItems,
-    };
   }
+
+  /**
+   * Add the filters for active sessions to the filter object.
+   * @param filters
+   * @param availableSessions
+   * @param sessions
+   */
+  addActiveSessionFilters = (filters, availableSessions, sessions) => {
+    const uniqueTargetIdsWithSessions = new Set(
+      sessions.map((session) => session.target_id),
+    );
+
+    // Don't add any filtering if the user selects both which is equivalent to no filters
+    if (availableSessions.length === 2) {
+      return;
+    }
+
+    availableSessions.forEach((availability) => {
+      if (availability === 'yes') {
+        filters.id.logicalOperator = 'or';
+        uniqueTargetIdsWithSessions.forEach((targetId) => {
+          filters.id.values.push({ equals: targetId });
+        });
+
+        // If there's no sessions just set it to a dummy value
+        // so the search returns no results
+        if (uniqueTargetIdsWithSessions.size === 0) {
+          filters.id.values.push({ equals: 'none' });
+        }
+      }
+
+      if (availability === 'no') {
+        filters.id.logicalOperator = 'and';
+
+        uniqueTargetIdsWithSessions.forEach((targetId) => {
+          filters.id.values.push({ notEquals: targetId });
+        });
+      }
+    });
+  };
 }

ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs

@@ -14,7 +14,7 @@
   </page.header>
 
   <page.body class='search-filtering'>
-    {{#if (or @model.targets this.search)}}
+    {{#if this.showFilters}}
       <Hds::SegmentedGroup as |S|>
         <S.TextInput
           @value={{this.search}}
@@ -28,7 +28,7 @@
             @name={{t 'resources.scope.title'}}
             @itemOptions={{this.availableScopes}}
             @checkedItems={{this.scopes}}
-            @applyFilter={{this.applyFilter}}
+            @applyFilter={{fn this.applyFilter 'scopes'}}
             @isSearchable={{true}}
             as |FD selectItem itemOptions|
           >
@@ -52,16 +52,15 @@
             {{/let}}
           </Dropdown>
         </S.Generic>
-        <S.Dropdown as |DD|>
-          <DD.ToggleButton @color='secondary' @text='Active session' />
-          <DD.Checkbox>{{t 'actions.yes'}}</DD.Checkbox>
-          <DD.Checkbox>{{t 'actions.no'}}</DD.Checkbox>
-        </S.Dropdown>
-        <S.Dropdown as |DD|>
-          <DD.ToggleButton @color='secondary' @text='Type' @count='2' />
-          <DD.Checkbox>{{t 'resources.target.types.tcp'}}</DD.Checkbox>
-          <DD.Checkbox>{{t 'resources.target.types.ssh'}}</DD.Checkbox>
-        </S.Dropdown>
+        <S.Generic>
+          <Dropdown
+            @name={{t 'resources.target.titles.active-sessions'}}
+            @itemOptions={{this.availableSessionOptions}}
+            @checkedItems={{this.availableSessions}}
+            @applyFilter={{fn this.applyFilter 'availableSessions'}}
+          />
+        </S.Generic>
+        {{! TODO: Target type filter}}
       </Hds::SegmentedGroup>
       {{#if @model.targets}}
         <Hds::Table
@@ -176,7 +175,11 @@
           />
           <A.Body
             @text={{t
-              'resources.target.messages.no-search-results.description'
+              (if
+                this.search
+                'resources.target.messages.no-search-results.description'
+                'resources.target.messages.no-filter-results.description'
+              )
               query=this.search
             }}
           />
@@ -204,7 +207,7 @@
         </M.Header>
         <M.Body>
           <Hds::Table
-            @model={{this.availableSessions}}
+            @model={{this.sortedTargetSessions}}
             @columns={{array
               (hash label=(t 'form.id.label'))
               (hash label=(t 'form.started.label'))

ui/desktop/tests/acceptance/projects/targets/index-test.js

@@ -143,6 +143,7 @@ module('Acceptance | projects | targets | index', function (hooks) {
 
   test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) {
     invalidateSession();
+    this.stubClientDaemonSearch();
     await visit(urls.targets);
     await a11yAudit();
 
@@ -175,7 +176,8 @@ module('Acceptance | projects | targets | index', function (hooks) {
 
   test('visiting targets list view with no targets', async function (assert) {
     this.server.db.targets.remove();
-    this.stubClientDaemonSearch('targets', 'targets');
+    this.server.db.sessions.remove();
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
 
     await visit(urls.projects);
 
@@ -187,7 +189,7 @@ module('Acceptance | projects | targets | index', function (hooks) {
   test('user cannot navigate to a target without proper authorization', async function (assert) {
     instances.target.authorized_actions =
       instances.target.authorized_actions.filter((item) => item !== 'read');
-    this.stubClientDaemonSearch('targets', 'targets');
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
 
     await visit(urls.projects);
 
@@ -216,7 +218,7 @@ module('Acceptance | projects | targets | index', function (hooks) {
       instances.target.authorized_actions.filter(
         (item) => item !== 'authorize-session',
       );
-    this.stubClientDaemonSearch('targets', 'targets');
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
 
     await visit(urls.projects);
 
@@ -249,7 +251,7 @@ module('Acceptance | projects | targets | index', function (hooks) {
   test('user can connect without target read permissions', async function (assert) {
     instances.target.authorized_actions =
       instances.target.authorized_actions.filter((item) => item !== 'read');
-    this.stubClientDaemonSearch('targets');
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
     this.ipcStub.withArgs('cliExists').returns(true);
     this.ipcStub.withArgs('connect').returns({
       session_id: instances.session.id,
@@ -275,7 +277,7 @@ module('Acceptance | projects | targets | index', function (hooks) {
       instances.target.authorized_actions.filter((item) => item !== 'read');
     this.ipcStub.withArgs('cliExists').returns(true);
     this.ipcStub.withArgs('connect').rejects();
-    this.stubClientDaemonSearch('targets', 'targets');
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
     const confirmService = this.owner.lookup('service:confirm');
     confirmService.enabled = true;
     await visit(urls.projects);
@@ -319,8 +321,10 @@ module('Acceptance | projects | targets | index', function (hooks) {
       'targets',
       'sessions',
       'targets',
-      'targets',
-      '',
+      {
+        resource: 'sessions',
+        func: () => [],
+      },
       'targets',
     );
     await visit(urls.projects);

ui/desktop/tests/acceptance/projects/targets/target-test.js

@@ -124,13 +124,7 @@ module('Acceptance | projects | targets | target', function (hooks) {
     setDefaultClusterUrl(this);
 
     this.ipcStub.withArgs('isClientDaemonRunning').returns(true);
-    this.stubClientDaemonSearch(
-      'targets',
-      'sessions',
-      'sessions',
-      'sessions',
-      'targets',
-    );
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
   });
 
   test('user can connect to a target with an address', async function (assert) {
@@ -142,6 +136,7 @@ module('Acceptance | projects | targets | target', function (hooks) {
       port: 'p_123',
       protocol: 'tcp',
     });
+    this.stubClientDaemonSearch();
 
     await visit(urls.target);
 
@@ -189,6 +184,7 @@ module('Acceptance | projects | targets | target', function (hooks) {
   test('user can retry on error', async function (assert) {
     assert.expect(1);
     this.ipcStub.withArgs('cliExists').rejects();
+    this.stubClientDaemonSearch();
     const confirmService = this.owner.lookup('service:confirm');
     confirmService.enabled = true;
 

ui/desktop/tests/acceptance/scopes-test.js

@@ -129,7 +129,7 @@ module('Acceptance | scopes', function (hooks) {
     setDefaultClusterUrl(this);
 
     this.ipcStub.withArgs('isClientDaemonRunning').returns(true);
-    this.stubClientDaemonSearch('targets', 'targets', 'sessions');
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
   });
 
   test('visiting index', async function (assert) {
@@ -182,21 +182,14 @@ module('Acceptance | scopes', function (hooks) {
     assert.expect(3);
     this.stubClientDaemonSearch(
       'targets',
-      'targets',
-      'targets',
-      'targets',
-      'targets',
-      'targets',
-      'targets',
+      'sessions',
       'targets',
       'sessions',
+      'targets',
       'sessions',
+      'targets',
       'sessions',
-      'sessions',
-      'sessions',
-      'sessions',
-      'sessions',
-      'sessions',
+      'targets',
     );
     await visit(urls.targets);
 
@@ -213,6 +206,7 @@ module('Acceptance | scopes', function (hooks) {
   test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) {
     invalidateSession();
     assert.expect(2);
+    this.stubClientDaemonSearch();
 
     await visit(urls.targets);
     await a11yAudit();
@@ -233,7 +227,8 @@ module('Acceptance | scopes', function (hooks) {
   test('visiting empty targets', async function (assert) {
     assert.expect(1);
     this.server.db.targets.remove();
-    this.stubClientDaemonSearch('targets', 'targets');
+    this.server.db.sessions.remove();
+    this.stubClientDaemonSearch('targets', 'sessions', 'targets');
 
     await visit(urls.targets);