FEATURE: per-topic unsubscribe option in emails

[hussam789]

User description

PR #2

---

PR Type

Enhancement

---

Description

• Add per-topic unsubscribe functionality via email links

• Implement topic unsubscribe route and controller action

• Update email templates with topic-specific unsubscribe URL

• Refactor code formatting and improve code quality

---

Diagram Walkthrough

flowchart LR
  A["Email Notification"] -->|"unsubscribe_url"| B["Topic Unsubscribe Route"]
  B -->|"GET /t/:id/unsubscribe"| C["TopicsController#unsubscribe"]
  C -->|"Toggle notification level"| D["TopicUser Model"]
  D -->|"Save changes"| E["User Preferences Updated"]
  A -->|"Render template"| F["Email Template with Link"]

Loading

File Walkthrough

	Relevant files
Enhancement	11 files topics_controller.rb Add unsubscribe action to topics controller                            +24/-2    topic.rb Add unsubscribe_url method to topic model                                +4/-0      user_notifications.rb Add topic unsubscribe URL to email options                              +2/-3      message_builder.rb Support topic-specific unsubscribe links in emails              +12/-11  topic-unsubscribe.js.es6 Create topic unsubscribe controller component                        +9/-0      topic-unsubscribe.js.es6 Create topic unsubscribe route handler                                      +23/-0    topic-unsubscribe.js.es6 Create topic unsubscribe view component                                    +3/-0      dropdown-button.js.es6 Add conditional title rendering to dropdown                            +4/-1      topic.scss Add styles for topic unsubscribe page                                        +12/-0    unsubscribe.hbs Create topic unsubscribe template                                                +8/-0      notification.html.erb Add unsubscribe link to email template                                      +19/-21
Formatting	2 files topic_user.rb Code formatting and style improvements                                      +8/-13    topic-from-params.js.es6 Refactor route with modern ES6 syntax                                        +13/-12
Configuration changes	2 files routes.rb Add unsubscribe routes for topics                                                +5/-3      app-route-map.js.es6 Register topic unsubscribe route mapping                                  +1/-0
Documentation	2 files client.en.yml Add client-side unsubscribe localization strings                  +3/-1      server.en.yml Add server-side unsubscribe localization strings                  +4/-1
Tests	1 files message_builder_spec.rb Update tests for unsubscribe URL parameter                              +2/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Unauthenticated state change Description: The unsubscribe action relies on current_user and toggles notification levels without validating a signed token in the request, enabling anyone with the link to change a user's topic notification settings (CSRF/unauthenticated state change via GET). topics_controller.rb [98-116] Referred Code def unsubscribe @topic_view = TopicView.new(params[:topic_id], current_user) if slugs_do_not_match || (!request.format.json? && params[:slug].blank?) return redirect_to @topic_view.topic.unsubscribe_url, status: 301 end tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) if tu.notification_level > TopicUser.notification_levels[:regular] tu.notification_level = TopicUser.notification_levels[:regular] else tu.notification_level = TopicUser.notification_levels[:muted] end tu.save! perform_show_response end
⚪	CSRF via GET route Description: Unsubscribe endpoints are exposed as GET routes without any authenticity protection or signed parameters, which facilitates CSRF or link-forgery to alter user notification settings. routes.rb [438-442] Referred Code get "t/:slug/:topic_id/moderator-liked" => "topics#moderator_liked", constraints: {topic_id: /\d+/} get "t/:slug/:topic_id/summary" => "topics#show", defaults: {summary: true}, constraints: {topic_id: /\d+/} get "t/:slug/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/} get "t/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/} get "t/:topic_id/summary" => "topics#show", constraints: {topic_id: /\d+/}
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Misspelled identifier: The function name stopNotificiationsText contains a typo which reduces readability and can cause confusion. Referred Code stopNotificiationsText: function() { return I18n.t("topic.unsubscribe.stop_notifications", { title: this.get("model.fancyTitle") }); }.property("model.fancyTitle"),
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit log: The new per-topic unsubscribe action changes a user's topic notification level but does not log the action with user ID, timestamp, or outcome for auditability. Referred Code def unsubscribe @topic_view = TopicView.new(params[:topic_id], current_user) if slugs_do_not_match || (!request.format.json? && params[:slug].blank?) return redirect_to @topic_view.topic.unsubscribe_url, status: 301 end tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) if tu.notification_level > TopicUser.notification_levels[:regular] tu.notification_level = TopicUser.notification_levels[:regular] else tu.notification_level = TopicUser.notification_levels[:muted] end tu.save! perform_show_response end
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Nil handling risk: The unsubscribe action assumes a TopicUser record exists and that tu.notification_level is present, with no handling for nil cases or save failures beyond raising. Referred Code tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) if tu.notification_level > TopicUser.notification_levels[:regular] tu.notification_level = TopicUser.notification_levels[:regular] else tu.notification_level = TopicUser.notification_levels[:muted] end tu.save! perform_show_response
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Authn/authz check: New unsubscribe routes expose a GET endpoint that changes user state, and the controller action does not visibly enforce authentication/authorization or CSRF protections in the diff. Referred Code get "t/:slug/:topic_id/moderator-liked" => "topics#moderator_liked", constraints: {topic_id: /\d+/} get "t/:slug/:topic_id/summary" => "topics#show", defaults: {summary: true}, constraints: {topic_id: /\d+/} get "t/:slug/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/} get "t/:topic_id/unsubscribe" => "topics#unsubscribe", constraints: {topic_id: /\d+/}

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Implement token-based authentication for unsubscribing The current unsubscribe feature requires users to be logged in, creating a poor user experience. It should be changed to use a secure, single-use token in the unsubscribe URL to allow unsubscribing without a login. Examples: app/controllers/topics_controller.rb [98-116] def unsubscribe @topic_view = TopicView.new(params[:topic_id], current_user) if slugs_do_not_match || (!request.format.json? && params[:slug].blank?) return redirect_to @topic_view.topic.unsubscribe_url, status: 301 end tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) if tu.notification_level > TopicUser.notification_levels[:regular] ... (clipped 9 lines) app/models/topic.rb [719-721] def unsubscribe_url "#{url}/unsubscribe" end Solution Walkthrough: Before: # app/controllers/topics_controller.rb def unsubscribe # Relies on a logged-in user via `current_user` @topic_view = TopicView.new(params[:topic_id], current_user) tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) if tu.notification_level > TopicUser.notification_levels[:regular] tu.notification_level = TopicUser.notification_levels[:regular] else tu.notification_level = TopicUser.notification_levels[:muted] end tu.save! perform_show_response end After: # app/controllers/topics_controller.rb def unsubscribe # Finds user via a secure token from params, no login required unsubscribe_key = UnsubscribeKey.find_by(key: params[:key]) # Validate key and topic... user = unsubscribe_key.user tu = TopicUser.find_by(user_id: user.id, topic_id: params[:topic_id]) # ... logic to change notification level ... tu.save! unsubscribe_key.destroy # Invalidate token after use # Render a simple confirmation page render "unsubscribe_confirmation" end Suggestion importance[1-10]: 10 __ Why: This suggestion addresses a critical design flaw in the new feature, as requiring a login to unsubscribe from an email link creates significant user friction and renders the feature impractical for many users.	High
Possible issue	Handle nil when user record missing To prevent a NoMethodError, handle cases where the TopicUser record is nil by using TopicUser.change, which gracefully creates or updates the record. app/controllers/topics_controller.rb [105-113] tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) -if tu.notification_level > TopicUser.notification_levels[:regular] - tu.notification_level = TopicUser.notification_levels[:regular] +new_level = if tu && tu.notification_level > TopicUser.notification_levels[:regular] + TopicUser.notification_levels[:regular] else - tu.notification_level = TopicUser.notification_levels[:muted] + TopicUser.notification_levels[:muted] end -tu.save! +TopicUser.change(current_user.id, params[:topic_id], notification_level: new_level) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a NoMethodError on a nil object, which would cause a server error for users without a pre-existing TopicUser record, and provides an idiomatic fix.	Medium
More