Skip to content

Introduce missing first and htmlToPdf methods to PdfexportHook#5491

Open
TheSyscall wants to merge 3 commits into
mainfrom
pdf-hook
Open

Introduce missing first and htmlToPdf methods to PdfexportHook#5491
TheSyscall wants to merge 3 commits into
mainfrom
pdf-hook

Conversation

@TheSyscall
Copy link
Copy Markdown

@TheSyscall TheSyscall commented Apr 8, 2026

This PR adds two methods to the pdfexport hook: first and htmlToPdf

Both functions were previously called without actually being part of the hook.
The only possible implementation of this hook comes from the pdfexport module which already defines the same two methods.

@TheSyscall TheSyscall requested a review from Al2Klimov April 8, 2026 08:51
@cla-bot cla-bot Bot added the cla/signed label Apr 8, 2026
@TheSyscall TheSyscall self-assigned this Apr 8, 2026
TheSyscall added a commit to Icinga/icingaweb2-module-pdfexport that referenced this pull request Apr 8, 2026
@TheSyscall TheSyscall changed the title Introduce htmlToPdf method Introduce PdfexportHook::htmlToPdf method Apr 8, 2026
Al2Klimov

This comment was marked as resolved.

Comment thread library/Icinga/Application/Hook/PdfexportHook.php Outdated
It made little sense that we ask a concrete hook implementation for the
first implementation of a hook. It essentially limited us to exactly one
possible pdfexport extension (our own), which is not how hooks are
supposed to work.
This method was always required because it was called by the reporting
module assuming that our own implementation of the pdfexport module was
the one chosen.

Because this behavior exists and our pdfexport module is the only
implementation of this hook this is not a breaking change.
This commit also includes phpdoc formating changes
@TheSyscall TheSyscall changed the title Introduce PdfexportHook::htmlToPdf method Introduce missing first and htmlToPdf methods to PdfexportHook May 20, 2026
@TheSyscall TheSyscall requested a review from Al2Klimov May 20, 2026 11:12
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