FEATURE: per-topic unsubscribe option in emails#9
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) | ||
|
|
||
| if tu.notification_level > TopicUser.notification_levels[:regular] |
There was a problem hiding this comment.
Allow email unsubscribe without login
The new /t/:slug/:topic_id/unsubscribe endpoint only works for users who already have a session. The action looks up the TopicUser via current_user.id (and the route was added to the ensure_logged_in filter above), so following the link from an email while logged out raises Discourse::NotLoggedIn and the request is redirected to / without ever lowering the notification level. That means the per-topic unsubscribe link you just added to emails silently does nothing unless the recipient is already logged into the site, which defeats the feature and violates common unsubscribe expectations. This controller needs a way to authenticate via the link itself (e.g., a signed token) or run without forcing a login so the user can actually unsubscribe when coming from email.
Useful? React with 👍 / 👎.
Test 2
Replicated from ai-code-review-evaluation/discourse-cursor#2