From 013aa2e5706bc9ec7e44f0fd77bf3a591e39c34b Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Mon, 17 Nov 2025 17:06:47 -0500 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Refactored=20sessions=20?= =?UTF-8?q?page=20filters=20to=20load=20separately=20(#3053)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../scopes/scope/session-recordings/index.js | 24 +- .../scopes/scope/sessions/index.js | 90 +++++- .../app/routes/scopes/scope/sessions/index.js | 275 ++++++++---------- .../templates/scopes/scope/sessions/index.hbs | 43 ++- ui/admin/app/utils/filter-options.js | 33 +++ .../tests/acceptance/sessions/list-test.js | 65 +++++ .../scopes/scope/sessions/index-test.js | 41 +-- 7 files changed, 351 insertions(+), 220 deletions(-) create mode 100644 ui/admin/app/utils/filter-options.js diff --git a/ui/admin/app/controllers/scopes/scope/session-recordings/index.js b/ui/admin/app/controllers/scopes/scope/session-recordings/index.js index cde82dc9a..776c7624c 100644 --- a/ui/admin/app/controllers/scopes/scope/session-recordings/index.js +++ b/ui/admin/app/controllers/scopes/scope/session-recordings/index.js @@ -9,29 +9,7 @@ import { action } from '@ember/object'; import { service } from '@ember/service'; import { assert } from '@ember/debug'; import { restartableTask } from 'ember-concurrency'; - -class FilterOptions { - @tracked search; - @tracked _options = []; - #allOptions = new Map(); - - get options() { - return this._options; - } - - set options(newOptions) { - this._options = newOptions; - newOptions.forEach((option) => { - this.#allOptions.set(option.id, option); - }); - } - - // Keep track of all filter options that are loaded so they can be - // displayed in the selected filters regardless of search input - get allOptions() { - return Array.from(this.#allOptions.values()); - } -} +import FilterOptions from 'admin/utils/filter-options'; export default class ScopesScopeSessionRecordingsIndexController extends Controller { // =services diff --git a/ui/admin/app/controllers/scopes/scope/sessions/index.js b/ui/admin/app/controllers/scopes/scope/sessions/index.js index 26791e78c..4100e6c77 100644 --- a/ui/admin/app/controllers/scopes/scope/sessions/index.js +++ b/ui/admin/app/controllers/scopes/scope/sessions/index.js @@ -8,18 +8,22 @@ import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { STATUS_SESSION_ACTIVE, - STATUS_SESSION_PENDING, STATUS_SESSION_CANCELING, + STATUS_SESSION_PENDING, statusTypes, } from 'api/models/session'; import { action } from '@ember/object'; -import { notifySuccess, notifyError } from 'core/decorators/notify'; +import { notifyError, notifySuccess } from 'core/decorators/notify'; +import { restartableTask } from 'ember-concurrency'; +import { assert } from '@ember/debug'; +import FilterOptions from 'admin/utils/filter-options'; export default class ScopesScopeSessionsIndexController extends Controller { // =services @service store; @service intl; + @service db; // =attributes @@ -34,6 +38,9 @@ export default class ScopesScopeSessionsIndexController extends Controller { 'sortDirection', ]; + userFilters = new FilterOptions(); + targetFilters = new FilterOptions(); + @tracked search; @tracked users = []; @tracked targets = []; @@ -54,8 +61,8 @@ export default class ScopesScopeSessionsIndexController extends Controller { get filters() { return { allFilters: { - users: this.model.associatedUsers, - targets: this.model.associatedTargets, + users: this.userFilters.allOptions, + targets: this.targetFilters.allOptions, status: this.sessionStatusOptions, }, selectedFilters: { @@ -77,6 +84,74 @@ export default class ScopesScopeSessionsIndexController extends Controller { })); } + /** + * Configurations for different filter options + * @returns {[object]} + */ + get filterConfigs() { + return { + userFilters: { + resource: 'user', + select: [ + { field: 'id', isDistinct: true }, + { field: 'name', isDistinct: true }, + ], + searchFields: ['id', 'name'], + filters: { + joins: [ + { + resource: 'session', + joinOn: 'user_id', + }, + ], + }, + }, + targetFilters: { + resource: 'target', + select: [ + { field: 'id', isDistinct: true }, + { field: 'name', isDistinct: true }, + ], + searchFields: ['id', 'name'], + filters: { + joins: [ + { + resource: 'session', + joinOn: 'target_id', + }, + ], + }, + }, + }; + } + + /** + * Generic retrieve function for session filter options + * @param {string} type - The type of options to retrieve (userFilters, targetFilters) + * @param {string} search - Search term + * @returns {Promise} + */ + async retrieveFilterOptions(type, search) { + const config = this.filterConfigs[type]; + assert(`Unknown filter type: ${type}`, config); + + return this.db.query(config.resource, { + select: config.select, + query: { + search: { text: search, fields: config.searchFields }, + filters: config.filters, + }, + page: 1, + pageSize: 250, + }); + } + + loadItems = restartableTask(async () => { + this.userFilters.options = await this.retrieveFilterOptions('userFilters'); + this.targetFilters.options = + await this.retrieveFilterOptions('targetFilters'); + }); + // =actions /** @@ -106,7 +181,7 @@ export default class ScopesScopeSessionsIndexController extends Controller { */ @action refresh() { - this.send('refreshAll'); + this.router.refresh('scopes.scope.sessions'); } /** @@ -131,4 +206,9 @@ export default class ScopesScopeSessionsIndexController extends Controller { this.sortDirection = sortOrder; this.page = 1; } + + onFilterSearch = restartableTask(async (filter, value) => { + this[filter].search = value; + this[filter].options = await this.retrieveFilterOptions(filter, value); + }); } diff --git a/ui/admin/app/routes/scopes/scope/sessions/index.js b/ui/admin/app/routes/scopes/scope/sessions/index.js index 0db68e505..159bce03a 100644 --- a/ui/admin/app/routes/scopes/scope/sessions/index.js +++ b/ui/admin/app/routes/scopes/scope/sessions/index.js @@ -5,15 +5,7 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; -import { action } from '@ember/object'; import { restartableTask, timeout } from 'ember-concurrency'; -import chunk from 'lodash/chunk'; - -// Maximum expression tree depth is 1000 so -// we limit the number of expressions in a query. -// See "Maximum Depth Of An Expression Tree" in -// https://www.sqlite.org/limits.html -const MAX_EXPR_NUM = 999; export default class ScopesScopeSessionsIndexRoute extends Route { // =services @@ -61,7 +53,8 @@ export default class ScopesScopeSessionsIndexRoute extends Route { /** * Loads all sessions under the current scope and encapsulates them into * an array of objects containing their associated users and targets. - * @return {Promise<{sessions: SessionModel[], allSessions: SessionModel[], associatedUsers: UserModel[], associatedTargets: TargetModel[], totalItems: number>}>} + * @param {Object} params + * @returns {Promise<{sessions: SessionModel[], doSessionsExist: boolean, associatedUsers: UserModel[], associatedTargets: TargetModel[], totalItems: number}>} */ async model(params) { const useDebounce = @@ -109,173 +102,157 @@ export default class ScopesScopeSessionsIndexRoute extends Route { direction: sortDirection, }; + const searchOptions = { + text: search, + relatedSearches: [ + { + resource: 'target', + fields: ['name'], + join: { + joinFrom: 'target_id', + joinOn: 'id', + }, + }, + { + resource: 'user', + fields: ['name'], + join: { + joinFrom: 'user_id', + joinOn: 'id', + }, + }, + ], + }; + const queryOptions = { scope_id, include_terminated: true, - query: { search, filters, sort }, + query: { search: searchOptions, filters, sort }, page, pageSize, }; + // Preload the associated targets and users into the cache + let refreshUsersPromise, refreshTargetsPromise; + const canListTargets = this.can.can('list model', scope, { + collection: 'targets', + }); + if (canListTargets) { + refreshTargetsPromise = this.store.query( + 'target', + { + scope_id: scope.id, + page: 1, + pageSize: 1, + }, + { pushToStore: false }, + ); + } + + const orgScope = await this.store.findRecord('scope', scope.scope.id); + const globalScope = await this.store.findRecord('scope', 'global'); + const canListUsers = + this.can.can('list model', globalScope, { collection: 'users' }) || + this.can.can('list model', orgScope, { collection: 'users' }); + + if (canListUsers) { + refreshUsersPromise = this.store.query( + 'user', + { + scope_id: 'global', + recursive: true, + page: 1, + pageSize: 1, + }, + { pushToStore: false }, + ); + } + const sessions = await this.store.query('session', queryOptions); const totalItems = sessions.meta?.totalItems; - const allSessions = await this.getAllSessions(scope_id); - const associatedUsers = await this.getAssociatedUsers(scope, allSessions); - const associatedTargets = await this.getAssociatedTargets( - scope, - allSessions, + const doSessionsExist = await this.getDoSessionsExist( + scope_id, + totalItems, ); + const associatedTargetsPromise = this.store.query( + 'target', + { + query: { + filters: { + id: sessions.map((session) => ({ + equals: session.target_id, + })), + }, + }, + }, + { peekDb: true }, + ); + const associatedUsersPromise = this.store.query( + 'user', + { + query: { + filters: { + id: sessions.map((user) => ({ + equals: user.user_id, + })), + }, + }, + }, + { peekDb: true }, + ); + + await Promise.all([ + refreshTargetsPromise, + refreshUsersPromise, + associatedTargetsPromise, + associatedUsersPromise, + ]); + return { sessions, - allSessions, - associatedUsers, - associatedTargets, + doSessionsExist, + canListUsers, + canListTargets, totalItems, }; }, ); /** - * Get all the sessions for a scope id - * @param scope_id - * @returns {Promise} + * Sets doSessionsExist to true if there are any sessions. + * @param {string} scope_id + * @param {number} totalItems + * @returns {Promise} */ - async getAllSessions(scope_id) { - const allSessionsQuery = { - scope_id, - include_terminated: true, - query: { filters: { scope_id: [{ equals: scope_id }] } }, - }; - - return this.store.query('session', allSessionsQuery, { - pushToStore: false, - }); - } - - /** - * Get all the users for a given scope and array of scope's sessions. - * @param scope - * @returns {Promise} - */ - async getAssociatedUsers(scope, allSessions) { - const orgScope = await this.store.findRecord('scope', scope.scope.id); - const globalScope = await this.store.findRecord('scope', 'global'); - - if ( - this.can.can('list model', globalScope, { collection: 'users' }) && - this.can.can('list model', orgScope, { collection: 'users' }) - ) { - const users = await this.store.query( - 'user', - { - scope_id: 'global', - recursive: true, - page: 1, - pageSize: 1, - }, - { pushToStore: false }, - ); - - if (!users.length) { - return []; - } - - const uniqueSessionUserIds = [ - ...new Set( - allSessions - .filter((session) => session.user_id) - .map((session) => session.user_id), - ), - ]; - const chunkedUserIds = chunk(uniqueSessionUserIds, MAX_EXPR_NUM); - const associatedUsersPromises = chunkedUserIds.map((userIds) => - this.store.query( - 'user', - { - scope_id: 'global', - query: { - filters: { - id: { values: userIds.map((userId) => ({ equals: userId })) }, - }, - }, - }, - { peekDb: true }, - ), - ); - const usersArray = await Promise.all(associatedUsersPromises); - return usersArray.flat(); + async getDoSessionsExist(scope_id, totalItems) { + if (totalItems > 0) { + return true; } - - return []; - } - - /** - * Get all the targets for a given scope and array of scope's sessions. - * @param scope - * @returns {Promise} - */ - async getAssociatedTargets(scope, allSessions) { - if (this.can.can('list model', scope, { collection: 'targets' })) { - const targets = await this.store.query( - 'target', - { - scope_id: scope.id, - query: { - filters: { - scope_id: [{ equals: scope.id }], - }, - }, - page: 1, - pageSize: 1, - }, - { pushToStore: false }, - ); - - if (!targets.length) { - return []; - } - - const uniqueSessionTargetIds = [ - ...new Set( - allSessions - .filter((session) => session.target_id) - .map((session) => session.target_id), - ), - ]; - - const chunkedTargetIds = chunk(uniqueSessionTargetIds, MAX_EXPR_NUM); - const associatedTargetsPromises = chunkedTargetIds.map((targetIds) => - this.store.query( - 'target', - { - scope_id: scope.id, - query: { - filters: { - id: { - values: targetIds.map((targetId) => ({ equals: targetId })), - }, - }, - }, - }, - { peekDb: true }, - ), - ); - const targetsArray = await Promise.all(associatedTargetsPromises); - return targetsArray.flat(); - } - - return []; + const options = { pushToStore: false, peekDb: true }; + const sessions = await this.store.query( + 'session', + { + scope_id, + include_terminated: true, + query: { filters: { scope_id: [{ equals: scope_id }] } }, + page: 1, + pageSize: 1, + }, + options, + ); + return sessions.length > 0; } // =actions /** - * refreshes all session route data. + * Loads initial filter options in controller so it happens outside of model hook + * @param controller */ - @action - async refreshAll() { - return super.refresh(...arguments); + setupController(controller) { + super.setupController(...arguments); + controller.loadItems.perform(); } } diff --git a/ui/admin/app/templates/scopes/scope/sessions/index.hbs b/ui/admin/app/templates/scopes/scope/sessions/index.hbs index f6f2dd828..f33dbf94f 100644 --- a/ui/admin/app/templates/scopes/scope/sessions/index.hbs +++ b/ui/admin/app/templates/scopes/scope/sessions/index.hbs @@ -21,7 +21,7 @@ - {{#if @model.allSessions}} + {{#if @model.doSessionsExist}}
- {{#if @model.associatedUsers}} + {{#if @model.canListUsers}} {{#each itemOptions as |itemOption|}} @@ -48,22 +57,42 @@ checked={{includes itemOption.id this.users}} {{on 'change' selectItem}} > - {{itemOption.name}} + {{or itemOption.name itemOption.id}} {{/each}} {{/if}} - {{#if @model.associatedTargets}} + {{#if @model.canListTargets}} + @searchTerm={{this.targetFilters.search}} + @updateSearchTerm={{fn + this.onFilterSearch.perform + 'targetFilters' + }} + @isLoading={{or + this.onFilterSearch.isRunning + this.loadItems.isRunning + }} + as |FD selectItem itemOptions| + > + {{#each itemOptions as |itemOption|}} + + {{itemOption.name}} + + {{/each}} + {{/if}} diff --git a/ui/admin/app/utils/filter-options.js b/ui/admin/app/utils/filter-options.js new file mode 100644 index 000000000..26dcc221a --- /dev/null +++ b/ui/admin/app/utils/filter-options.js @@ -0,0 +1,33 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { tracked } from '@glimmer/tracking'; + +/** + * FilterOptions class to manage filter options state including + * search query and available options with caching for all options. + */ +export default class FilterOptions { + @tracked search; + @tracked _options = []; + #allOptions = new Map(); + + get options() { + return this._options; + } + + set options(newOptions) { + this._options = newOptions; + newOptions.forEach((option) => { + this.#allOptions.set(option.id, option); + }); + } + + // Keep track of all filter options that are loaded so they can be + // displayed in the selected filters regardless of search input + get allOptions() { + return Array.from(this.#allOptions.values()); + } +} diff --git a/ui/admin/tests/acceptance/sessions/list-test.js b/ui/admin/tests/acceptance/sessions/list-test.js index 1bb8deed6..1d99ddbf4 100644 --- a/ui/admin/tests/acceptance/sessions/list-test.js +++ b/ui/admin/tests/acceptance/sessions/list-test.js @@ -297,6 +297,71 @@ module('Acceptance | sessions | list', function (hooks) { assert.dom(selectors.NO_RESULTS_MSG).includesText('No results found'); }); + test('user can search for sessions by associated user name', async function (assert) { + setRunOptions({ + rules: { + 'color-contrast': { + // [ember-a11y-ignore]: axe rule "color-contrast" automatically ignored on 2025-08-01 + enabled: false, + }, + }, + }); + + instances.sessions[0].update({ + userId: instances.admin.id, + }); + instances.sessions[1].update({ + userId: instances.dev.id, + }); + instances.sessions[2].update({ + userId: instances.dev.id, + }); + + await visit(urls.projectScope); + await click(commonSelectors.HREF(urls.sessions)); + await fillIn(commonSelectors.SEARCH_INPUT, 'dev'); + await waitFor(selectors.TABLE_SESSION_ID(instances.sessions[0].id), { + count: 0, + }); + + assert.dom(selectors.TABLE_SESSION_ID(instances.sessions[1].id)).exists(); + assert.dom(selectors.TABLE_SESSION_ID(instances.sessions[2].id)).exists(); + assert.dom(commonSelectors.TABLE_ROWS).exists({ count: 2 }); + }); + + test('user can search for sessions by associated target name', async function (assert) { + setRunOptions({ + rules: { + 'color-contrast': { + // [ember-a11y-ignore]: axe rule "color-contrast" automatically ignored on 2025-08-01 + enabled: false, + }, + }, + }); + + instances.sessions[0].update({ + targetId: instances.tcpTarget.id, + }); + instances.sessions[1].update({ + targetId: instances.sshTarget.id, + }); + instances.sessions[2].update({ + targetId: instances.sshTarget.id, + }); + + await visit(urls.projectScope); + + await click(commonSelectors.HREF(urls.sessions)); + await fillIn(commonSelectors.SEARCH_INPUT, instances.sshTarget.name); + await waitFor(selectors.TABLE_SESSION_ID(instances.sessions[0].id), { + count: 0, + }); + + assert.dom(selectors.TABLE_SESSION_ID(instances.sessions[1].id)).exists(); + assert.dom(selectors.TABLE_SESSION_ID(instances.sessions[2].id)).exists(); + assert.dom(commonSelectors.TABLE_ROWS).exists({ count: 2 }); + }); + test('user can filter for sessions by user', async function (assert) { setRunOptions({ rules: { diff --git a/ui/admin/tests/unit/controllers/scopes/scope/sessions/index-test.js b/ui/admin/tests/unit/controllers/scopes/scope/sessions/index-test.js index b0bc4c3ad..f9f0af5a2 100644 --- a/ui/admin/tests/unit/controllers/scopes/scope/sessions/index-test.js +++ b/ui/admin/tests/unit/controllers/scopes/scope/sessions/index-test.js @@ -17,13 +17,11 @@ module('Unit | Controller | scopes/scope/sessions/index', function (hooks) { status: 'active', created_time: '2024-03-12T20:44:14.808699Z', }; - const user = { id: 'u_123', name: 'admin' }; - const target = { id: 't_123', name: 'target' }; const model = { sessions: [session], - associatedUsers: [user], - associatedTargets: [target], - allSessions: [session], + canListUsers: true, + canListTargets: true, + doSessionsExist: true, totalItems: 1, }; @@ -58,18 +56,8 @@ module('Unit | Controller | scopes/scope/sessions/index', function (hooks) { name: 'Terminated', }, ], - targets: [ - { - id: 't_123', - name: 'target', - }, - ], - users: [ - { - id: 'u_123', - name: 'admin', - }, - ], + targets: [], + users: [], }); assert.deepEqual(controller.filters.selectedFilters, { users: [], @@ -112,23 +100,4 @@ module('Unit | Controller | scopes/scope/sessions/index', function (hooks) { assert.strictEqual(controller.page, 1); assert.deepEqual(controller.users, selectedItems); }); - - test('refresh action calls refreshAll', async function (assert) { - assert.expect(2); - let controller = this.owner.lookup( - 'controller:scopes/scope/sessions/index', - ); - controller.set('target', { - send(actionName, ...args) { - assert.strictEqual(actionName, 'refreshAll'); - assert.deepEqual( - args, - [], - 'refreshAll was called with the correct arguments', - ); - }, - }); - - await controller.refresh(); - }); });