[3.0] Load the current user before finalize() records dates - #9316
Open
albertlast wants to merge 1 commit into
Open
[3.0] Load the current user before finalize() records dates#9316albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
Install::finalize() called Time::strftime() to build the log_activity date, and later Logging::updateStats(), which does the same thing. Both end up in Time::__construct(), which reads User::$me to resolve the time zone. But User::setMe()/User::loadMe() were not called until much later in the same method, so installation died with: Error: Typed static property SMF\User::$me must not be accessed before initialization in Sources/Time.php:191 The installer therefore aborted on its last step, leaving the forum without the member, topic and message stats that finalize() is responsible for writing, including latestMember and latestRealName. Moves the user initialisation up to just after the settings are reloaded, which is the first point at which it can run, and leaves the rest of the "we've just installed" block where it was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
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.
Note
This change was produced by an LLM. The code, the commit message and this
description were all written by Claude (Anthropic), driven by @albertlast. It has
not yet had human code review.
Everything stated below was verified by actually running it against a fresh
PostgreSQL install of this branch, rather than only reasoned about. Even so,
please review it as untrusted work: the diagnosis may be right while the fix is
not what SMF would prefer stylistically or architecturally.
Description
Install::finalize()callsTime::strftime()to build thelog_activitydate, and alittle later
Logging::updateStats(), which does the same thing. Both end up inTime::__construct(), which readsUser::$meto resolve the time zone:But
User::setMe()/User::loadMe()are not called until roughly a hundred linesfurther down the same method, so installation dies on its final step:
The installer aborts at step 7 of 7, after the schema and the admin account have been
created, so the forum looks installed but
finalize()never ran. Everything it isresponsible for is missing, including
latestMemberandlatestRealName, which thenproduce their own warnings on the board index.
This moves the user initialisation up to just after the settings are reloaded, which
is the first point at which it can run, and leaves the rest of the "we've just
installed" block where it was.
Worth noting: fixing only the
strftime()call would not have been enough, becauseLogging::updateStats()reaches the same code viaSources/Logging.php:466a fewlines later and would have failed identically.
How this was verified
Full install from an empty database, PostgreSQL 17 / PHP 8.4:
latestMember,totalMembers,totalTopics,totalMessagesall absent; nolog_activityrow.log_activityrow that previouslycrashed, sets all four settings, and logs zero errors across the whole install.
This conflicts with #9302 ("[3.0] Upgrader login fixes"), which is actively
refactoring the same method. I test-merged them to confirm rather than guessing:
Sources/Maintenance/Tools/Install.phpconflicts.The conflict is purely textual. #9302 rewrites
Maintenance::$contexttoUtils::$contextthroughoutfinalize(), including the lines immediately aroundwhere this patch inserts. It does not fix this bug — every one of its hunks in
finalize()is that rename, thestrftime()call stays at line 1151 and thesetMe()/loadMe()block stays ~100 lines below it.Since #9302 is already touching this method, folding the reordering into it may be
tidier than merging both. Happy to rebase this onto #9302, or to close it if that is
the preferred route. The other PRs in this batch all merge onto #9302 cleanly; this is
the only one that does not.
Issues References (Fixes|Related|Closes)