Fix the style demo installation - #444
Open
ECYaz wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes demo style installation failures during approval/management by ensuring the remote hook runs with an initialized phpBB session and by making demo install failures degrade cleanly (instead of fatalling/aborting approval or returning a broken AJAX response).
Changes:
- Start a session in the phpBB 3.3.x demo install hook before invoking
acp_styleslogic. - Prevent demo install failures/exceptions from aborting approval/AJAX by catching throwables and returning an explicit failure message when no URL is produced.
- Make hook fetch failures degrade to an error result rather than turning warnings into exceptions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
misc/style_demo_hooks/style_demo_hook.php |
Adds an explicit manager property used by the hook runner. |
misc/style_demo_hooks/phpBB_3.3.x/style_demo_install.php |
Initializes session/auth/user setup prior to running the install hook. |
language/en/contributions.php |
Adds a user-facing language string for demo install failure. |
controller/contribution/manage.php |
Returns a proper failure message payload when no demo URL is produced. |
contribution/style/type.php |
Ensures demo failures don’t abort approval/AJAX by catching \Throwable. |
contribution/style/demo/manager.php |
Suppresses hook warnings to preserve JSON replies (but needs a small fix for false return handling). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The demo board hook fatals on any current 3.3 board: acp_styles logs the installation through $phpbb_log->add() using the session's user id and ip, and style_demo_install.php never starts a session. The hook predates that core change; add_log() used to guard against an empty user. Begin a session before running the manager. The failure also surfaced badly on the Titania side. The hook's error page raised a warning inside file_get_contents() that error handlers convert to an exception, so the moderator saw a broken AJAX reply instead of a result, and a failed installation on the approval path aborted the page. Degrade a failed hook call to the existing error result instead, and answer the manage page's install request with a proper message when no URL was produced. Declares the two properties the demo classes created dynamically.
ECYaz
force-pushed
the
fix/410-style-demo-install
branch
from
July 30, 2026 16:21
d557939 to
d120d45
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #410
The demo board hook fatals during installation: acp_styles logs the install through $phpbb_log->add() using the session's user id and ip, and style_demo_install.php never starts a session. The hook predates that core change (phpBB 3.2.0, ticket 13468), add_log() used to guard against an empty user. The style row is committed before the fatal, so retries appear to work.
The hook now starts a session first, a failed hook call degrades to the existing error result, the install button answers with a proper message when no URL was produced, and a demo failure can no longer abort the approval.
Note the live hook's auth phase responds fine, so if the install still fails on phpbb.com after this, the remaining cause is in the install leg (paths, database access or Cloudflare between the servers) and will now be visible in the logs instead of an AJAX error.