Repository navigation
fix(ui): store UI states per row so saves are immediate and can't overwrite each other - #3947
Merged
Merged
Conversation
JSON-RPC requests were stateless: leantime.rpc read the user's web session but its writes were dropped. Every saved UI state (accordion open/closed, and soon sidebar widths) went to the database only, so the next page rendered the old value from the session and was right only one load later. - StartSession persists browser RPC requests (AJAX to /api/jsonrpc with the session cookie) like web requests: lock-free, changed keys merged on save. API clients without a session cookie and cron stay stateless. - Menu::setSubmenuState writes on top of the stored states instead of the request's session copy, so a state saved by another request in the meantime isn't dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbcnaZTbGANSNAWkGT6HGs
marcelfolaron
requested review from
broskees
and
a balanced review from Copilot
and removed request for
a team
October 18, 2026 15:25
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Overlapping requests can still lose database preference updates and overwrite newer session state.
2 open issues
What changed in this PR
Fixes delayed UI-state restoration by persisting the app’s browser JSON-RPC session changes.
Changes:
- Enables session persistence for AJAX JSON-RPC requests carrying a session cookie.
- Merges submenu saves with stored preferences.
- Adds session-classification and preference-preservation tests.
| File | Description |
|---|---|
| tests/Unit/app/Domain/Menu/Repositories/SubmenuStateTest.php | Tests preservation of stored preferences and session fallback. |
| tests/Unit/app/Core/Middleware/BrowserRpcSessionTest.php | Tests which requests persist sessions. |
| app/Domain/Menu/Repositories/Menu.php | Reads stored preferences before saving submenu state. |
| app/Core/Middleware/StartSession.php | Enables persistence for browser RPC sessions. |
Review effort: Balanced
Rework of the first approach after review: persisting browser JSON-RPC
sessions joined more writers to a session merge that only works per
top-level key (all UI states share `usersettings`), and the serialized
submenuToggle blob was still a read-modify-write. Both could still drop a
state saved at the same time.
Instead, UI states leave the session:
- Leantime\Core\UI\UiState stores each state as its own zp_settings row
("usersetting.{id}.ui.{name}") with one atomic upsert, and reads all of
the user's states once per request (scoped binding, lazy connection).
The old serialized submenuToggle blob is read as a fallback, so saved
choices carry over.
- Menu::setSubmenuState / getSubmenuState, Template::getToggleState, the
sidebar menu, the project selector groups, elements.accordion and both
calendars' saved view read and write through it; nothing writes
usersettings.submenuToggle to the session any more.
- StartSession is back to the original: JSON-RPC requests don't persist
the session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbcnaZTbGANSNAWkGT6HGs
marcelfolaron
changed the title
marcelfolaron
added a commit
that referenced
this pull request
Oct 18, 2026
The content box now reads saved sidebar widths from UiState (one row per state), the store #3947 introduces, instead of the session copy, which JSON-RPC saves never updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbcnaZTbGANSNAWkGT6HGs
From review:
- zp_settings.key compares case-insensitively on MySQL, so "Foo" and
"foo" would have shared one row. Stored names now escape everything
outside [a-z0-9_-] ("~" + hex), so distinct names stay distinct under
any collation.
- The old store wrote through a dot-notation session path, so a name with
dots was saved nested in the serialized blob. The fallback flattens it
back (Arr::dot), so those choices carry over too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbcnaZTbGANSNAWkGT6HGs
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.
Saved UI states showed up one page load late. Close an accordion, reload, and it's open again; reload once more and it's closed. Two states saved at the same moment could also overwrite each other.
Why
Api.Api.setSubmenuState), and JSON-RPC requests don't persist the session. The session copy (usersettings.submenuToggle) was dropped and only the database blob kept the change, so the next page rendered the old value.usersettings. The session merge from perf: speed up dashboard & hot paths (session locking, query dedupe, event patterns) #3439 works per top-level key, so parallel writers of different UI states could clobber each other there too.Fix: UI states leave the session
Leantime\Core\UI\UiState: each state is its ownzp_settingsrow (usersetting.{userId}.ui.{name}), written with one atomicupsert(). That's portable across MySQL, Postgres and SQL Server: no read-modify-write, no blob, no lock.submenuToggleblob is read as a fallback, and newer rows win.Menu::setSubmenuState/getSubmenuStateTemplate::getToggleState(old accordion, subtask arrow, dashboard calendar)elements.accordionusersettings.submenuToggleto the session any more. That also removesgetSubmenuState's rewrite of the wholeusersettingsarray into the session on every page view.StartSessionis unchanged; the first version of this PR changed it, and that was reverted after review.The upcoming
layout.content-box(#3948) stores its sidebar widths here too.Testing
UiStateTest(SQLite, 8 tests):SubmenuStateTest: a state saved throughMenu::setSubmenuStateis read back on the next request and nothing goes to the session.AccordionTest: passes with states from UiState.setSubmenuStatecalls fired at the same moment left three rows; none was lost.🤖 Generated with Claude Code
https://claude.ai/code/session_01KbcnaZTbGANSNAWkGT6HGs