Make term input decorable by nilmerg · Pull Request #317 · Icinga/ipl-web

nilmerg · 2025-10-01T08:44:47Z

The two latest commits in combination with Icinga/ipl-html#197 should resolve the TODO introduced in 446e737.

Auto-submission is lazily detected on the first call of hasBeenAutoSubmitted(), so it will work even when TermInputElement is registered before a request is set on the form.

The issue of onRegistered() not being called when inside a fieldset is addressed in the linked ipl-html PR.

The director uses a TermInput inside a fieldset (ArrayElement in DictionaryItem). This only works because the TermInput's onRegistered() is never called, so the old TermInput::hasBeenAutoSubmitted() always returned false and ON_ENRICH was always emittted by TermInput::assemble().

To stay compatible with the director, the deprecated TermInput tracks whether prepareMultipartUpdate(), if it was never called but the TermInput was auto-submitted ON_ENRICH is emitted.

We don't want a label for the term input, so let's ensure it doesn't get one with either decorator implementation.

Plus another small correction I accidentally noticed.

Allows to load it using the form element loading of `CompatForm`

The request was read in `onRegistered()`, which failed if no request was set yet. It's now read when `hasBeenAutoSubmitted()` is first called, at the end of the input's `assemble()`.

Consumers of the deprecated `TermInput` may render the whole form upon auto-submission instead of sending multipart updates. Then `ON_ENRICH` is now emitted before rendering, if `prepareMultipartUpdate()` wasn't called.

nilmerg · 2026-10-09T13:55:31Z

+            $termContainerId = Attribute::sanitizeId($this->getValueOfNameAttribute()) . '-terms';
+            $mainInputId = Attribute::sanitizeId($this->getValueOfNameAttribute()) . '-search-input';
+            $autoSubmittedBy = $this->form?->getRequest()?->getHeader('X-Icinga-Autosubmittedby') ?? [];
+
+            $this->hasBeenAutoSubmitted = in_array($mainInputId, $autoSubmittedBy, true)
+                || in_array($termContainerId, $autoSubmittedBy, true);


I think this still fits better inside onRegistered but with the event Form::ON_HANDLE_REQUEST in place, in order to reliably get access to the request.

nilmerg · 2026-10-09T13:56:13Z

+        return $this->terms;
+    }
+
+    public function getElements()


can be dropped now, the todo is solved by now since FieldsetElement::hasValue assembles by default.

resolve #1357 Add a `TermInput` to the _general-settings_ section, that allows to configure the default relations to include when sending events to Icinga Notifications. Like the existing toggle, the input is locked if its setting is managed locally. Both are locked independently of each other, so the default relations can still be configured if the toggle is not available. This also applies if no Unix socket for Icinga Notifications was discovered, which now only locks the toggle instead of the entire section. The entire Notifications section is locked in case: - No database is configured - The schema version is outdated - Database synchronization is disabled - No Instance is found - Multiple responsible instances are found, which indicates multiple environments. As each environment would require its own toggle + terminput this is not supported. Additionally Icinga Notifications sources are now created for each service user if several exist. The `TermInput` currently relies on a workaround, but once Icinga/ipl-web#317 is merged it will rely on the new implementation and the current workaround can simply be removed.

lippserd

Please also add tests for TermInputElement and for the compatibility path of the deprecated TermInput.

TermInputElement relies on $form from ipl-html#197, so we must not forget to raise the ipl/html requirement in composer.json before releasing. It probably makes sense to open an issue for that and add it to the milestone.

+1 to dropping the getElements() override. ipl-html#197 can then use getElements() as well.

lippserd · 2026-10-14T08:44:03Z

+        if ($this->hasBeenAutoSubmitted === null) {
+            $termContainerId = Attribute::sanitizeId($this->getValueOfNameAttribute()) . '-terms';
+            $mainInputId = Attribute::sanitizeId($this->getValueOfNameAttribute()) . '-search-input';
+            $autoSubmittedBy = $this->form?->getRequest()?->getHeader('X-Icinga-Autosubmittedby') ?? [];
+
+            $this->hasBeenAutoSubmitted = in_array($mainInputId, $autoSubmittedBy, true)
+                || in_array($termContainerId, $autoSubmittedBy, true);
+        }


This caches false if the input is assembled before the form has a request, for example through getElements(), hasValue() or validate() before handleRequest(). Detection then stays false for the lifetime of the input. Please return false without caching while there is no request.

lippserd · 2026-10-14T08:44:03Z

+    protected function beforeRender(): void
    {
-        $myName = Attribute::sanitizeId($this->getValueOfNameAttribute());
-
-        $termInputId = $myName . '-term-input';
-        $dataInputId = $myName . '-data-input';
-        $searchInputId = $myName . '-search-input';
-        $suggestionsId = $myName . '-suggestions';
-
-        $termContainer = $this->termContainer();
-
-        $suggestions = (new HtmlElement('div'))
-            ->setAttribute('hidden', true)
-            ->setAttribute('id', $suggestionsId)
-            ->setAttribute('class', 'search-suggestions');
-        if ($this->suggestions !== null) {
-            $suggestions->addHtml($this->suggestions);
-        }
-
-        $termInput = $this->createElement('hidden', 'value', [
-            'id' => $termInputId,
-            'disabled' => true
-        ]);
-
-        $dataInput = new class ('data', [
-            'ignore' => true,
-            'id' => $dataInputId,
-            'validators' => ['callback' => function ($data) use ($termContainer) {
-                $changes = [];
-                $invalid = $this->validateTerms($data['type'], $data['terms'] ?? [], $changes);
-                $this->changes = $changes;
-
-                $terms = $this->getTerms();
-                foreach ($changes as $index => $termData) {
-                    $terms[$index]->applyTermData($termData);
-                }
-
-                return ! $invalid;
-            }]
-        ]) extends HiddenElement {
-            /** @var TermInput */
-            private $parent;
-
-            public function setParent(TermInput $parent): void
-            {
-                $this->parent = $parent;
-            }
-
-            public function setValue($value)
-            {
-                $data = json_decode($value, true);
-                if (($data['type'] ?? null) === 'paste') {
-                    array_push($data['terms'], ...array_map(function ($t) {
-                        return ['search' => $t];
-                    }, $this->parent->parseValue($data['input'])));
-                }
-
-                return parent::setValue($data);
-            }
-
-            public function getValueAttribute()
-            {
-                return null;
-            }
-        };
-        $dataInput->setParent($this);
-
-        $label = $this->getLabel();
-        $this->setLabel(null);
-
-        // TODO: Separator customization
-        $mainInput = $this->createElement('text', 'value', [
-            'id' => $searchInputId,
-            'label' => $label,
-            'required' => $this->isRequired(),
-            'placeholder' => $this->translate('Type to search. Separate multiple terms by comma.'),
-            'class' => 'term-input',
-            'autocomplete' => 'off',
-            'data-term-separator' => ',',
-            'data-enrichment-type' => 'terms',
-            'data-with-multi-completion' => true,
-            'data-no-auto-submit-on-remove' => true,
-            'data-term-direction' => $this->getTermDirection(),
-            'data-maintain-term-order' => $this->getOrdered() && ! $this->getAttribute('disabled')->getValue(),
-            'data-read-only-terms' => $this->getReadOnly(),
-            'data-data-input' => '#' . $dataInputId,
-            'data-term-input' => '#' . $termInputId,
-            'data-term-container' => '#' . $termContainer->getAttribute('id')->getValue(),
-            'data-term-suggestions' => '#' . $suggestionsId
-        ]);
-        $mainInput->getAttributes()
-            ->registerAttributeCallback('value', function () {
-                return null;
-            });
-        if ($this->getSuggestionUrl() !== null) {
-            $mainInput->getAttributes()->registerAttributeCallback('data-suggest-url', function () {
-                return (string) $this->getSuggestionUrl();
-            });
-        }
-
-        $this->addElement($termInput);
-        $this->addElement($dataInput);
-        $this->addElement($mainInput);
-
-        $mainInput->prependWrapper((new HtmlElement(
-            'div',
-            Attributes::create(['class' => [
-                'term-input-area',
-                $this->getTermDirection(),
-                $this->getReadOnly() ? 'read-only' : null
-            ]]),
-            $termContainer,
-            new HtmlElement('label', null, $mainInput)
-        )));
-
-        $this->addHtml($suggestions);
+        parent::beforeRender();

-        if (! $this->hasBeenAutoSubmitted()) {
-            $this->emit(self::ON_ENRICH, [$this->getTerms()]);
+        if ($this->hasBeenAutoSubmitted() && ! $this->multipartUpdatePrepared) {
+            $this->emit(static::ON_ENRICH, [$this->getTerms()]);
        }
    }


This emits ON_ENRICH after validation on every render. assemble() used to emit it only once. Please emit it only once here too. A short comment explaining why the deprecated TermInput needs this would also help.

To reproduce, put a TermInput with an ON_ENRICH listener into a FieldsetElement of a CompatForm, handle a request with X-Icinga-Autosubmittedby: <input id>-search-input and render the form. The listener already runs twice in that one render(), because beforeRender() is reached twice via renderWrapped(). Every further render() runs it again.

lippserd · 2026-10-14T08:44:03Z

+                return ! $invalid;
+            }]
+        ]) extends HiddenElement {
+            private ?TermInputElement $parent;


null is never assigned.

Suggested change

private ?TermInputElement $parent;

private TermInputElement $parent;

nilmerg self-assigned this Oct 1, 2025

cla-bot Bot added the cla/signed label Oct 1, 2025

nilmerg changed the title ~~Make term input decoratorable~~ Make term input decorable Oct 1, 2025

BastianLedererIcinga force-pushed the make-term-input-decoratorable branch from 00646dd to 5bcb5e2 Compare October 6, 2026 13:47

BastianLedererIcinga marked this pull request as ready for review October 6, 2026 14:13

BastianLedererIcinga requested a review from lippserd October 6, 2026 14:16

BastianLedererIcinga assigned lippserd Oct 6, 2026

BastianLedererIcinga mentioned this pull request Oct 7, 2026

Configure relations Icinga/icingadb-web#1417

Merged

nilmerg and others added 7 commits October 7, 2026 09:46

TermInput: Override getLabel(), not the actual *label*

f5bef68

We don't want a label for the term input, so let's ensure it doesn't get one with either decorator implementation.

TermInput: Assemble upon preparing a multipart update

8aec362

Plus another small correction I accidentally noticed.

TermInput: Rename to TermInputElement

8ce9ddf

Allows to load it using the form element loading of `CompatForm`

Deprecate the old TermInput class

f4051c8

TermInputElement: Add todo

d2c362f

TermInputElement: Detect autosubmission lazily

06a1c76

The request was read in `onRegistered()`, which failed if no request was set yet. It's now read when `hasBeenAutoSubmitted()` is first called, at the end of the input's `assemble()`.

BastianLedererIcinga force-pushed the make-term-input-decoratorable branch from 5bcb5e2 to a89b417 Compare October 7, 2026 07:47

nilmerg commented Oct 9, 2026

View reviewed changes

lippserd requested changes Oct 14, 2026

View reviewed changes

Navigation Menu

Repository navigation

Make term input decorable - #317

Make term input decorable#317
nilmerg wants to merge 7 commits into
mainfrom
make-term-input-decoratorable

nilmerg commented Oct 1, 2025 •

edited by BastianLedererIcinga

Loading

Uh oh!

nilmerg Oct 9, 2026

Uh oh!

nilmerg Oct 9, 2026

Uh oh!

lippserd left a comment

Uh oh!

lippserd Oct 14, 2026

Uh oh!

lippserd Oct 14, 2026

Uh oh!

lippserd Oct 14, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	private ?TermInputElement $parent;
	private TermInputElement $parent;

Conversation

nilmerg commented Oct 1, 2025 • edited by BastianLedererIcinga Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nilmerg Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

nilmerg Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

lippserd left a comment

Choose a reason for hiding this comment

Uh oh!

lippserd Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

lippserd Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

lippserd Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nilmerg commented Oct 1, 2025 •

edited by BastianLedererIcinga

Loading