Skip to content

feat(core): reset notification preferences to their defaults - #5082

Merged
imorland merged 2 commits into
flarum:2.xfrom
ernestdefoe:feat/reset-notification-preferences
Oct 6, 2026
Merged

imorland merged 2 commits into
flarum:2.xfrom
ernestdefoe:feat/reset-notification-preferences

Conversation

@ernestdefoe

@ernestdefoe ernestdefoe commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Fixes: the dead end described in https://discuss.flarum.org/d/39466 (no issue number).

Changes proposed in this pull request:
Clicking a column header in Settings › Notifications switches the whole column off in one click, with no way back short of re-ticking every box. This adds a Reset to defaults button under the notification grid. It calls a new DELETE /api/users/{id}/notification-preferences endpoint, which removes every notification preference from the user's stored preferences in one save, so the registered defaults apply again.

  • UserResource: the notificationPreferences.reset endpoint, next to the avatar endpoints. Only the user themselves can call it, the same rule as the preferences field. The keys are built the way they are registered (every type in Notification::getSubjectModels() × every driver in NotificationSyncer::getNotificationDrivers(), via User::getNotificationPreferenceKey()). It edits the stored value rather than the accessor's merged one, so the defaults of other preferences are never written back as stored values.
  • SettingsPage: the button sends the DELETE and pushes the response into the store, as AvatarEditor does when removing an avatar.
  • Locale: core.forum.settings.reset_notifications_button.
  • User::setPreference() is unchanged (an earlier version of this PR changed how it handles null; that's gone).

Reviewers should focus on:
Whether editing the stored preferences directly (getAttributes()['preferences']) is how you'd like this done, rather than going through setPreference().

Screenshot
The button sits under the notification grid in Settings.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Frontend changes: tests are green (run yarn test in js/).
  • Frontend changes: tests have been added, or are not appropriate here.
  • Backend changes: tests are green (run composer test). Ran tests/integration/api/users on SQLite: the new tests pass and the existing ones are unchanged; the three that fail locally also fail without this PR.
  • Backend changes: tests have been added, or are not appropriate here.
  • Where applicable, changes are suitable for all supported database drivers (MySQL, MariaDB, PostgreSQL, SQLite). Eloquent only, no raw SQL.
  • Core developer confirmed locally this works as intended.
  • The description above is written by me and describes what this pull request actually does.

Required changes:

js/dist is not included, following the repository's convention of building it in CI.

🤖 Generated with Claude Code

Turning a whole column off in Settings > Notifications takes one click and
there was no way back short of re-ticking every box, or rewriting
preferences from the browser console.

A "Reset to defaults" button under the grid sends null for every notify_*
preference. User::setPreference() now treats null as "put this preference
back to its registered default"; the defaults live on the server, so the
client cannot restore them itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ernestdefoe
ernestdefoe requested a review from a team as a code owner October 3, 2026 23:46

@imorland imorland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for picking this up, @ernestdefoe! The dead end from the discuss thread is real: one click on a column header switches a whole column off, with no easy way back. A reset button is a nice answer to it.

I ran this on a dev forum against a real member account, and it does what it says. After a reset, every notification preference reads as its default again, and the member's other preferences are untouched. Ordinary saves behave exactly as before.

I'd like to suggest one change of approach before it goes in: doing the reset on the server, with its own endpoint.

As you point out in the description, the client never sees the defaults, which is why setPreference() needed a new rule for null. If the server does the reset itself, that rule isn't needed, and setPreference() can stay as it is. The null handling applies to every preference sent through the API, so I'd rather keep that out of this PR. It also takes care of the note in your description: the keys can be removed in one save, rather than each call writing the others back with their defaults. On my forum, 59 of the 60 ended up stored that way.

Something along the lines of the avatar endpoints already in UserResource:

Endpoint\Endpoint::make('notificationPreferences.reset')
    ->route('DELETE', '/{id}/notification-preferences')
    ->authenticated()
    ->action(function (Context $context) {
        // Only the user themselves, the same rule as the `preferences` field.
        // Drop every notification preference in one save, then return the user.
    }),
  • Which keys: build the list the same way the drivers register them: every type in Notification::getSubjectModels() × every driver in NotificationSyncer::getNotificationDrivers(), via User::getNotificationPreferenceKey(). On my forum this gives exactly the same keys as the notify_ prefix, but it comes from the same place as registration rather than from a naming convention.
  • Frontend: app.request({ method: 'DELETE', url: … }), then app.store.pushPayload(response), as AvatarEditor does when removing an avatar.
  • Tests:
    • every notification preference reads as its default afterwards;
    • other preferences are untouched;
    • another member gets a 403, and a guest a 401.

A couple of small things:

  • Please switch the description to the PR template.
  • The title should be feat(core): …, to match the repo's convention.

Thanks again! This fixes a real annoyance, and your description made it easy to test. Happy to help if anything here is unclear.

Following review: the reset now has its own endpoint instead of sending
null through the preferences field.

- `DELETE /api/users/{id}/notification-preferences`, alongside the avatar
  endpoints. Only the user themselves, the same rule as the `preferences`
  field; another member gets a 403 and a guest a 401.
- The keys are built the way they are registered: every type in
  `Notification::getSubjectModels()` x every driver in
  `NotificationSyncer::getNotificationDrivers()`, through
  `User::getNotificationPreferenceKey()`.
- They are removed from the stored preferences in one save. The stored
  value is edited, not the accessor's, so the defaults of other preferences
  are never written back as stored values.
- `User::setPreference()` is back to how it was; the `null` rule is gone.
- The Settings button calls the endpoint and pushes the response into the
  store, as AvatarEditor does when removing an avatar.

Tests: notification preferences read as their defaults afterwards, other
preferences are untouched and nothing else is stored, another member gets a
403, a guest a 401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ernestdefoe ernestdefoe changed the title feat: reset notification preferences to their defaults feat(core): reset notification preferences to their defaults Oct 6, 2026

@imorland imorland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @ernestdefoe, this is exactly what I had in mind.

I ran it on a dev forum. The button puts every notification preference back to its default without a reload, and the grid updates straight away. Nothing else in the member's stored preferences changes, and another member or a guest is turned away. The full core test suite passes with it too.

On your question about getAttributes()['preferences']: going around setPreference() is right, since that would write the merged map back. If you fancy a small follow-up, the JSON handling could live on the User model (something like forgetPreferences(array $keys)), so the resource doesn't need to know how preferences are stored. Not a blocker.

@imorland imorland added this to the 2.0-pre milestone Oct 6, 2026
@imorland
imorland merged commit 4087ab4 into flarum:2.x Oct 6, 2026
24 checks passed
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