Skip to content

Add V2/Source hook for icinga-notifications-web#1369

Open
sukhwinder33445 wants to merge 8 commits into
mainfrom
update-notifications-hook
Open

Add V2/Source hook for icinga-notifications-web#1369
sukhwinder33445 wants to merge 8 commits into
mainfrom
update-notifications-hook

Conversation

@sukhwinder33445
Copy link
Copy Markdown
Contributor

@sukhwinder33445 sukhwinder33445 commented Apr 28, 2026

This PR adds a new V2/Icinga2Source hook implementation to integrate with the V2/SourceHook interface from icinga-notifications-web.

Supporting changes required by the hook:

  • Introduce QueryValuesProvider — Decouples value suggestion logic from ObjectSuggestions into a standalone Generator-based class. This allows the hook's getValueSuggestions() to reuse the same suggestion infrastructure without a Suggestions context.
  • QueryColumnsProvider: Add setShowRelationLabels() setter — Exposes the relation label display option so the hook can control it.
  • QueryColumnsProvider: Make collectFilterColumns() protected — No longer needs to be public; the override in SearchControls that called it statically is removed as the optimization it provided is no longer necessary.
  • ObjectSuggestions: Remove label-to-path resolution for columns with spaces to ensure consistency between the suggestions in the search bar and suggestions provided by the Icinga2Source.

resolves: #1365

Requires:

@cla-bot cla-bot Bot added the cla/signed CLA is signed by all contributors of a PR label Apr 28, 2026
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 2 times, most recently from ded5a93 to c3c926f Compare April 28, 2026 07:56
@sukhwinder33445 sukhwinder33445 requested a review from Copilot April 28, 2026 07:56

This comment was marked as resolved.

@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 2 times, most recently from c3c926f to 9f7ac77 Compare April 28, 2026 08:22
@sukhwinder33445 sukhwinder33445 requested a review from Copilot April 28, 2026 08:27
@sukhwinder33445 sukhwinder33445 self-assigned this Apr 28, 2026

This comment was marked as resolved.

@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 2 times, most recently from 7d02f46 to 680cc30 Compare April 28, 2026 09:11
@BastianLedererIcinga
Copy link
Copy Markdown
Contributor

I reduced the SourceHook interface in Icinga/icinga-notifications-web#469, feel free to comment if you'd like anything changed about it.
Adjusting to the new implementation shouldn't be too complicated, the callables that are currently being registered on the SearchEditor in Source::getRuleFilterEditor() can be used to implement the new interface functions with minor adjustments.

@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 3 times, most recently from 912ca23 to f1465b1 Compare May 5, 2026 12:34
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 4 times, most recently from 497472d to f027079 Compare May 6, 2026 12:40

This comment was marked as resolved.

@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from f027079 to ee166d4 Compare May 6, 2026 14:16
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Source.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 2 times, most recently from 4da5fe6 to bb489df Compare May 11, 2026 06:41
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 3c0a7e1 to 37d7a17 Compare May 19, 2026 07:40
Comment thread run.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 37d7a17 to 0e73027 Compare May 19, 2026 10:15
Comment thread library/Icingadb/Data/QueryValuesProvider.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 0e73027 to 476c960 Compare May 19, 2026 13:39
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 476c960 to 3adab77 Compare May 20, 2026 07:52
@sukhwinder33445 sukhwinder33445 requested a review from nilmerg May 20, 2026 07:54
@nilmerg nilmerg force-pushed the update-notifications-hook branch from effff0c to bd7e1c0 Compare May 20, 2026 13:00
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 41e3c57 to 43d32f0 Compare May 21, 2026 07:40
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
Comment thread library/Icingadb/ProvidedHook/Notifications/V2/Icinga2Source.php Outdated
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch 2 times, most recently from 76f44ab to 51b99f3 Compare May 21, 2026 09:11

This comment was marked as resolved.

@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 51b99f3 to 9c43f91 Compare May 21, 2026 10:34
Copy link
Copy Markdown
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase

sukhwinder33445 and others added 8 commits May 21, 2026 16:34
Decouple value suggestion logic into a standalone `Generator` instance.
This allows callers to fetch column values without any `Suggestions`
context. A plain `foreach` on the provider is enough.

`ObjectSuggestions::fetchValueSuggestions` is reduced to a thin wrapper
that resolves label-to-path input before delegating to the new class.
Moving the code to `QueryValuesProvider` does not seem helpful, as
this covers a rare case that is not really necessary.

To ensure consistency between the suggestions in the search bar and in
`SourceHook`, this must be removed.
Remove the overwritten method `fetchFilterColumns` of `SearchControls`
trait as this optimization is no longer necessary. Since the method is
now only used within its own class, it can be made protected.
The code has been moved directly to the `QueryValuesProvider` class.

`QueryValuesProvider`: Bypassing the ORM is no longer required.
necessary and has been removed. The yield type must be an array
containing at least the keys `search` and `label` for `searchSuggestion`
to work. Previously these keys were created by wrapper classes.
Co-authored-by: Johannes Meyer <johannes.meyer@icinga.com>
- `assertValidCondition()`: prevent columns with wildcards other than a
trailing [*].
- `enrichCondition()`: Remove trailing [*] as we usually do when
  rerendering the condition.
@sukhwinder33445 sukhwinder33445 force-pushed the update-notifications-hook branch from 9c43f91 to b38d2e8 Compare May 21, 2026 14:34
@nilmerg nilmerg dismissed BastianLedererIcinga’s stale review May 21, 2026 14:53

All of his discussions are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement updated integration for Icinga Notifications Web

4 participants