Skip to content

Commit d29c32a

Browse files
nilmergnilmerg
authored
Merge pull request #51 from Icinga/enhance-searchbar-validation-50
Enhance searchbar validation
2 parents 0ebacde + 409862f commit d29c32a

File tree

10 files changed

+764
-50
lines changed

10 files changed

+764
-50
lines changed

‎asset/js/widget/BaseInput.js‎

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
173173
}
174174

175175
updateTerms(changedTerms) {
176+
// Reset the data input, otherwise the value remains and is sent continuously with subsequent requests
177+
this.dataInput.value = '';
178+
176179
for (const termIndex of Object.keys(changedTerms)) {
177180
let label = this.termContainer.querySelector(`[data-index="${ termIndex }"]`);
178181
if (! label) {
@@ -288,6 +291,24 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
288291
return this.usedTerms.length > 0;
289292
}
290293

294+
hasSyntaxError(input) {
295+
if (typeof input === 'undefined') {
296+
input = this.input;
297+
}
298+
299+
return 'hasSyntaxError' in input.dataset;
300+
}
301+
302+
clearSyntaxError(input) {
303+
if (typeof input === 'undefined') {
304+
input = this.input;
305+
}
306+
307+
delete input.dataset.hasSyntaxError;
308+
input.removeAttribute('pattern');
309+
input.removeAttribute('title');
310+
}
311+
291312
getQueryString() {
292313
return this.termsToQueryString(this.usedTerms);
293314
}
@@ -559,7 +580,12 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
559580
if (event.detail && 'terms' in event.detail) {
560581
this.termInput.value = event.detail.terms;
561582
} else {
562-
this.termInput.value = this.termsToQueryString(this.usedTerms);
583+
let renderedTerms = this.termsToQueryString(this.usedTerms);
584+
if (this.hasSyntaxError()) {
585+
renderedTerms += this.input.value;
586+
}
587+
588+
this.termInput.value = renderedTerms;
563589
}
564590

565591
// Enable the hidden input, otherwise it's not submitted
@@ -603,7 +629,14 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
603629

604630
let termData = { label: this.readPartialTerm(input) };
605631
this.updateTermData(termData, input);
606-
this.complete(input, { term: termData });
632+
633+
if (! input.value && this.hasSyntaxError(input)) {
634+
this.clearSyntaxError(input);
635+
}
636+
637+
if (! this.hasSyntaxError(input)) {
638+
this.complete(input, { term: termData });
639+
}
607640

608641
if (! isTerm) {
609642
this.autoSubmit(this.input, 'remove', this.clearSelectedTerms());
@@ -615,6 +648,15 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
615648
let input = event.target;
616649
let termIndex = Number(input.parentNode.dataset.index);
617650

651+
if (this.hasSyntaxError(input) && ! (/[A-Z]/.test(event.key.charAt(0)) || event.ctrlKey || event.metaKey)) {
652+
// Clear syntax error flag if the user types entirely new input after having selected the entire input
653+
// (This way the input isn't empty but switches from input to input immediately, causing the clearing
654+
// in onInput to not work)
655+
if (input.selectionEnd - input.selectionStart === input.value.length) {
656+
this.clearSyntaxError(input);
657+
}
658+
}
659+
618660
let removedTerms;
619661
switch (event.key) {
620662
case ' ':
@@ -734,6 +776,10 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
734776

735777
onTermFocusOut(event) {
736778
let input = event.target;
779+
if (this.hasSyntaxError(input)) {
780+
return;
781+
}
782+
737783
// skipSaveOnBlur is set if the input is about to be removed anyway.
738784
// If saveTerm would remove the input as well, the other removal will fail
739785
// without any chance to handle it. (Element.remove() blurs the input)
@@ -759,13 +805,17 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
759805
this.deselectTerms();
760806

761807
let input = event.target;
762-
let value = this.readPartialTerm(input);
763-
this.complete(input, { trigger: 'script', term: { label: value } });
808+
if (! this.hasSyntaxError(input)) {
809+
let value = this.readPartialTerm(input);
810+
this.complete(input, { trigger: 'script', term: { label: value } });
811+
}
764812
}
765813

766814
onButtonClick(event) {
767-
// Register current input value, otherwise it's not included
768-
this.exchangeTerm();
815+
if (! this.hasSyntaxError()) {
816+
// Register current input value, otherwise it's not included
817+
this.exchangeTerm();
818+
}
769819

770820
if (this.hasTerms()) {
771821
this.input.required = false;
@@ -778,7 +828,7 @@ define(["../notjQuery", "Completer"], function ($, Completer) {
778828
}
779829

780830
onPaste(event) {
781-
if (this.hasTerms()) {
831+
if (this.hasTerms() || this.input.value) {
782832
return;
783833
}
784834

‎asset/js/widget/FilterInput.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,10 @@ define(["../notjQuery", "BaseInput"], function ($, BaseInput) {
13771377
let input = event.target;
13781378
let isTerm = input.parentNode.dataset.index >= 0;
13791379

1380+
if (this.hasSyntaxError(input)) {
1381+
return;
1382+
}
1383+
13801384
let currentValue = this.readPartialTerm(input);
13811385
if (isTerm && ! currentValue) {
13821386
// Switching contexts requires input first
@@ -1497,7 +1501,9 @@ define(["../notjQuery", "BaseInput"], function ($, BaseInput) {
14971501
let input = event.target;
14981502
let termIndex = Number(input.parentNode.dataset.index);
14991503

1500-
if (termIndex >= 0) {
1504+
if (this.hasSyntaxError(input)) {
1505+
// pass
1506+
} else if (termIndex >= 0) {
15011507
let value = this.readPartialTerm(input);
15021508
if (! this.checkValidity(input)) {
15031509
this.reportValidity(input);
@@ -1531,7 +1537,7 @@ define(["../notjQuery", "BaseInput"], function ($, BaseInput) {
15311537
onPaste(event) {
15321538
if (! this.hasTerms()) {
15331539
super.onPaste(event);
1534-
} else {
1540+
} else if (! this.input.value) {
15351541
let terms = event.clipboardData.getData('text/plain');
15361542
if (this.termType === 'logical_operator') {
15371543
if (! this.validOperator(terms[0]).exactMatch) {

‎src/Control/SearchBar.php‎

Lines changed: 147 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
use ipl\Validator\CallbackValidator;
1414
use ipl\Web\Common\FormUid;
1515
use ipl\Web\Control\SearchBar\Terms;
16+
use ipl\Web\Control\SearchBar\ValidatedColumn;
17+
use ipl\Web\Control\SearchBar\ValidatedOperator;
18+
use ipl\Web\Control\SearchBar\ValidatedValue;
1619
use ipl\Web\Filter\ParseException;
1720
use ipl\Web\Filter\QueryString;
1821
use ipl\Web\Url;
@@ -22,8 +25,17 @@ class SearchBar extends Form
2225
{
2326
use FormUid;
2427

25-
/** @var string Emitted in case of an auto submit */
26-
const ON_CHANGE = 'on_change';
28+
/** @var string Emitted in case the user added a new condition */
29+
const ON_ADD = 'on_add';
30+
31+
/** @var string Emitted in case the user inserted a new condition */
32+
const ON_INSERT = 'on_insert';
33+
34+
/** @var string Emitted in case the user changed an existing condition */
35+
const ON_SAVE = 'on_save';
36+
37+
/** @var string Emitted in case the user removed a condition */
38+
const ON_REMOVE = 'on_remove';
2739

2840
protected $defaultAttributes = [
2941
'data-enrichment-type' => 'search-bar',
@@ -204,13 +216,49 @@ private function protectId($id)
204216

205217
public function isValidEvent($event)
206218
{
207-
if ($event !== self::ON_CHANGE) {
208-
return parent::isValidEvent($event);
219+
switch ($event) {
220+
case self::ON_ADD:
221+
case self::ON_SAVE:
222+
case self::ON_INSERT:
223+
case self::ON_REMOVE:
224+
return true;
225+
default:
226+
return parent::isValidEvent($event);
227+
}
228+
}
229+
230+
private function validateCondition($eventType, $indices, $termsData, &$changes)
231+
{
232+
// TODO: In case of the query string validation, all three are guaranteed to be set.
233+
// The Parser also provides defaults, why shouldn't we here?
234+
$column = ValidatedColumn::fromTermData($termsData[0]);
235+
$operator = isset($termsData[1])
236+
? ValidatedOperator::fromTermData($termsData[1])
237+
: null;
238+
$value = isset($termsData[2])
239+
? ValidatedValue::fromTermData($termsData[2])
240+
: null;
241+
242+
$this->emit($eventType, [$column, $operator, $value]);
243+
244+
if ($eventType !== self::ON_REMOVE) {
245+
if (! $column->isValid() || $column->hasBeenChanged()) {
246+
$changes[$indices[0]] = array_merge($termsData[0], $column->toTermData());
247+
}
248+
249+
if ($operator && ! $operator->isValid()) {
250+
$changes[$indices[1]] = array_merge($termsData[1], $operator->toTermData());
251+
}
252+
253+
if ($value && (! $value->isValid() || $value->hasBeenChanged())) {
254+
$changes[$indices[2]] = array_merge($termsData[2], $value->toTermData());
255+
}
209256
}
210257

211-
return true;
258+
return $column->isValid() && (! $operator || $operator->isValid()) && (! $value || $value->isValid());
212259
}
213260

261+
214262
protected function assemble()
215263
{
216264
$termContainerId = $this->protectId('terms');
@@ -243,32 +291,72 @@ protected function assemble()
243291
return true;
244292
}
245293

246-
$validatedData = $data;
247-
// TODO: I'd like to not pass a ref here, but the Events trait does not support event return values
248-
$this->emit(self::ON_CHANGE, [&$validatedData]);
294+
switch ($data['type']) {
295+
case 'add':
296+
case 'exchange':
297+
$type = self::ON_ADD;
298+
299+
break;
300+
case 'insert':
301+
$type = self::ON_INSERT;
302+
303+
break;
304+
case 'save':
305+
$type = self::ON_SAVE;
306+
307+
break;
308+
case 'remove':
309+
$type = self::ON_REMOVE;
310+
311+
break;
312+
default:
313+
return true;
314+
}
249315

250316
$changes = [];
251-
foreach (isset($validatedData['terms']) ? $validatedData['terms'] : [] as $termIndex => $termData) {
252-
if ($termData !== $data['terms'][$termIndex]) {
253-
if (isset($termData['invalidMsg']) && ! isset($termData['pattern'])) {
254-
$termData['pattern'] = sprintf(
255-
'^\s*(?!%s\b).*\s*$',
256-
$data['terms'][$termIndex]['label']
257-
);
258-
}
317+
$invalid = false;
318+
$indices = [null, null, null];
319+
$termsData = [null, null, null];
320+
foreach (isset($data['terms']) ? $data['terms'] : [] as $termIndex => $termData) {
321+
switch ($termData['type']) {
322+
case 'column':
323+
$indices[0] = $termIndex;
324+
$termsData[0] = $termData;
325+
326+
break;
327+
case 'operator':
328+
$indices[1] = $termIndex;
329+
$termsData[1] = $termData;
330+
331+
break;
332+
case 'value':
333+
$indices[2] = $termIndex;
334+
$termsData[2] = $termData;
335+
336+
break;
337+
default:
338+
if ($termsData[0] !== null) {
339+
if (! $this->validateCondition($type, $indices, $termsData, $changes)) {
340+
$invalid = true;
341+
}
342+
}
343+
344+
$indices = $termsData = [null, null, null];
345+
}
346+
}
259347

260-
$changes[$termIndex] = $termData;
348+
if ($termsData[0] !== null) {
349+
if (! $this->validateCondition($type, $indices, $termsData, $changes)) {
350+
$invalid = true;
261351
}
262352
}
263353

264354
if (! empty($changes)) {
265355
$this->changes = ['#' . $searchInputId, $changes];
266356
$termContainer->applyChanges($changes);
267-
// TODO: Not every change must be invalid, change this once there are Event objects
268-
return false;
269357
}
270358

271-
return true;
359+
return ! $invalid;
272360
})
273361
]
274362
]);
@@ -291,8 +379,42 @@ protected function assemble()
291379
'data-choose-column' => t('Please enter a valid column.'),
292380
'validators' => [
293381
new CallbackValidator(function ($q, CallbackValidator $validator) {
382+
$invalid = false;
383+
384+
$parser = QueryString::fromString($q);
385+
$parser->on(QueryString::ON_CONDITION, function (Filter\Condition $condition) use (&$invalid) {
386+
$columnIndex = $condition->metaData()->get('columnIndex');
387+
if (isset($this->changes[1][$columnIndex])) {
388+
$change = $this->changes[1][$columnIndex];
389+
$condition->setColumn($change['search']);
390+
} elseif (empty($this->changes)) {
391+
$column = ValidatedColumn::fromFilterCondition($condition);
392+
$operator = ValidatedOperator::fromFilterCondition($condition);
393+
$value = ValidatedValue::fromFilterCondition($condition);
394+
$this->emit(self::ON_ADD, [$column, $operator, $value]);
395+
396+
$condition->setColumn($column->getSearchValue());
397+
$condition->setValue($value->getSearchValue());
398+
399+
if (! $column->isValid()) {
400+
$invalid = true;
401+
$condition->metaData()->merge($column->toMetaData());
402+
}
403+
404+
if (! $operator->isValid()) {
405+
$invalid = true;
406+
$condition->metaData()->merge($operator->toMetaData());
407+
}
408+
409+
if (! $value->isValid()) {
410+
$invalid = true;
411+
$condition->metaData()->merge($value->toMetaData());
412+
}
413+
}
414+
});
415+
294416
try {
295-
$filter = QueryString::parse($q);
417+
$filter = $parser->parse();
296418
} catch (ParseException $e) {
297419
$charAt = $e->getCharPos() - 1;
298420
$char = $e->getChar();
@@ -301,7 +423,8 @@ protected function assemble()
301423
->setValue(substr($q, $charAt))
302424
->addAttributes([
303425
'title' => sprintf(t('Unexpected %s at start of input'), $char),
304-
'pattern' => sprintf('^(?!%s).*', $char === ')' ? '\)' : $char)
426+
'pattern' => sprintf('^(?!%s).*', $char === ')' ? '\)' : $char),
427+
'data-has-syntax-error' => true
305428
]);
306429

307430
$probablyValidQueryString = substr($q, 0, $charAt);
@@ -311,7 +434,8 @@ protected function assemble()
311434

312435
$this->getElement($this->getSearchParameter())->setValue('');
313436
$this->setFilter($filter);
314-
return true;
437+
438+
return ! $invalid;
315439
})
316440
]
317441
]);

0 commit comments

Comments
 (0)