Finding the object and the URL associated with a notification

Question

I have a Notification model which inherits from the ActiveRecord class. Each notification is associated with a specific type of action, which is determined by an integer field (action) whose value is between 0 and 6. A notification is also associated with a model (e.g. User or Project) whose ID is stored as the object_id field.

Inside a view, I am trying to prepare the notification text and url, which looks something like this:

# notif_string prepares the text of notification.
# For example: "username commented on"
= link_to(notif_string(notification) + notification.objectname,
            notification.url)

The following methods in the Notification class where flagged as complex by Code Climate:

# Finds the object on which action is performed.
# Action can be of following types:
# 0: Commented on Project,
# 1: Followed Project,
# 2: Forked,
# 3: Followed User,
# 4: Created Project,
# 5: Commented on Issue
# 6: Created Issue 
def objectname
  case action
  when 0
    return Project.find(Comment.find(object_id).polycomment_id.to_i).name
  when 5
    return Issue.find(Comment.find(object_id).polycomment_id.to_i)
                .friendly_text
  when 3
    return User.find(object_id).username
  when 1, 2, 4
    return Project.find(object_id).name
  when 6
    return Issue.find(object_id).friendly_text
  end
end

# Find the url to which user which be directed when they click on the
# notification item
def url
  case action
  when 0
    return Project.find(Comment.find(object_id).polycomment_id.to_i).urlbase
  when 5
    return Issue.find(Comment.find(object_id).polycomment_id.to_i).show_url
  when 1, 2, 4
    return Project.find(object_id).urlbase
  when 6
    return Issue.find(object_id).show_url
  else
    return "/#{actor.username}"
  end
end

Yes, my cases are almost repeated but I am not sure how can reduce them. Most of the blogs suggest that I use polymorphism but again I don't know how I can apply it here.

Answer 1

First, object_id is a built-in method of Object in Ruby, so I recommend renaming this field to model_id to avoid any clashes.

---

You want polymorphic classes for the views of different notification types. Since these classes will only be used by views, we'll make them all inherit from a common base class named NotificationView which will provide all the information the view needs:

class NotificationView
  attr_reader :action_description, :object_name, :url
end

Every subclass of NotificationView will initialize by setting these fields according to a given Notification model. For example:

class IssueCreatedNotificationView < NotificationView
  def initialize(notification)
    issue = Issue.find(notification.model_id)
    @action_description = "created issue"
    @object_name = issue.friendly_text
    @url = issue.show_url
  end
end

(I would put these classes in /app/models/notification_views/)

Now you need a method that constructs the appropriate NotificationView given a Notification. Since the type of a notification is determined by an integer field, you will need a case statement. But you'll only need a single case statement in your code because you can take advantage of polymorphism once you have a NotificationView instance.

I would put this method in the Notification class:

def view
  case action
  when 2
    ProjectForkedNotificationView.new(self)
  when 6
    IssueCreatedNotificationView.new(self)
  # etc...
  end
end

Then in your controller, you can get a NotificationView by calling Notification#view. For example:

@view = Notification.find(params[:id]).view

And you can use this object in your view:

= link_to(@view.action_description + @view.object_name, @view.url)

---

You may want to add some utility methods to the NotificationView base class, if you need them in your views, for example:

def message
  "#{action_description} #{object_name}"
end

Answer 2

This doesn't directly answer your question, but I also recommend switching from integer actions to enums: http://edgeapi.rubyonrails.org/classes/ActiveRecord/Enum.html

Interestingly, this should not require any changes to your database at all, but will provide a lot of convenience methods and make your intent more clear. I do recommend hard coding a hash of the integer to the symbol in your enum definition to avoid any ambiguity: http://www.justinweiss.com/articles/creating-easy-readable-attributes-with-activerecord-enums/