Skip to content

Add a generic hook system: Actions, Filters and Resolvers - #2127

Open
kainhofer wants to merge 45 commits into
Admidio:masterfrom
kainhofer:ActionsFilters_Hooks
Open

Add a generic hook system: Actions, Filters and Resolvers#2127
kainhofer wants to merge 45 commits into
Admidio:masterfrom
kainhofer:ActionsFilters_Hooks

Conversation

@kainhofer

Copy link
Copy Markdown
Contributor

Summary

Adds Admidio's foundation for extensibility: Actions (notify), Filters (transform), and Resolvers (ask for one answer), dispatched by a single static registry (Admidio\Hooks\Hooks). The goal is to let a plugin observe or adjust core behaviour — a value before it's saved, a form before it's rendered, a login attempt, a list row — without patching core files.

Documentation: https://www.admidio.org/dokuwiki/doku.php?id=en:entwickler:hooks

The design has three layers:

  • Generic entity hooks (entity_created, entity_value, …) for genuinely cross-cutting consumers — an audit log, a sync framework.
  • Entity-specific hooks (oidc_client_created, event_updated, …), the normal plugin-facing CRUD API. Both layers are dispatched automatically from one central Entity lifecycle, for all 36 entities that opted in — nothing is hand-wired per module.
  • Semantic/system hooks where the interesting boundary isn't one entity save: forms (form_built, form_select_options), pages (page_title, page_before_render), components (component_visible), translation (translation_missing as a resolver), login/logout, email, and a first generic list vocabulary (list_columns, list_data, list_row_actions, list_rendered_data) proven end-to-end on the Contacts list.

Persistence hooks are transaction-aware: dispatched only after the outermost commit, coalesced when one record is saved twice in a transaction, and carry an immutable EntityChangeSet (old/new values per field, a snapshot for deletions) so a listener never needs its own before/after cache. ChangeNotification — the existing "mail admins about profile changes" feature — was migrated onto this as the first real consumer, replacing 14 hand-written call sites with three listeners.

Not in this PR: a plugin loading/discovery mechanism (deliberately out of scope — this PR is the hook engine, not the plugin system that would register callbacks into it), and converting the remaining list-based modules (Inventory, Category Report, Changelog) to the new list hooks, which Contacts alone was chosen to prove first.

General Idea

The general idea of filters and actions is similar to the WordPress approach, which makes WP so versatile and easy to tweak. The modifications should not need to implement certain classes or interfaces or follow strict rules. Simply registering one callback function that will be called automatically by the core is enough to modify core behavior.

This actions/filters/resolvers feature also needs a generic plugin system, which I'm currently also designing and implementing. Having an easy, simple plugin system together with this actions/filters/resolvers provides an enormous amount of possibilities to configure / modify even core behavior and easily extend admidio in ways I can't even think about yet.

Simple examples

Add some JS to the settings page: Place this php code in some file that is loaded (e.g. at the end of common.php for now, later these callbacks will typically be registered by plugins):

Admidio\Hooks\Hooks::addAction('page_before_render', function (Admidio\UI\Presenter\PagePresenter  $page) {
    if ($page->getHtmlID() === 'adm_preferences') {
        $page->addJavascript('alert("Discuss all changes with the webmaster first.");', true);
    }
});
image

That's it... All other uses are equally simple, like restricting the email field to a certain domain when modifying profiles:

Admidio\Hooks\Hooks::addFilter('user_data_value', function (mixed $value, Admidio\Infrastructure\Entity\Entity $userData, string $columnName, mixed $oldValue) {
    if ($columnName !== 'usd_value') {
        return $value;
    }
    // reject an email outside the association's domain
    if ($userData->getValue('usf_name_intern') === 'EMAIL' && !str_ends_with($value, '@example.org')) {
        throw new Exception('Only @example.org addresses are allowed.');
    }
    return $value;
});

image

The profile change mail notifications were already modified to listen to the user_created/user_updated/user_deleted/user_data_.../membership_... actions instead of being hardcoded in the setValue calls. ChangeNotificatiion::registerListeners:

    protected function registerListeners(): void
    {
        foreach (array('created', 'updated', 'deleted') as $stage) {
            Hooks::addAction('user_' . $stage, array($this, 'onUserChanged'), Hooks::DEFAULT_PRIORITY, 1, 'change_notification');
            Hooks::addAction('user_data_' . $stage, array($this, 'onUserDataChanged'), Hooks::DEFAULT_PRIORITY, 1, 'change_notification');
            Hooks::addAction('membership_' . $stage, array($this, 'onMembershipChanged'), Hooks::DEFAULT_PRIORITY, 1, 'change_notification');
        }
    }

The mode=list view of the announcements was:
* broken with PHP errors crashing
* copied from forum and never adjusted to announcements
* never reachable from the UI (forum provides the choice between list and cards, announcements always defaults to cards)
* Announcements should be prominently visible, so a list view defies that purpose anyway.

So, this commit removes the broken announcements list view for good.
It's presence confused Claude.ai, who always tried to fix it.
A callback given by its function name could not be removed again, an explicit ID was
only unique within one priority, and every exception of a callback was swallowed.
Registrations are records of their own now, exceptions propagate, and resolvers and a
reset for tests are part of the API.
It fired between the insert and the commit, before the profile fields were written and
the session was reloaded.
setValue() overwrote previousValue on every change, so a field that was set twice before
the save logged the intermediate value as the old one, and a field set back to its
original value was logged and written again.
A caller that sets columnsValueChanged for a connected object, User::save() for its
profile fields for example, could reach the update path with no own column to set.
validate() only checked a single-valued select, so an array value and a radio group were
never compared with the entries the form was built with. A control that deliberately
takes free input opts out with allowCustomValues.
The IDs were split between adm_* and admidio-*. They are now all adm_<module>[_<view>],
following the install steps. Three were copied from another page and named their new one
instead: the SSO key page, the password reset page and the contact assignment page.
An immutable value object built from the change tracking that Entity keeps anyway, so a
hook callback does not need a before/after cache of its own. It also carries the snapshot
of a deleted record, which the cleared object can no longer provide.
An entity that returns a hook ID dispatches the generic and its own hooks around every
insert, update and delete, and entity_value filters a value before setValue() checks it.
An entity without a hook ID stays silent, and the installation and the update switch the
hooks off entirely.
The hook ID is a public API, so it is stated per entity instead of being derived from the
class name. Sessions, auto logins, the changelog and the OAuth tokens stay unnamed and
therefore silent, and User, Key and OIDCClient withhold their secrets from a change set.
They run on the CLI without a server: FakeDatabase is a Database over in-memory SQLite, so
save() and delete() really write and the lifecycle is executed and not simulated.
Nested transactions only commit at the outer end, so anything that must not describe a
change until it is really in the database needs somewhere to wait. A transaction that is
rolled back, fails to commit or is simply left open at the end of the request runs the
other queue instead.
The post-actions now run at the commit and no longer at the statement, and the saves of one
record within one transaction become the one change that happened as far as anybody outside
can tell. User::save() loses its hand-written user_created, which Entity dispatches with a
change set.
FakeDatabase commits and rolls back for real now. BufferedStatement is needed because the
SQLite driver of PDO answers rowCount() with 0 for a SELECT, which is the number
Entity::readData() decides on.
deleteDependentRecords() removed its records with one DELETE, so a membership or
a profile value that disappeared with its owner never reached the hook API. Every
record is now described before the statement runs, and the change set names the
record whose deletion caused it.
The transformation of getValue() moved into formatValue(), so a value that does
not come from the loaded user can be shown the same way.
…e sets

ChangeNotification listened to nothing and was called by hand from fourteen
places in User and Membership, four of them in a setValue(), so a value that was
never saved was mailed anyway. It is now a listener on the committed hooks of
adm_users, adm_user_data and adm_members, and it reports user_state_changed once
per affected user.
clear() set the column and left it marked as changed, so every user that was
read from the database carried a pending activation: each save wrote the column
back and reported it as a change from empty to true. The default now belongs to
the creation of the record, and an activation is logged like any other change.
A form was finished implicitly by whoever rendered it, and getElements()
returned a copy, so nothing could change a form it did not build. finalize()
now marks that point once per form and dispatches form_built there, with an
element API to act on it. The stored form is the rendered one, so a removed
element is also rejected by validate().
Only the users who really had a value in the field are described, because
hookBulkDeletion() reads the rows that exist, and none of them is reported as a
change of that person.
setHtmlID() wrote the member only, but the template variables are assigned in
the constructor and every page sets its ID afterwards, so <body id> was always
empty. show() now assigns it together with the title and the headline, after
page_title, page_headline and page_before_render have run.
The rights of each component were decided in one switch with twenty exits, so
nothing could adjust them. isVisible() and isAdministrable() are now wrappers
around those switches that apply component_visible and component_administrable,
and the category report asks the component instead of checking the right itself.
translation_missing supplies a text that no language file has, and its answer
joins the text cache, so it is asked once per session like any other lookup.
translation_text runs after the cache on every call, because it is the only one
of the four that may depend on the request. translation_fallback_used and
translation_unresolved are diagnostics and cannot break the page.
Its visibility asked the right only, so the menu entry and the eight CLI
commands stayed when the module was disabled and answered with an error page.
Every other module asks its setting first.
form_select_options runs at the end of finalize(), after form_built, and only
for the types whose entries validate() checks, so removing an entry also means
a request that submits it is refused. validate() finishes the form too, so a
form that is validated right after it was built is judged against the same
elements as one that came out of the session.
…xception

An attempt was observable nowhere, so a rate limit or an audit trail had no
place to sit. The hooks are at the attempt boundary and not in
User::checkLogin(), because a user name that belongs to nobody is refused before
a User exists. None of them is given the password, and the two diagnostics
cannot replace the reason a login was refused.
A mail had no place for a suppression list and no signal when it failed. The
recipients are filtered once, before the delivery splits them into packages, and
the two diagnostics are given the subject, the count and the error. None of the
three receives the message itself: PHPMailer keeps the SMTP user name and
password in public properties.
The ad-hoc changelog_headline filter duplicated what PagePresenter::show()
already does through page_headline once the page renders; removed.
user_readable_name only ever reached User. Entity::readableName() and its
five overrides now end by calling the new Entity::filterReadableName(),
which dispatches the generic entity_readable_name and, for an entity with
a hook ID, <hookId>_readable_name as well - the same generic-then-specific
order the persistence lifecycle already uses.
…eneral PagePresenter capability

category_report_title and category_report_headline duplicated page_title
and page_headline, but the headline is also needed for the export filename
and the PDF header, neither of which ever reaches PagePresenter::show() -
removing the ad-hoc filter outright would have silently stopped filtering
those, while html/print were already filtering the headline twice (once
ad-hoc, once through show()).

PagePresenter gains getFilteredTitle()/getFilteredHeadline(), which run
page_title/page_headline and cache the result behind a $textFiltered flag,
the same one-shot guarantee finalize() already gives a form's elements.
show() calls the same internal filterText(), so asking for the filtered
text before show() and letting show() run afterward dispatches the filters
exactly once either way. category_report.php now builds its page and calls
setTitle()/setHeadline() once, unconditionally, including for csv, and
reads the filtered text back for the filename, the PDF header and both
render modes.
role_right_data was the class name lower-cased rather than the word a
plugin author would reach for; role_right_assignment says what the table
holds. Nothing consumes the old name yet, so there is nothing to migrate.
ItemsData::deleteItem() logged ItemData, ItemBorrowData and the Item
itself to the changelog but never called hookBulkDeletion(), so
inventory_item_deleted and the other two never fired for the one place
an item is actually deleted. Added next to each existing logging call,
without changing the DELETE statements themselves - hookBulkDeletion()
reports what a condition selects, it does not decide what gets removed,
so the item's own organization guard stays exactly as it was.
RolesDependencies::delete() overrides Entity::delete() because the table
has a composite key, and logged the deletion without ever calling
hookBulkDeletion(), so role_dependency_deleted and its generic entity_*
counterpart never fired for the one place a dependency is actually
removed. Fixed by capturing the two ids before hookBulkDeletion() clears
them and calling it next to the existing logDeletion(), same shape as the
inventory item fix. Verified against the real class, not a stand-in - its
constructor takes nothing but a Database.
A registration record disappearing and a user becoming valid look like
two unrelated entity changes, whether an administrator approves a pending
registration or a returning user's self-registration is merged into their
existing account. Both paths now dispatch user_registration_accepted with
the resulting User and one of UserRegistration::ACCEPTED_BY_APPROVAL /
ACCEPTED_BY_ASSIGNMENT.

This is the first hook that does not belong to one entity's lifecycle, so
it uses Database::registerAfterCommit() directly - the same primitive
EntityHookQueue itself calls - registered right after each method's own
endTransaction(). That is correct however deep the nesting goes:
CoreTasks::registrationApprove() wraps acceptRegistration() in a
transaction of its own to also assign the requested groups, and the event
now waits for that outer commit instead of firing early, and drops
entirely if that scope rolls back.
list_columns, list_data, list_rendered_data and list_row_actions (§2.7)
now exist, dispatched from contacts.php/contacts_data.php with the
stable listId 'contacts' - the module the plan names first and the one
whose per-row action icons make it the clearest fit for
list_row_actions.

contacts_data.php still builds every row by column position, and there
is no shared list/column pipeline yet to keep a filtered column list
and the row data in step - so list_columns may relabel a column but not
add or remove one, enforced by an explicit count check rather than
trusted. The fetch mode changed from PDO::FETCH_BOTH to FETCH_ASSOC:
nothing in the row-building code reads a numeric index, so the
duplicate keys FETCH_BOTH added were never used and would only have
confused a list_data filter.

Converting InventoryItemPresenter, CategoryReport and changelog_data.php
to the same vocabulary is not part of this change; each builds its rows
differently enough to need its own pass.
…ge set

entity_updating and its relatives received the EntityChangeSet and
nothing else, so a callback could observe what changed and veto by
throwing, but could not read a field the change set does not carry, call
a method on the record, or connect it to a plugin's own data before the
save completes.

dispatchHook() now passes $this as a second argument to every stage
except deleted and delete_failed, which get null: delete() clears the
object before either can be dispatched - immediately for a failure
outside a transaction, later through the commit/rollback queue for
everything else - and a bulk deletion reuses one scratch object for
every row it removes, so it would as often be the wrong record as an
empty one. EntityChangeSet::getSnapshot() is what those two describe the
record from.

The extra argument is additive: a callback registered as
function (EntityChangeSet $changeSet) keeps working unchanged, since
Hooks::call() spreads the dispatch arguments and PHP does not error when
a callback declares fewer parameters than it is given.
Switching to FETCH_ASSOC when list_data was added assumed nothing read
a numeric index into $row - checked by grepping for a literal digit,
which missed $row[$ColumnNumberSql] a little further down. That loop
walks the list-configuration's dynamically named columns by position,
so every list-configuration column, name included, silently rendered
empty. list_data still filters the same row either way; only the fetch
mode was wrong.
ProfileFields::saveUserData() clears a profile field by calling
setValue('usd_value', '') and then, seeing the record is now empty,
deletes it rather than saving it. usd_value is nullable, so setValue()
already turns '' into null and overwrites dbColumns with it before
delete() runs. buildDeletionChangeSet() read every column straight from
dbColumns, so the deletion's change set reported null -> null instead of
the value that was actually cleared, and ChangeNotification's
recordChange() - correctly - treats an unchanged old/new pair as no
change and drops it from the mail.

Fixed by reading previousValue for a column that has already been
changed since it was read or saved, and dbColumns - unaffected either
way - for one that has not, the same source the create/update change-set
builder already uses. getSnapshot() inherits the same value through
buildChangeSet(), so it needed no separate fix. Only the hook change set
was wrong; Entity::logDeletion() never logged a per-column before-value
for a deletion in the first place, so the changelog was never affected.
readDataByFirstnameLastName() always calls clear(), which no longer resets
usr_valid to active. When no contact matched, the object kept clear()'s empty
defaults instead of becoming a new record, so imported contacts were saved
inactive and invisible to that same lookup, causing duplicate imports.
usr_valid became a value of the record, so it is no longer the technical noise
the test treated it as. The not-logged case now uses the login counters, and a
new test covers the change usr_valid actually produces.
relation:list stopped emitting the internal user ids when the command was scoped
to the organization. The assertion now reads user1_uuid, which shows the same
thing: the two rows are the two directions of the relation.
@kainhofer
kainhofer force-pushed the ActionsFilters_Hooks branch from 4ce0172 to 07fb2dc Compare September 1, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants