Skip to content

Fix settings form state ownership - #1669

Open
skyfallwastaken wants to merge 2 commits into
mainfrom
fix/settings-form-state-20260901-022516
Open

Fix settings form state ownership#1669
skyfallwastaken wants to merge 2 commits into
mainfrom
fix/settings-form-state-20260901-022516

Conversation

@skyfallwastaken

Copy link
Copy Markdown
Member

Summary of the problem

Settings controls mutated nested Inertia props directly, leaving editable state without a clear local owner or an explicit resynchronisation path when Rails returned fresh props.

Describe your changes

Editors, Notifications, Privacy and Slack & GitHub now bind through minimal local scalar state that resynchronises from the corresponding user prop. Editors continues to derive the goals control state from the editable display style. Browser coverage verifies dependent state and restoration after a rejected update.

Appearance keeps its overrideable derived theme value because it already follows prop updates without mutating the user prop.

Screenshots / Media

No visual changes.

Copilot AI lite review requested due to automatic review settings September 1, 2026 02:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR moves editable settings values from direct mutation of Inertia props into locally owned Svelte state that resynchronizes when fresh server props arrive.

  • Adds local scalar state for editor, notification, privacy, and Slack preferences.
  • Continues deriving goal-control availability from the editable editor display style.
  • Adds browser coverage for dependent editor controls and restoration after a rejected update.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
app/javascript/pages/Users/Settings/Editors.svelte Replaces direct prop mutation with synchronized local state and derives goal availability from the editable display-style value.
app/javascript/pages/Users/Settings/Notifications.svelte Moves the weekly-summary preference into locally owned state synchronized from fresh user props.
app/javascript/pages/Users/Settings/Privacy.svelte Moves the public-stats preference into locally owned state synchronized from fresh user props.
app/javascript/pages/Users/Settings/SlackGithub.svelte Moves the Slack-status preference into locally owned state synchronized from fresh user props.
test/system/settings/editors_settings_test.rb Verifies dependent checkbox disabling and restoration of server values following a rejected settings update.

Reviews (2): Last reviewed commit: "Preserve settings state during SSR" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants