Skip to content

Conversation

@akshayutture-augment
Copy link

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@akshayutture-augment
Copy link
Author

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Confidence Score: 1/5

  • This PR has a critical bug that will cause runtime crashes when users try to unsubscribe
  • The unsubscribe controller action contains a critical nil reference bug on line 105-113 of app/controllers/topics_controller.rb. When TopicUser.find_by returns nil (which happens when a user hasn't previously interacted with a topic), calling .notification_level will raise a NoMethodError. This will break the unsubscribe functionality completely for new topic interactions.
  • app/controllers/topics_controller.rb requires immediate attention to fix the nil reference bug in the unsubscribe action

Important Files Changed

File Analysis

Filename Score Overview
app/controllers/topics_controller.rb 1/5 Added unsubscribe action with nil reference bug on TopicUser lookup
app/assets/javascripts/discourse/controllers/topic-unsubscribe.js.es6 3/5 New controller with typo in function name (stopNotificiations)
app/models/topic.rb 5/5 Added unsubscribe_url method
app/mailers/user_notifications.rb 5/5 Added unsubscribe_url to email context and minor refactoring

Sequence Diagram

sequenceDiagram
    participant User
    participant Email
    participant Browser
    participant TopicsController
    participant TopicUser
    participant TopicView
    participant MessageBus

    User->>Email: Receives notification email
    Email->>Email: Contains unsubscribe_url link
    User->>Browser: Clicks unsubscribe link
    Browser->>TopicsController: GET /t/:slug/:id/unsubscribe
    TopicsController->>TopicsController: ensure_logged_in filter
    TopicsController->>TopicView: Create TopicView
    TopicView-->>TopicsController: Return topic data
    TopicsController->>TopicUser: find_by(user_id, topic_id)
    TopicUser-->>TopicsController: Return TopicUser (or nil)
    TopicsController->>TopicUser: Update notification_level
    TopicUser->>TopicUser: save!
    TopicUser->>MessageBus: Publish notification_level_change
    TopicsController->>Browser: Render topic show page
    Browser-->>User: Display unsubscribe confirmation
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

18 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +105 to +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]
else
tu.notification_level = TopicUser.notification_levels[:muted]
end

tu.save!
Copy link

Choose a reason for hiding this comment

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

logic: find_by returns nil if no TopicUser record exists, causing NoMethodError on line 107

Suggested change
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!
tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id])
if tu
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!
else
TopicUser.create!(
user_id: current_user.id,
topic_id: params[:topic_id],
notification_level: TopicUser.notification_levels[:muted]
)
end
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/topics_controller.rb
Line: 105:113

Comment:
**logic:** `find_by` returns `nil` if no `TopicUser` record exists, causing `NoMethodError` on line 107

```suggestion
    tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id])

    if tu
      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!
    else
      TopicUser.create!(
        user_id: current_user.id,
        topic_id: params[:topic_id],
        notification_level: TopicUser.notification_levels[:muted]
      )
    end
```

How can I resolve this? If you propose a fix, please make it concise.


export default ObjectController.extend({

stopNotificiationsText: function() {
Copy link

Choose a reason for hiding this comment

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

syntax: typo in function name: 'Notificiations' should be 'Notifications'

Suggested change
stopNotificiationsText: function() {
stopNotificationsText: function() {
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/assets/javascripts/discourse/controllers/topic-unsubscribe.js.es6
Line: 5:5

Comment:
**syntax:** typo in function name: 'Notificiations' should be 'Notifications'

```suggestion
  stopNotificationsText: function() {
```

How can I resolve this? If you propose a fix, please make it concise.

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.

3 participants