From d414df4ca4f3c6a0295f561410f0cee4249de610 Mon Sep 17 00:00:00 2001 From: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> Date: Mon, 18 Aug 2025 13:39:43 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Add=20custom=20sort=20su?= =?UTF-8?q?pport=20to=20sql=20parser=20(#2959)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: 🎸 Add custom sort support to sql parser * test: 💍 fix column numner * refactor: 💡 apply pr feedback * fix: 🐛 use every method for checking attrs * refactor: 💡 PR feedback + ensure default sort * refactor: 💡 Consolidate attributes field when sorting ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-17441 --------- Co-authored-by: Zhihe Li --- addons/api/addon/services/sqlite.js | 7 ++ addons/api/addon/utils/sort-results.js | 49 ++++++++-- addons/api/addon/utils/sqlite-query.js | 59 +++++++++--- addons/api/addon/workers/utils/schema.js | 16 +++- .../unit/handlers/indexed-db-handler-test.js | 37 +++++--- .../unit/handlers/sqlite-handler-test.js | 6 +- .../api/tests/unit/utils/sort-results-test.js | 67 +++++++------ .../api/tests/unit/utils/sqlite-query-test.js | 95 +++++++++++++++---- .../app/routes/scopes/scope/aliases/index.js | 2 +- .../routes/scopes/scope/auth-methods/index.js | 9 +- .../scopes/scope/credential-stores/index.js | 9 +- .../app/routes/scopes/scope/groups/index.js | 9 +- .../scopes/scope/host-catalogs/index.js | 9 +- .../app/routes/scopes/scope/roles/index.js | 29 ++---- .../app/routes/scopes/scope/scopes/index.js | 9 +- .../scopes/scope/session-recordings/index.js | 25 +++-- .../app/routes/scopes/scope/sessions/index.js | 2 +- .../app/routes/scopes/scope/targets/index.js | 23 +++-- .../app/routes/scopes/scope/users/index.js | 9 +- .../templates/scopes/scope/roles/index.hbs | 5 +- .../app/utils/sort-name-with-id-fallback.js | 12 --- .../tests/acceptance/groups/create-test.js | 3 + ui/admin/tests/acceptance/groups/list-test.js | 2 + ui/admin/tests/acceptance/roles/list-test.js | 8 -- .../tests/acceptance/roles/principals-test.js | 2 + .../tests/acceptance/targets/list-test.js | 19 ++-- .../scopes/scope/groups/index-test.js | 2 + .../scope/roles/role/add-principals-test.js | 2 + .../utils/sort-name-with-id-fallback-test.js | 67 ------------- 29 files changed, 333 insertions(+), 260 deletions(-) delete mode 100644 ui/admin/app/utils/sort-name-with-id-fallback.js delete mode 100644 ui/admin/tests/unit/utils/sort-name-with-id-fallback-test.js diff --git a/addons/api/addon/services/sqlite.js b/addons/api/addon/services/sqlite.js index acee1f96e..310e9536d 100644 --- a/addons/api/addon/services/sqlite.js +++ b/addons/api/addon/services/sqlite.js @@ -28,6 +28,13 @@ export const modelMapping = { scope_id: 'scope.scope_id', created_time: 'created_time', }, + group: { + id: 'id', + name: 'name', + description: 'description', + scope_id: 'scope.scope_id', + created_time: 'created_time', + }, }; // A list of tables that we support searching using FTS5 in SQLite. diff --git a/addons/api/addon/utils/sort-results.js b/addons/api/addon/utils/sort-results.js index a59dc94ea..39040cac3 100644 --- a/addons/api/addon/utils/sort-results.js +++ b/addons/api/addon/utils/sort-results.js @@ -41,11 +41,11 @@ function getSortableValue(schema, record, attribute) { export const sortResults = (results, { querySort, schema }) => { querySort = querySort ?? {}; - const sortAttribute = querySort.attribute || SORT_DEFAULT_ATTRIBUTE; + const sortAttribute = querySort.attributes?.[0] || SORT_DEFAULT_ATTRIBUTE; // Default sort direction is ascending unless we are sorting by `created_time` (default sort attribute) const defaultSortDirection = - sortAttribute === SORT_DEFAULT_ATTRIBUTE && !querySort.sortFunction + sortAttribute === SORT_DEFAULT_ATTRIBUTE ? SORT_DIRECTION_DESCENDING : SORT_DIRECTION_ASCENDING; const sortDirection = querySort.direction || defaultSortDirection; @@ -58,10 +58,44 @@ export const sortResults = (results, { querySort, schema }) => { throw new Error('Invalid sort direction'); } - // Execute custom sort function if one is provided. - if (querySort.sortFunction) { + const sortAttributeDataType = schema.attributes.get(sortAttribute)?.type; + + const sortFunction = + sortFunctions[sortAttributeDataType] ?? sortFunctions.string; + + // Execute custom sort for multiple attributes provided. + if (querySort.attributes?.length > 1) { + const { attributes } = querySort; return results.toSorted((a, b) => { - const sortResult = querySort.sortFunction(a, b); + let sortValueA, sortValueB; + + attributes.forEach((attr) => { + const valueA = getSortableValue(schema, a, attr); + const valueB = getSortableValue(schema, b, attr); + + if (valueA && !sortValueA) { + sortValueA = valueA; + } + if (valueB && !sortValueB) { + sortValueB = valueB; + } + }); + const sortResult = sortFunction(sortValueA, sortValueB); + return sortDirection === SORT_DIRECTION_ASCENDING + ? sortResult + : -1 * sortResult; + }); + } + // Execute custom sort for attribute map provided. + if (querySort.customSort) { + const { attributeMap } = querySort.customSort; + return results.toSorted((a, b) => { + const sortValueA = getSortableValue(schema, a, sortAttribute); + const sortValueB = getSortableValue(schema, b, sortAttribute); + const sortResult = sortFunction( + attributeMap[sortValueA], + attributeMap[sortValueB], + ); return sortDirection === SORT_DIRECTION_ASCENDING ? sortResult @@ -69,11 +103,6 @@ export const sortResults = (results, { querySort, schema }) => { }); } - const sortAttributeDataType = schema.attributes.get(sortAttribute)?.type; - - const sortFunction = - sortFunctions[sortAttributeDataType] ?? sortFunctions.string; - return results.toSorted((a, b) => { // Extract the values to sort and sort them. const sortValueA = getSortableValue(schema, a, sortAttribute); diff --git a/addons/api/addon/utils/sqlite-query.js b/addons/api/addon/utils/sqlite-query.js index 55cf20818..5747add6d 100644 --- a/addons/api/addon/utils/sqlite-query.js +++ b/addons/api/addon/utils/sqlite-query.js @@ -33,18 +33,7 @@ export function generateSQLExpressions( addFilterConditions(filters, parameters, conditions); addSearchConditions(search, resource, parameters, conditions); - const { attribute, direction } = sort; - let orderByClause = ''; - if (attribute) { - // We have to check if the attribute is valid for the resource - // as we can't use parameterized queries for ORDER BY - const validAttributes = Object.keys(modelMapping[resource]); - if (validAttributes.includes(attribute)) { - orderByClause = `ORDER BY ${attribute} ${direction === 'desc' ? 'DESC' : 'ASC'}`; - } - } else if (modelMapping[resource]?.created_time) { - orderByClause = `ORDER BY created_time DESC`; - } + const orderByClause = constructOrderByClause(resource, sort); const whereClause = conditions.length ? `WHERE ${and(conditions)}` : ''; @@ -53,7 +42,7 @@ export function generateSQLExpressions( parameters.push(pageSize, (page - 1) * pageSize); } - const selectClause = `SELECT ${select ? select.join(', ') : '*'} FROM ${resource}`; + const selectClause = `SELECT ${select ? select.join(', ') : '*'} FROM "${resource}"`; return { // Replace any empty newlines or leading whitespace on each line to be consistent with formatting @@ -145,6 +134,50 @@ function addSearchConditions(search, resource, parameters, conditions) { conditions.push(searchConditions); } +function constructOrderByClause(resource, sort) { + const defaultOrderByClause = 'ORDER BY created_time DESC'; + + const { attributes, customSort, direction, isCoalesced } = sort; + const sortDirection = direction === 'desc' ? 'DESC' : 'ASC'; + + // We have to check if the attributes are valid for the resource + // as we can't use parameterized queries for ORDER BY + const validAttributes = Object.keys(modelMapping[resource] ?? {}); + if (attributes?.some((attr) => !validAttributes.includes(attr))) { + return modelMapping[resource]?.created_time ? defaultOrderByClause : ''; + } + + if (customSort?.attributeMap) { + const whenClauses = Object.keys(customSort.attributeMap).reduce( + (acc, key) => { + return acc + `WHEN '${key}' THEN '${customSort.attributeMap[key]}' `; + }, + '', + ); + return `ORDER BY CASE ${attributes.join(', ')} ${whenClauses}END ${sortDirection}`; + } else if (attributes?.length > 0) { + const commaSeparatedVals = attributes.join(', '); + + // In places where `collate nocase` is used, it is to ensure case is ignored on the initial sort. + // Then, a sort on the same condition is performed to ensure upper-case strings are given preference in a tie. + if (isCoalesced) { + return `ORDER BY COALESCE(${attributes.join(', ')}) COLLATE NOCASE ${sortDirection}, COALESCE(${commaSeparatedVals}) ${sortDirection}`; + } + + const attributesWithNoCollate = attributes + .map((attr) => `${attr} COLLATE NOCASE ${sortDirection}`) + .join(', '); + const attributesWithDirection = attributes + .map((attr) => `${attr} ${sortDirection}`) + .join(', '); + return `ORDER BY ${attributesWithNoCollate}, ${attributesWithDirection}`; + } else if (modelMapping[resource]?.created_time) { + return defaultOrderByClause; + } else { + return ''; + } +} + // Comparison Operators const OPERATORS = { equals: ' = ?', diff --git a/addons/api/addon/workers/utils/schema.js b/addons/api/addon/workers/utils/schema.js index c11e3d0aa..8dd749715 100644 --- a/addons/api/addon/workers/utils/schema.js +++ b/addons/api/addon/workers/utils/schema.js @@ -14,7 +14,7 @@ const createTargetTables = ` CREATE TABLE IF NOT EXISTS target ( id TEXT NOT NULL PRIMARY KEY, type TEXT NOT NULL, - name TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, description TEXT, address TEXT, scope_id TEXT NOT NULL, @@ -105,6 +105,16 @@ CREATE TABLE IF NOT EXISTS token ( ${createTargetTables} ${createAliasTables} +CREATE TABLE IF NOT EXISTS "group" ( + id TEXT NOT NULL PRIMARY KEY, + name TEXT, + description TEXT, + scope_id TEXT NOT NULL, + created_time TEXT NOT NULL, + data TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_group_scope_id_created_time ON "group"(scope_id, created_time DESC); + COMMIT;`; export const INSERT_STATEMENTS = (resource, items, modelMapping) => { @@ -121,8 +131,8 @@ export const INSERT_STATEMENTS = (resource, items, modelMapping) => { const numColumns = Array(columns.length + 1); const placeholders = `(${numColumns.fill('?').join(', ')})`; - return `REPLACE INTO ${resource} (${columns.join(', ')}, data) VALUES ${items.map(() => placeholders).join(', ')};`; + return `REPLACE INTO "${resource}" (${columns.join(', ')}, data) VALUES ${items.map(() => placeholders).join(', ')};`; }; export const DELETE_STATEMENT = (resource, ids) => - `DELETE FROM ${resource} WHERE id IN (${ids.map(() => '?').join(',')})`; + `DELETE FROM "${resource}" WHERE id IN (${ids.map(() => '?').join(',')})`; diff --git a/addons/api/tests/unit/handlers/indexed-db-handler-test.js b/addons/api/tests/unit/handlers/indexed-db-handler-test.js index 3723ba9b4..38c446b4d 100644 --- a/addons/api/tests/unit/handlers/indexed-db-handler-test.js +++ b/addons/api/tests/unit/handlers/indexed-db-handler-test.js @@ -262,7 +262,7 @@ module('Unit | Handler | indexed-db-handler', function (hooks) { ); const results = await store.query('target', { - query: { sort: { attribute: 'name' } }, + query: { sort: { attributes: ['name'] } }, }); assert.deepEqual( @@ -292,10 +292,10 @@ module('Unit | Handler | indexed-db-handler', function (hooks) { ); const resultsDesc = await store.query('alias', { - query: { sort: { attribute: 'name', direction: 'desc' } }, + query: { sort: { attributes: ['name'], direction: 'desc' } }, }); const resultsAsc = await store.query('alias', { - query: { sort: { attribute: 'name', direction: 'asc' } }, + query: { sort: { attributes: ['name'], direction: 'asc' } }, }); assert.deepEqual( resultsDesc.map(({ name }) => name), @@ -313,16 +313,11 @@ module('Unit | Handler | indexed-db-handler', function (hooks) { const alias3 = this.server.create('alias', { name: 'alias3' }); const alias4 = this.server.create('alias', { name: 'alias4' }); - const sortName = (recordA, recordB) => { - const nameMap = { - alias3: 'Alpha', - alias4: 'Beta', - alias1: 'Delta', - alias2: 'Epsilon', - }; - return String(nameMap[recordA.attributes.name]).localeCompare( - String(nameMap[recordB.attributes.name]), - ); + const nameMap = { + alias3: 'Alpha', + alias4: 'Beta', + alias1: 'Delta', + alias2: 'Epsilon', }; const shuffledAliases = faker.helpers.shuffle([ @@ -340,10 +335,22 @@ module('Unit | Handler | indexed-db-handler', function (hooks) { ); const resultsDesc = await store.query('alias', { - query: { sort: { sortFunction: sortName, direction: 'desc' } }, + query: { + sort: { + attributes: ['name'], + customSort: { attributeMap: nameMap }, + direction: 'desc', + }, + }, }); const resultsAsc = await store.query('alias', { - query: { sort: { sortFunction: sortName, direction: 'asc' } }, + query: { + sort: { + attributes: ['name'], + customSort: { attributeMap: nameMap }, + direction: 'asc', + }, + }, }); assert.deepEqual( resultsDesc.map(({ name }) => name), diff --git a/addons/api/tests/unit/handlers/sqlite-handler-test.js b/addons/api/tests/unit/handlers/sqlite-handler-test.js index 0dd8273ce..3b3f17dd0 100644 --- a/addons/api/tests/unit/handlers/sqlite-handler-test.js +++ b/addons/api/tests/unit/handlers/sqlite-handler-test.js @@ -303,7 +303,7 @@ module('Unit | Handler | sqlite-handler', function (hooks) { ); const results = await store.query('target', { - query: { sort: { attribute: 'name' } }, + query: { sort: { attributes: ['name'] } }, }); assert.deepEqual( @@ -345,10 +345,10 @@ module('Unit | Handler | sqlite-handler', function (hooks) { ); const resultsDesc = await store.query('target', { - query: { sort: { attribute: 'name', direction: 'desc' } }, + query: { sort: { attributes: ['name'], direction: 'desc' } }, }); const resultsAsc = await store.query('target', { - query: { sort: { attribute: 'name', direction: 'asc' } }, + query: { sort: { attributes: ['name'], direction: 'asc' } }, }); assert.deepEqual( resultsDesc.map(({ name }) => name), diff --git a/addons/api/tests/unit/utils/sort-results-test.js b/addons/api/tests/unit/utils/sort-results-test.js index 364758dfd..1bc1aee15 100644 --- a/addons/api/tests/unit/utils/sort-results-test.js +++ b/addons/api/tests/unit/utils/sort-results-test.js @@ -37,7 +37,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, ]); - const querySort = { attribute: 'name' }; + const querySort = { attributes: ['name'] }; // Note: `name` is not defined as a schema attribute but exists on the record const schema = { attributes: new Map() }; @@ -55,7 +55,7 @@ module('Unit | Utility | sortResults', function (hooks) { }); const querySort = { - attribute: 'name', + attributes: ['name'], direction: 'invalid sort direction', }; const schema = { attributes: new Map() }; @@ -119,7 +119,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySort = { attribute: 'id' }; + const querySort = { attributes: ['id'] }; const schema = { attributes: new Map() }; schema.attributes.set('id', { type: 'string' }); const sortedResults = sortResults(shuffledResults, { querySort, schema }); @@ -152,7 +152,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySort = { attribute: 'name' }; + const querySort = { attributes: ['name'] }; const schema = { attributes: new Map() }; schema.attributes.set('name', { type: 'string' }); const sortedResults = sortResults(shuffledResults, { querySort, schema }); @@ -182,8 +182,8 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySortAsc = { attribute: 'name', direction: 'asc' }; - const querySortDesc = { attribute: 'name', direction: 'desc' }; + const querySortAsc = { attributes: ['name'], direction: 'asc' }; + const querySortDesc = { attributes: ['name'], direction: 'desc' }; const schema = { attributes: new Map() }; schema.attributes.set('name', { type: 'string' }); const sortedResultsAsc = sortResults(shuffledResults, { @@ -227,8 +227,8 @@ module('Unit | Utility | sortResults', function (hooks) { result04, result05, ]); - const querySortAsc = { attribute: 'name', direction: 'asc' }; - const querySortDesc = { attribute: 'name', direction: 'desc' }; + const querySortAsc = { attributes: ['name'], direction: 'asc' }; + const querySortDesc = { attributes: ['name'], direction: 'desc' }; const schema = { attributes: new Map() }; schema.attributes.set('name', { type: 'string' }); @@ -272,7 +272,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySort = { attribute: 'date' }; + const querySort = { attributes: ['date'] }; const schema = { attributes: new Map() }; schema.attributes.set('date', { type: 'date' }); @@ -304,8 +304,8 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySortAsc = { attribute: 'date', direction: 'asc' }; - const querySortDesc = { attribute: 'date', direction: 'desc' }; + const querySortAsc = { attributes: ['date'], direction: 'asc' }; + const querySortDesc = { attributes: ['date'], direction: 'desc' }; const schema = { attributes: new Map() }; schema.attributes.set('date', { type: 'date' }); @@ -350,7 +350,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySort = { attribute: 'sessions' }; + const querySort = { attributes: ['sessions'] }; const schema = { attributes: new Map() }; schema.attributes.set('sessions', { type: 'number' }); @@ -382,8 +382,8 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySortAsc = { attribute: 'sessions', direction: 'asc' }; - const querySortDesc = { attribute: 'sessions', direction: 'desc' }; + const querySortAsc = { attributes: ['sessions'], direction: 'asc' }; + const querySortDesc = { attributes: ['sessions'], direction: 'desc' }; const schema = { attributes: new Map() }; schema.attributes.set('sessions', { type: 'number' }); @@ -419,7 +419,7 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySort = { attribute: 'active' }; + const querySort = { attributes: ['active'] }; const schema = { attributes: new Map() }; schema.attributes.set('active', { type: 'boolean' }); @@ -442,8 +442,8 @@ module('Unit | Utility | sortResults', function (hooks) { result03, result04, ]); - const querySortAsc = { attribute: 'active', direction: 'asc' }; - const querySortDesc = { attribute: 'active', direction: 'desc' }; + const querySortAsc = { attributes: ['active'], direction: 'asc' }; + const querySortDesc = { attributes: ['active'], direction: 'desc' }; const schema = { attributes: new Map() }; schema.attributes.set('active', { type: 'boolean' }); @@ -482,32 +482,39 @@ module('Unit | Utility | sortResults', function (hooks) { name: 'target4', }); - const sortFunction = (recordA, recordB) => { - const nameMap = { - target3: 'Alpha', - target4: 'Beta', - target1: 'Delta', - target2: 'Epsilon', - }; - return String(nameMap[recordA.attributes.name]).localeCompare( - String(nameMap[recordB.attributes.name]), - ); + const nameMap = { + target3: 'Alpha', + target4: 'Beta', + target1: 'Delta', + target2: 'Epsilon', }; + const schema = { attributes: new Map() }; test.each( 'it sorts by `name` using custom sorting function', { 'ascending (default)': { - querySort: { sortFunction }, + querySort: { + attributes: ['name'], + customSort: { attributeMap: nameMap }, + }, expectedResults: ['target3', 'target4', 'target1', 'target2'], }, descending: { - querySort: { sortFunction, direction: 'desc' }, + querySort: { + attributes: ['name'], + customSort: { attributeMap: nameMap }, + direction: 'desc', + }, expectedResults: ['target2', 'target1', 'target4', 'target3'], }, ascending: { - querySort: { sortFunction, direction: 'asc' }, + querySort: { + attributes: ['name'], + customSort: { attributeMap: nameMap }, + direction: 'asc', + }, expectedResults: ['target3', 'target4', 'target1', 'target2'], }, }, diff --git a/addons/api/tests/unit/utils/sqlite-query-test.js b/addons/api/tests/unit/utils/sqlite-query-test.js index 86c110b9e..cac353217 100644 --- a/addons/api/tests/unit/utils/sqlite-query-test.js +++ b/addons/api/tests/unit/utils/sqlite-query-test.js @@ -28,7 +28,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT * FROM target + SELECT * FROM "target" WHERE rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) ORDER BY created_time DESC`.removeExtraWhiteSpace(), ); @@ -44,7 +44,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT * FROM token + SELECT * FROM "token" WHERE (id = ?)`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, ['tokenKey']); @@ -59,8 +59,8 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT count(*) as total FROM target - ORDER BY created_time DESC`.removeExtraWhiteSpace(), + SELECT count(*) as total FROM "target" + ORDER BY created_time DESC`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, []); }); @@ -160,7 +160,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT * FROM target + SELECT * FROM "target" ${expectedWhereClause} ORDER BY created_time DESC`.removeExtraWhiteSpace(), ); @@ -176,15 +176,55 @@ module('Unit | Utility | sqlite-query', function (hooks) { ], function (assert, [direction, expectedDirection]) { const query = { - sort: { attribute: 'name', direction }, + sort: { attributes: ['name'], direction }, }; const { sql, parameters } = generateSQLExpressions('target', query); assert.strictEqual( sql, ` - SELECT * FROM target - ORDER BY name ${expectedDirection}`.removeExtraWhiteSpace(), + SELECT * FROM "target" + ORDER BY name COLLATE NOCASE ${expectedDirection}, name ${expectedDirection}`.removeExtraWhiteSpace(), + ); + assert.deepEqual(parameters, []); + }, + ); + + test.each( + 'it generates sort order clause correctly', + { + 'sort on multiple attributes and coalesced': { + sort: { + attributes: ['name', 'id'], + isCoalesced: true, + }, + expectedOrderByClause: `ORDER BY COALESCE(name, id) COLLATE NOCASE DESC, COALESCE(name, id) DESC`, + }, + 'sort on multiple attributes': { + sort: { + attributes: ['name', 'id'], + }, + expectedOrderByClause: `ORDER BY name COLLATE NOCASE DESC, id COLLATE NOCASE DESC, name DESC, id DESC`, + }, + 'sort on mapped attributes': { + sort: { + attributes: ['type'], + customSort: { attributeMap: { ssh: 'SSH', tcp: 'Generic TCP' } }, + }, + expectedOrderByClause: `ORDER BY CASE type WHEN 'ssh' THEN 'SSH' WHEN 'tcp' THEN 'Generic TCP' END DESC`, + }, + }, + function (assert, { sort, expectedOrderByClause }) { + const query = { + sort: { ...sort, direction: 'desc' }, + }; + + const { sql, parameters } = generateSQLExpressions('target', query); + assert.strictEqual( + sql, + ` + SELECT * FROM "target" + ${expectedOrderByClause}`.removeExtraWhiteSpace(), ); assert.deepEqual(parameters, []); }, @@ -206,7 +246,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT * FROM target + SELECT * FROM "target" ORDER BY created_time DESC LIMIT ? OFFSET ?`.removeExtraWhiteSpace(), ); @@ -230,7 +270,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT * FROM target + SELECT * FROM "target" WHERE (id != ? AND id != ?) AND (status = ? OR status = ?) AND rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) ORDER BY created_time DESC`.removeExtraWhiteSpace(), ); @@ -254,7 +294,7 @@ module('Unit | Utility | sqlite-query', function (hooks) { }, created_time: [{ gte: date }], }, - sort: { attribute: 'name', direction: 'desc' }, + sort: { attributes: ['name'], direction: 'desc' }, }; const { sql, parameters } = generateSQLExpressions('target', query, { @@ -266,9 +306,9 @@ module('Unit | Utility | sqlite-query', function (hooks) { assert.strictEqual( sql, ` - SELECT data FROM target + SELECT data FROM "target" WHERE (type = ?) AND (status = ? OR status = ?) AND (created_time >= ?) AND rowid IN (SELECT rowid FROM target_fts WHERE target_fts MATCH ?) - ORDER BY name DESC + ORDER BY name COLLATE NOCASE DESC, name DESC LIMIT ? OFFSET ?`.removeExtraWhiteSpace(), ); @@ -290,19 +330,38 @@ module('Unit | Utility | sqlite-query', function (hooks) { emptySearch: { search: '' }, emptyFilter: { filters: {} }, emptyFilterAttributes: { filters: { type: [] } }, - invalidSortAttribute: { attribute: 'test', direction: 'asc' }, + invalidSortAttribute: { + sort: { attributes: ['test'], direction: 'asc' }, + }, + invalidCoalescedSortAttribute: { + sort: { + attributes: ['id', 'test'], + direction: 'asc', + isCoalesced: true, + }, + }, onlyPage: { page: 10 }, onlyPageSize: { pageSize: 5 }, }, function (assert, query) { - const { sql, parameters } = generateSQLExpressions('target', query); + const { sql: targetSql, parameters: targetParameters } = + generateSQLExpressions('target', query); assert.strictEqual( - sql, + targetSql, ` - SELECT * FROM target + SELECT * FROM "target" ORDER BY created_time DESC`.removeExtraWhiteSpace(), ); - assert.deepEqual(parameters, []); + assert.deepEqual(targetParameters, []); + + const { sql: resourceSql, parameters: resourceParameters } = + generateSQLExpressions('resource', query); + assert.strictEqual( + resourceSql, + ` + SELECT * FROM "resource"`.removeExtraWhiteSpace(), + ); + assert.deepEqual(resourceParameters, []); }, ); }); diff --git a/ui/admin/app/routes/scopes/scope/aliases/index.js b/ui/admin/app/routes/scopes/scope/aliases/index.js index ee7bd2394..e6845eba8 100644 --- a/ui/admin/app/routes/scopes/scope/aliases/index.js +++ b/ui/admin/app/routes/scopes/scope/aliases/index.js @@ -75,7 +75,7 @@ export default class ScopesScopeAliasesIndexRoute extends Route { }) ) { const sort = { - attribute: sortAttribute, + attributes: [sortAttribute], direction: sortDirection, }; diff --git a/ui/admin/app/routes/scopes/scope/auth-methods/index.js b/ui/admin/app/routes/scopes/scope/auth-methods/index.js index 71c352448..a28f6061c 100644 --- a/ui/admin/app/routes/scopes/scope/auth-methods/index.js +++ b/ui/admin/app/routes/scopes/scope/auth-methods/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeAuthMethodsIndexRoute extends Route { // =services @@ -83,8 +82,12 @@ export default class ScopesScopeAuthMethodsIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; types.forEach((type) => { filters.type.push({ equals: type }); diff --git a/ui/admin/app/routes/scopes/scope/credential-stores/index.js b/ui/admin/app/routes/scopes/scope/credential-stores/index.js index 503bff94f..65607b674 100644 --- a/ui/admin/app/routes/scopes/scope/credential-stores/index.js +++ b/ui/admin/app/routes/scopes/scope/credential-stores/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeCredentialStoresIndexRoute extends Route { // =services @@ -80,8 +79,12 @@ export default class ScopesScopeCredentialStoresIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; let credentialStores; let totalItems = 0; diff --git a/ui/admin/app/routes/scopes/scope/groups/index.js b/ui/admin/app/routes/scopes/scope/groups/index.js index ce2a0ebc9..b120e8a0e 100644 --- a/ui/admin/app/routes/scopes/scope/groups/index.js +++ b/ui/admin/app/routes/scopes/scope/groups/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeGroupsIndexRoute extends Route { // =services @@ -68,8 +67,12 @@ export default class ScopesScopeGroupsIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; let groups = []; let totalItems = 0; diff --git a/ui/admin/app/routes/scopes/scope/host-catalogs/index.js b/ui/admin/app/routes/scopes/scope/host-catalogs/index.js index c7da30b42..e666bcd8a 100644 --- a/ui/admin/app/routes/scopes/scope/host-catalogs/index.js +++ b/ui/admin/app/routes/scopes/scope/host-catalogs/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeHostCatalogsIndexRoute extends Route { // =services @@ -73,8 +72,12 @@ export default class ScopesScopeHostCatalogsIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; let hostCatalogs; let totalItems = 0; diff --git a/ui/admin/app/routes/scopes/scope/roles/index.js b/ui/admin/app/routes/scopes/scope/roles/index.js index 6c9e523d3..c08c5dcf5 100644 --- a/ui/admin/app/routes/scopes/scope/roles/index.js +++ b/ui/admin/app/routes/scopes/scope/roles/index.js @@ -6,8 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; -import { GRANT_SCOPE_THIS } from 'api/models/role'; export default class ScopesScopeRolesIndexRoute extends Route { // =attributes @@ -33,21 +31,6 @@ export default class ScopesScopeRolesIndexRoute extends Route { }, }; - customSortFunction = { - name: sortNameWithIdFallback, - grant_scope_ids: (recordA, recordB) => { - const a = recordA.attributes?.grant_scope_ids.includes(GRANT_SCOPE_THIS); - const b = recordB.attributes?.grant_scope_ids.includes(GRANT_SCOPE_THIS); - if (a === b) { - return 0; - } else if (a) { - return 1; - } else { - return -1; - } - }, - }; - // =services @service can; @@ -84,10 +67,14 @@ export default class ScopesScopeRolesIndexRoute extends Route { scope_id: [{ equals: scope_id }], }; - const sortFunction = this.customSortFunction[sortAttribute]; - const sort = sortFunction - ? { sortFunction, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + const sort = + sortAttribute === 'name' + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; let roles; let totalItems = 0; diff --git a/ui/admin/app/routes/scopes/scope/scopes/index.js b/ui/admin/app/routes/scopes/scope/scopes/index.js index c011d0c5b..bcbcc5b2f 100644 --- a/ui/admin/app/routes/scopes/scope/scopes/index.js +++ b/ui/admin/app/routes/scopes/scope/scopes/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeScopesIndexRoute extends Route { // =attributes @@ -70,8 +69,12 @@ export default class ScopesScopeScopesIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; const subScopes = await this.store.query('scope', { scope_id, diff --git a/ui/admin/app/routes/scopes/scope/session-recordings/index.js b/ui/admin/app/routes/scopes/scope/session-recordings/index.js index bed76f962..00a7d73dd 100644 --- a/ui/admin/app/routes/scopes/scope/session-recordings/index.js +++ b/ui/admin/app/routes/scopes/scope/session-recordings/index.js @@ -71,17 +71,6 @@ export default class ScopesScopeSessionRecordingsIndexRoute extends Route { return this.retrieveData.perform({ ...params, useDebounce }); } - sortState = (recordA, recordB) => { - const stateMap = { - [STATE_SESSION_RECORDING_AVAILABLE]: this.intl.t('states.completed'), - [STATE_SESSION_RECORDING_STARTED]: this.intl.t('states.recording'), - [STATE_SESSION_RECORDING_UNKNOWN]: this.intl.t('states.failed'), - }; - return String(stateMap[recordA.attributes.state]).localeCompare( - String(stateMap[recordB.attributes.state]), - ); - }; - retrieveData = restartableTask( async ({ search, @@ -122,10 +111,20 @@ export default class ScopesScopeSessionRecordingsIndexRoute extends Route { filters['create_time_values.target.id'].push({ equals: target }); }); + const stateMap = { + [STATE_SESSION_RECORDING_AVAILABLE]: this.intl.t('states.completed'), + [STATE_SESSION_RECORDING_STARTED]: this.intl.t('states.recording'), + [STATE_SESSION_RECORDING_UNKNOWN]: this.intl.t('states.failed'), + }; + const sort = sortAttribute === 'state' - ? { sortFunction: this.sortState, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute], + customSort: { attributeMap: stateMap }, + direction: sortDirection, + } + : { attributes: [sortAttribute], direction: sortDirection }; if ( this.can.can('list scope', scope, { diff --git a/ui/admin/app/routes/scopes/scope/sessions/index.js b/ui/admin/app/routes/scopes/scope/sessions/index.js index cb806ea0f..99cdb3489 100644 --- a/ui/admin/app/routes/scopes/scope/sessions/index.js +++ b/ui/admin/app/routes/scopes/scope/sessions/index.js @@ -98,7 +98,7 @@ export default class ScopesScopeSessionsIndexRoute extends Route { }); const sort = { - attribute: sortAttribute, + attributes: [sortAttribute], direction: sortDirection, }; diff --git a/ui/admin/app/routes/scopes/scope/targets/index.js b/ui/admin/app/routes/scopes/scope/targets/index.js index 6366fcbff..2bff28bda 100644 --- a/ui/admin/app/routes/scopes/scope/targets/index.js +++ b/ui/admin/app/routes/scopes/scope/targets/index.js @@ -64,16 +64,6 @@ export default class ScopesScopeTargetsIndexRoute extends Route { return this.retrieveData.perform({ ...params, useDebounce }); } - sortType = (recordA, recordB) => { - const typeMap = { - [TYPE_TARGET_SSH]: this.intl.t('resources.target.types.ssh'), - [TYPE_TARGET_TCP]: this.intl.t('resources.target.types.tcp'), - }; - return String(typeMap[recordA.attributes.type]).localeCompare( - String(typeMap[recordB.attributes.type]), - ); - }; - retrieveData = restartableTask( async ({ search, @@ -117,10 +107,19 @@ export default class ScopesScopeTargetsIndexRoute extends Route { this.addActiveSessionFilters(filters, availableSessions, sessions); } + const typeMap = { + [TYPE_TARGET_SSH]: this.intl.t('resources.target.types.ssh'), + [TYPE_TARGET_TCP]: this.intl.t('resources.target.types.tcp'), + }; + const sort = sortAttribute === 'type' - ? { sortFunction: this.sortType, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute], + customSort: { attributeMap: typeMap }, + direction: sortDirection, + } + : { attributes: [sortAttribute], direction: sortDirection }; let targets; let totalItems = 0; diff --git a/ui/admin/app/routes/scopes/scope/users/index.js b/ui/admin/app/routes/scopes/scope/users/index.js index 3559050d9..95d70a3b9 100644 --- a/ui/admin/app/routes/scopes/scope/users/index.js +++ b/ui/admin/app/routes/scopes/scope/users/index.js @@ -6,7 +6,6 @@ import Route from '@ember/routing/route'; import { service } from '@ember/service'; import { restartableTask, timeout } from 'ember-concurrency'; -import { sortNameWithIdFallback } from 'admin/utils/sort-name-with-id-fallback'; export default class ScopesScopeUsersIndexRoute extends Route { // =attributes @@ -71,8 +70,12 @@ export default class ScopesScopeUsersIndexRoute extends Route { const sort = sortAttribute === 'name' - ? { sortFunction: sortNameWithIdFallback, direction: sortDirection } - : { attribute: sortAttribute, direction: sortDirection }; + ? { + attributes: [sortAttribute, 'id'], + direction: sortDirection, + isCoalesced: true, + } + : { attributes: [sortAttribute], direction: sortDirection }; let users; let totalItems = 0; diff --git a/ui/admin/app/templates/scopes/scope/roles/index.hbs b/ui/admin/app/templates/scopes/scope/roles/index.hbs index 024038c61..7ae2f745a 100644 --- a/ui/admin/app/templates/scopes/scope/roles/index.hbs +++ b/ui/admin/app/templates/scopes/scope/roles/index.hbs @@ -59,10 +59,7 @@ > {{t 'form.name.label'}} - {{t 'resources.role.titles.grants-applied'}} + {{t 'resources.role.titles.grants-applied'}}