bug: fix filter and search (#12587)
* chore: remove commented out code and skipped tests * refact: triggeredBy requires filter expression not qp * refact: use filter expression dsl instead of named params * fix: add type * docs: add in-line reference to filter expression DSL * fix: update filter copy for non-matches * fix: correct conditional logic to render no match copy
This commit is contained in:
Jai
GitHub
parent
aed56e5732
commit
b3985db31f
|
|
@ -120,6 +120,7 @@ export default class EvaluationsController extends Controller {
|
|||
{ key: 'rolling-update', label: 'Rolling Update' },
|
||||
{ key: 'deployment-watcher', label: 'Deployment Watcher' },
|
||||
{ key: 'failed-follow-up', label: 'Failed Follow Up' },
|
||||
{ key: 'max-disconnect-timeout', label: 'Max Disconnect Timeout' },
|
||||
{ key: 'max-plan-attempts', label: 'Max Plan Attempts' },
|
||||
{ key: 'alloc-failure', label: 'Allocation Failure' },
|
||||
{ key: 'queued-allocs', label: 'Queued Allocations' },
|
||||
|
|
@ -151,6 +152,60 @@ export default class EvaluationsController extends Controller {
|
|||
];
|
||||
}
|
||||
|
||||
filters = ['status', 'qpNamespace', 'type', 'triggeredBy', 'searchTerm'];
|
||||
|
||||
get hasFiltersApplied() {
|
||||
return this.filters.reduce((result, filter) => {
|
||||
// By default we always set qpNamespace to the '*' wildcard
|
||||
// We need to ensure that if namespace is the only filter, that we send the correct error message to the user
|
||||
if (this[filter] && filter !== 'qpNamespace') {
|
||||
result = true;
|
||||
}
|
||||
return result;
|
||||
}, false);
|
||||
}
|
||||
|
||||
get currentFilters() {
|
||||
const result = [];
|
||||
for (const filter of this.filters) {
|
||||
const isNamespaceWildcard =
|
||||
filter === 'qpNamespace' && this[filter] === '*';
|
||||
if (this[filter] && !isNamespaceWildcard) {
|
||||
result.push({ [filter]: this[filter] });
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
get noMatchText() {
|
||||
let text = '';
|
||||
const cleanNames = {
|
||||
status: 'Status',
|
||||
qpNamespace: 'Namespace',
|
||||
type: 'Type',
|
||||
triggeredBy: 'Triggered By',
|
||||
searchTerm: 'Search Term',
|
||||
};
|
||||
if (this.hasFiltersApplied) {
|
||||
for (let i = 0; i < this.currentFilters.length; i++) {
|
||||
const filter = this.currentFilters[i];
|
||||
const [name] = Object.keys(filter);
|
||||
const filterName = cleanNames[name];
|
||||
const filterValue = filter[name];
|
||||
if (this.currentFilters.length === 1)
|
||||
return `${filterName}: ${filterValue}.`;
|
||||
if (i !== 0 && i !== this.currentFilters.length - 1)
|
||||
text = text.concat(`, ${filterName}: ${filterValue}`);
|
||||
if (i === 0) text = text.concat(`${filterName}: ${filterValue}`);
|
||||
if (i === this.currentFilters.length - 1) {
|
||||
return text.concat(`, ${filterName}: ${filterValue}.`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return text;
|
||||
}
|
||||
|
||||
@tracked pageSize = this.userSettings.pageSize;
|
||||
@tracked nextToken = null;
|
||||
@tracked previousTokens = [];
|
||||
|
|
|
|||
|
|
@ -32,23 +32,60 @@ export default class EvaluationsIndexRoute extends Route {
|
|||
nextToken,
|
||||
pageSize,
|
||||
searchTerm,
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
status,
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
triggeredBy,
|
||||
type,
|
||||
qpNamespace: namespace,
|
||||
}) {
|
||||
const generateFilter = () => {
|
||||
/*
|
||||
We use our own DSL for filter expressions. This function takes our query parameters and builds a query that matches our DSL.
|
||||
Documentation can be found here: https://www.nomadproject.io/api-docs#filtering
|
||||
*/
|
||||
const generateFilterExpression = () => {
|
||||
const searchFilter = searchTerm
|
||||
? `ID contains "${searchTerm}" or JobID contains "${searchTerm}" or NodeID contains "${searchTerm}" or TriggeredBy contains "${searchTerm}"`
|
||||
: null;
|
||||
const typeFilter =
|
||||
type === 'client' ? `NodeID is not empty` : `NodeID is empty`;
|
||||
const triggeredByFilter = `TriggeredBy contains "${triggeredBy}"`;
|
||||
const statusFilter = `Status contains "${status}"`;
|
||||
|
||||
if (searchTerm && type) return `${searchFilter} ${typeFilter}`;
|
||||
if (searchTerm) return searchFilter;
|
||||
if (type) return typeFilter;
|
||||
let filterExp;
|
||||
if (searchTerm) {
|
||||
if (!type && !status && !triggeredBy) {
|
||||
return searchFilter;
|
||||
}
|
||||
filterExp = `(${searchFilter})`;
|
||||
if (type) {
|
||||
filterExp = `${filterExp} and ${typeFilter}`;
|
||||
}
|
||||
if (triggeredBy) {
|
||||
filterExp = `${filterExp} and ${triggeredByFilter}`;
|
||||
}
|
||||
if (status) {
|
||||
filterExp = `${filterExp} and ${statusFilter}`;
|
||||
}
|
||||
return filterExp;
|
||||
}
|
||||
|
||||
if (type || status || triggeredBy) {
|
||||
const lookup = {
|
||||
[type]: typeFilter,
|
||||
[status]: statusFilter,
|
||||
[triggeredBy]: triggeredByFilter,
|
||||
};
|
||||
|
||||
filterExp = [type, status, triggeredBy].reduce((result, filter) => {
|
||||
const expression = lookup[filter];
|
||||
if (!!filter && result !== '') {
|
||||
result = result.concat(` and ${expression}`);
|
||||
} else if (filter) {
|
||||
result = expression;
|
||||
}
|
||||
return result;
|
||||
}, '');
|
||||
return filterExp;
|
||||
}
|
||||
|
||||
return null;
|
||||
};
|
||||
|
|
@ -59,10 +96,7 @@ export default class EvaluationsIndexRoute extends Route {
|
|||
namespace,
|
||||
per_page: pageSize,
|
||||
next_token: nextToken,
|
||||
// TODO: add support for status and triggeredBy filters
|
||||
//status,
|
||||
//triggeredBy,
|
||||
filter: generateFilter(),
|
||||
filter: generateFilterExpression(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@
|
|||
/>
|
||||
</div>
|
||||
<div class="toolbar-item is-right-aligned">
|
||||
{{!-- TODO: add support for status and triggered by
|
||||
<SingleSelectDropdown
|
||||
data-test-evaluation-status-facet
|
||||
@label="Status"
|
||||
|
|
@ -34,7 +33,6 @@
|
|||
@selection={{this.type}}
|
||||
@onSelect={{action this.setQueryParam "type"}}
|
||||
/>
|
||||
--}}
|
||||
<SingleSelectDropdown
|
||||
data-test-evaluation-namespace-facet
|
||||
@label="Namespace"
|
||||
|
|
@ -73,7 +71,7 @@
|
|||
<tr
|
||||
data-test-evaluation="{{row.model.shortId}}"
|
||||
style="cursor: pointer;"
|
||||
class="{{if (eq this.currentEval row.model.id) 'is-active'}}"
|
||||
class="{{if (eq this.currentEval row.model.id) "is-active"}}"
|
||||
tabindex="0"
|
||||
{{on "click" (fn this.handleEvaluationClick row.model)}}
|
||||
{{on "keyup" (fn this.handleEvaluationClick row.model)}}
|
||||
|
|
@ -166,11 +164,11 @@
|
|||
No Matches
|
||||
</h3>
|
||||
<p class="empty-message-body">
|
||||
{{#if this.status}}
|
||||
{{#if this.hasFiltersApplied}}
|
||||
<span data-test-no-eval-match>
|
||||
No evaluations match the status
|
||||
No evaluations that match:
|
||||
<strong>
|
||||
{{this.status}}
|
||||
{{this.noMatchText}}
|
||||
</strong>
|
||||
</span>
|
||||
{{else}}
|
||||
|
|
|
|||
|
|
@ -143,9 +143,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'Forwards the correct query parameters on default query when route initially loads'
|
||||
);
|
||||
|
|
@ -163,8 +160,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
});
|
||||
|
||||
module('filters', function () {
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
test.skip('it should enable filtering by evaluation status', async function (assert) {
|
||||
test('it should enable filtering by evaluation status', async function (assert) {
|
||||
assert.expect(2);
|
||||
|
||||
server.get('/evaluations', getStandardRes);
|
||||
|
|
@ -177,25 +173,20 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
status: 'pending',
|
||||
next_token: '',
|
||||
triggeredBy: '',
|
||||
filter: '',
|
||||
filter: 'Status contains "pending"',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
return [];
|
||||
});
|
||||
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
/*
|
||||
await clickTrigger('[data-test-evaluation-status-facet]');
|
||||
await selectChoose('[data-test-evaluation-status-facet]', 'Pending');
|
||||
|
||||
assert
|
||||
.dom('[data-test-no-eval-match]')
|
||||
.exists('Renders a message saying no evaluations match filter status');
|
||||
*/
|
||||
});
|
||||
|
||||
test('it should enable filtering by namespace', async function (assert) {
|
||||
|
|
@ -213,9 +204,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
|
|
@ -230,8 +218,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
.exists('Renders a message saying no evaluations match filter status');
|
||||
});
|
||||
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
test.skip('it should enable filtering by triggered by', async function (assert) {
|
||||
test('it should enable filtering by triggered by', async function (assert) {
|
||||
assert.expect(2);
|
||||
|
||||
server.get('/evaluations', getStandardRes);
|
||||
|
|
@ -244,10 +231,8 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
status: '',
|
||||
next_token: '',
|
||||
triggeredBy: 'periodic-job',
|
||||
filter: '',
|
||||
filter: `TriggeredBy contains "periodic-job"`,
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
|
|
@ -265,8 +250,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
.exists('Renders a message saying no evaluations match filter status');
|
||||
});
|
||||
|
||||
// TODO: add support for type filter.
|
||||
test.skip('it should enable filtering by type', async function (assert) {
|
||||
test('it should enable filtering by type', async function (assert) {
|
||||
assert.expect(2);
|
||||
|
||||
server.get('/evaluations', getStandardRes);
|
||||
|
|
@ -279,9 +263,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
status: '',
|
||||
next_token: '',
|
||||
triggeredBy: '',
|
||||
filter: 'NodeID is not empty',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
|
|
@ -313,9 +295,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: `ID contains "${searchTerm}" or JobID contains "${searchTerm}" or NodeID contains "${searchTerm}" or TriggeredBy contains "${searchTerm}"`,
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
|
|
@ -328,6 +307,81 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
.dom('[data-test-empty-evaluations-list]')
|
||||
.exists('Renders a message saying no evaluations match filter status');
|
||||
});
|
||||
|
||||
test('it should enable combining filters and search', async function (assert) {
|
||||
assert.expect(5);
|
||||
|
||||
server.get('/evaluations', getStandardRes);
|
||||
|
||||
await visit('/evaluations');
|
||||
|
||||
const searchTerm = 'Lasso';
|
||||
server.get('/evaluations', function (_server, fakeRequest) {
|
||||
assert.deepEqual(
|
||||
fakeRequest.queryParams,
|
||||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: `ID contains "${searchTerm}" or JobID contains "${searchTerm}" or NodeID contains "${searchTerm}" or TriggeredBy contains "${searchTerm}"`,
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
return [];
|
||||
});
|
||||
await typeIn('[data-test-evaluations-search] input', searchTerm);
|
||||
|
||||
server.get('/evaluations', function (_server, fakeRequest) {
|
||||
assert.deepEqual(
|
||||
fakeRequest.queryParams,
|
||||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: `(ID contains "${searchTerm}" or JobID contains "${searchTerm}" or NodeID contains "${searchTerm}" or TriggeredBy contains "${searchTerm}") and NodeID is not empty`,
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
return [];
|
||||
});
|
||||
await clickTrigger('[data-test-evaluation-type-facet]');
|
||||
await selectChoose('[data-test-evaluation-type-facet]', 'Client');
|
||||
|
||||
server.get('/evaluations', function (_server, fakeRequest) {
|
||||
assert.deepEqual(
|
||||
fakeRequest.queryParams,
|
||||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: `NodeID is not empty`,
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
return [];
|
||||
});
|
||||
await click('[data-test-evaluations-search] button');
|
||||
|
||||
server.get('/evaluations', function (_server, fakeRequest) {
|
||||
assert.deepEqual(
|
||||
fakeRequest.queryParams,
|
||||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: `NodeID is not empty and Status contains "complete"`,
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
return [];
|
||||
});
|
||||
await clickTrigger('[data-test-evaluation-status-facet]');
|
||||
await selectChoose('[data-test-evaluation-status-facet]', 'Complete');
|
||||
|
||||
assert
|
||||
.dom('[data-test-empty-evaluations-list]')
|
||||
.exists('Renders a message saying no evaluations match filter status');
|
||||
});
|
||||
});
|
||||
|
||||
module('page size', function (hooks) {
|
||||
|
|
@ -352,9 +406,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '50',
|
||||
next_token: '',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes a request with the per_page set by the user'
|
||||
);
|
||||
|
|
@ -388,9 +439,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: 'next-token-1',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
|
|
@ -416,9 +464,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: 'next-token-2',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes another server request using the options selected by the user'
|
||||
);
|
||||
|
|
@ -444,9 +489,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: 'next-token-1',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'It makes a request using the stored old token.'
|
||||
);
|
||||
|
|
@ -467,9 +509,6 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
per_page: '25',
|
||||
next_token: '',
|
||||
filter: '',
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
// status: '',
|
||||
// triggeredBy: '',
|
||||
},
|
||||
'When there are no more stored previous tokens, we will request with no next-token.'
|
||||
);
|
||||
|
|
@ -483,8 +522,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
await click('[data-test-eval-pagination-prev]');
|
||||
});
|
||||
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
test.skip('it should clear all query parameters on refresh', async function (assert) {
|
||||
test('it should clear all query parameters on refresh', async function (assert) {
|
||||
assert.expect(1);
|
||||
|
||||
server.get('/evaluations', function () {
|
||||
|
|
@ -512,9 +550,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
status: '',
|
||||
next_token: '',
|
||||
triggeredBy: '',
|
||||
filter: '',
|
||||
},
|
||||
'It clears all query parameters when making a refresh'
|
||||
|
|
@ -529,8 +565,7 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
await click('[data-test-eval-refresh]');
|
||||
});
|
||||
|
||||
// TODO: add support for status and triggeredBy filters.
|
||||
test.skip('it should reset pagination when filters are applied', async function (assert) {
|
||||
test('it should reset pagination when filters are applied', async function (assert) {
|
||||
assert.expect(1);
|
||||
|
||||
server.get('/evaluations', function () {
|
||||
|
|
@ -562,10 +597,8 @@ module('Acceptance | evaluations list', function (hooks) {
|
|||
{
|
||||
namespace: '*',
|
||||
per_page: '25',
|
||||
status: 'pending',
|
||||
next_token: '',
|
||||
triggeredBy: '',
|
||||
filter: '',
|
||||
filter: 'Status contains "pending"',
|
||||
},
|
||||
'It clears all next token when filtered request is made'
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in New Issue