Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have an Account, User and Group model. All User and Group records belong to an Account, so it is possible for both users and groups to be related to various accounts.

Basically, I have some really hacky, pieced together SQL queries that lookup all Account records related to a given User or Group record through various associations.

Even though it's fast and only requires a single query for everything to work, I really hate the way it's written.

I was wondering if there is any way I could do this more programatically with something like arel, or if there was a better approach. I also have some security concerns about the code.

class Account < ActiveRecord::Base

  belongs_to :accountable, touch: true, polymorphic: true

  CLAUSE_FOR_RELATED = '"accounts"."accountable_type" = \'%s\' AND "accounts"."accountable_id" IN (%s)'.freeze

  def self.related_to_user(user)
    groups = user.groups.select('"groups".id')
    friends = user.following_friendships.select('"friendships".following_id')

    queries = []
    queries.push ['Group', groups.to_sql]
    queries.push ['User', friends.to_sql]
    queries.push ['User', user.id]

    self.related_to(queries)
  end

  def self.related_to_group(group)
    queries = [
      ['User', group.members.select('"users".id').to_sql]
    ]

    self.related_to(queries)
  end

  def self.related_to(queries)
    clauses = queries.map do |clause|
      sprintf(CLAUSE_FOR_RELATED, clause[0], clause[1])
    end

    self.where(clauses.join(" OR "))
  end
end

Any help or input at all would be greatly appreciated because the existence of this piece of code has been bugging me for a while.

share|improve this question
    
Is the SQL tag properly used here? Was looking for SQL to review in your code but found only tiny snippets. No bad karma just seems a bit misleading. –  Phrancis May 21 at 4:31
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.