vendredi 27 mai 2016

Referencing several models from one model class- Bad practice?

I want to obtain all notifications for current user with this code:

current_user.follows.each do |follow|
    follow.followable.open_notifications.each do |notification|
        if !notification.reads.include?(current_user)
            @open_notifications += notification 
        end
    end
end

So far I had this code in my controller however I know this kind of logic should be place in model class. After moving code:

OpenNotification Controller:

OpenNotification.fetch_unread(current_user) 

OpenNotification Model:

def self.fetch_unread(user)
    @open_notifications = []
    user.follows.each do |follow|
        follow.followable.open_notifications.each do |notification|
            if !notification.reads.include?(user)
                @open_notifications += notification 
            end
        end
    end
    @open_notifications
end

EDIT:

Classes involved:

  • User
  • Follow - who(user) follows what(followable)
  • Followable(polymorphic - may be User, Event or a Place)
  • OpenNotification - stores information about the change of followable object
  • Read - who read which notification (user_id and open_notification_id)

User:

has_many :follows, class_name: 'Follow',
                 source: :user

has_many :follows_as_fallowable, 
                class_name: 'Follow',
                as: :followable

has_many :followers, through: :follows_as_fallowable,
                   source: :user

Event:

has_many :follows, as: :followable
has_many :followers, through: :follows,
                         source: :user

has_many :open_notifications, as: :followable  

OpenNotification:

belongs_to :followable, polymorphic: true
has_many :reads

Reads:

belongs_to :user
belongs_to :open_notification


My question is if it is a good practice to refer to multiple classes from the class responsible for one particular resource?
If this is not a good practice, how the code should be refactored?

Aucun commentaire:

Enregistrer un commentaire