Repository navigation
FEATURE: per-topic unsubscribe option in emails - #1768
kaihao-zhao wants to merge 2 commits into
Conversation
![unblocked-local-kaihao[bot]](https://avatars.githubusercontent.com/u/304571795?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found.
About Unblocked
Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.
📖 Documentation — Learn more in our docs.
💬 Ask questions — Mention @unblocked-local-kaihao to request a review or summary, or ask follow-up questions.
👍 Give feedback — React to comments with 👍 or 👎 to help us improve.
⚙️ Customize — Adjust settings in your preferences.
| get "t/:slug/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/} | ||
| get "t/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/} |
There was a problem hiding this comment.
These GET routes immediately modify the logged-in user's notification level. Rails' CSRF protection does not validate GET requests, and unsubscribe also bypasses check_xhr, so an attacker can force an authenticated browser to unsubscribe from a topic through a crafted link or navigation. Keep GET read-only and apply the change through a CSRF-protected POST confirmation.
| if tu.notification_level > TopicUser.notification_levels[:regular] | ||
| tu.notification_level = TopicUser.notification_levels[:regular] |
There was a problem hiding this comment.
For a mailing-list-mode user whose topic is tracking or watching, this sets the level to regular but does not stop the emails. Jobs::NotifyMailingListSubscribers excludes only muted topics, so every subsequent post is still emailed despite the unsubscribe confirmation. Select muted for mailing-list-mode users regardless of their previous notification level.
| tu.notification_level = TopicUser.notification_levels[:muted] | ||
| end | ||
|
|
||
| tu.save! |
There was a problem hiding this comment.
Saving directly leaves notifications_reason_id unchanged. For an emailed topic with no notification reason, TopicUser.update_last_read will overwrite the new regular or muted level with tracking once the reading threshold is met. In mailing-list mode, this can re-enable emails for the muted topic. Use TopicUser.change, as set_notifications does, so the explicit choice is preserved.
| tu.save! | |
| TopicUser.change(current_user.id, params[:topic_id], notification_level: tu.notification_level) |
| export default Discourse.Route.extend({ | ||
| model(params) { | ||
| const topic = this.store.createRecord("topic", { id: params.id }); | ||
| return PostStream.loadTopicView(params.id).then(json => { |
There was a problem hiding this comment.
This only loads the normal topic view: PostStream.loadTopicView requests /t/:id.json, not the unsubscribe endpoint. When an unsubscribe link is followed inside Discourse, the click interceptor performs an Ember transition without calling TopicsController#unsubscribe. The page therefore claims notifications have stopped while leaving the subscription unchanged. Ensure client-side entry applies the unsubscribe operation before showing confirmation, using a CSRF-protected mutation.
2 participants
See title.