Overwriting/Adding an ActiveRecord association dynamically using a singleton class

Question

The business logic is this: Users are in a Boat through a join table, I guess let's call that model a Ticket. But when a User instance wants to check who else is on the boat, there's a condition that asks if that user has permission see everyone on the Boat, or just certain people on the Boat. If a User can see everyone, the normal deal is fine: some_user.boats.first.users returns all users with a ticket for that boat. But for some users, the only people that are on the boat (as far as they're concerned) are people in, let's say the dining room. So if User's ticket is "tagged" (using an acts_as_taggable style system) with "Dining Room", the only Users returned from some_user.boats.first.users should be Users with tickets tagged "Dining Room".

Just for the record, I'm not trying to design something to be insane from the getgo - I'm trying to wedge this arbitrary grouping into a (mostly) existent system. So we've got:

class User
  has_many :tickets
  has_many :boats, :through => :tickets
end

class Ticket
  belongs_to :user
  belongs_to :boat
end

class Boat
  has_many :tickets
  has_many :users, :through => :tickets
end

Initially, I thought that I could conditionally modify the virtual class like:

singleton = class << a_user_instance ; self ; end
singleton.class_eval(<<-code
  has_many :tickets, :include => :tags, :conditions => ['tags.id in (?)', [#{tag_ids.to_s(:db)}]]
code
)

That gets all the way down to generating the SQL, but when generated, it generates SQL ending in:

LEFT OUTER JOIN "tags" ON ("tags"."id" = "taggings"."tag_id") WHERE ("tickets"._id = 1069416589 AND (tags.id in (5001,4502)))

I've tried digging around the ActiveRecord code, but I can't find anywhere that would prefix that 'id' in the SQL above with an underscore. I know that associations are loaded when an ActiveRecord class is loaded, and I'd assume the same with a singleton class. shrug.

I also used an alias_method_chain like:

singleton = class << a_user_instance ; self ; end
singleton.class_eval(<<-code
  def tickets_with_tag_filtering
    tags = Tag.find(etc, etc)
    tickets_without_tag_filtering.scoped(:include => :tags, :conditions => {:'tags.id' => tags})
  end
  alias_method_chain :tickets, :tag_filtering
code
)

But while that approach produces the desired Tickets, any joins on those tickets use the conditions in the class, not the virtual class. some_user.boats.first.users returns all users.

Any type of comment will be appreciated, especially if I'm barking up the wrong tree with this approach. Thanks!

Answer 1

Why not just grab all users on the boat and include their tags.

Then run a quick filter to include & return only the users with the same tag as the inquiring user.

Answer 2

What version of Rails are you using? Have you tried upgrading to see if the underscore issue is fixed? It's like it can't find the foreign key to put in as "tag_id" or somethin'.

My ruby-fu is limited, so I'm not sure how to dynamically include the correct method options at run-time.

Just to help you clarify, you have to worry about this two places. You want to filter a user's viewable users so they only see users with the same tags. Your structure is:

user <--> tickets <--> boats <--> tickets <--> users

... right?

So, you need to filter both sets of tickets down to the ones with the current_user's tags.

Maybe you just need a current_user.viewable_users() method and then filter everything through that? I'm not sure what existing functionality you've got to preserve.

Blech, I don't feel like I'm helping you at all. Sorry.

Answer 3

So a wild guess about your underscore issue is that Rails is generating the assocation code based on the context at the time of evaluation. Being in a singleton class could mess this up, like so:

"#{owner.table_name}.#{association.class.name}_id = #{association.id}"

You could get in there and define a class name property on your singleton class and see if that fixes the issue.

On the whole I don't recommend this. It creates behavior that is agonizing to track down and impossible to extend effectively. It creates a landmine in the codebase that will wound you or someone you love at a later time.

Instead, consider using a named_scope declaration:

class User
   has_many :taggings, :through => :tickets

   named_scope :visible_to, lambda { |looking_user|
      { :include => [ :tickets, :taggings ], 
        :conditions => [ "tickets.boat_id in (?) and taggings.ticket_id = tickets.id and taggings.tag_id in (?)", looking_user.boat_ids, looking_user.tag_ids ] 
      }
    }
end

While you may have to go back and edit some code, this is much more flexible in the ways it can be used:

Boat.last.users.visible_to( current_user )

It's clear that a restriction is being placed on the find, and what the purpose of that restriction is. Because the conditions are dynamically calculated at runtime, you can deal with the next weird modification your client hits you with. Say some of their users have xray vision and clairvoyance:

class User
   named_scope :visible_to, lambda { |looking_user|
      if looking_user.superhuman?
        {}
      else
        { :include => [ :tickets, :taggings ], 
          :conditions => [ "tickets.boat_id in (?) and taggings.ticket_id = tickets.id and taggings.tag_id in (?)", looking_user.boat_ids, looking_user.tag_ids ] 
        }
      end
    }
end

By returning an empty hash, you can effectively nullify the effect of the scope.

Answer 4

Your approach is the problem. I know it seems expedient at the moment to hack something in where you don't have to refactor the existing call sites, but I believe given time this will come back to haunt you as the source of bugs and complexity.

Sleeping dogs that lie come back to bite you hard, in my experience. Typically in the form of a future developer who doesn't know your association is "magic" and uses it assuming it's just pail ole rails. He/she likely won't even have a reason to write a test case that would expose the behavior either, which raises the odds you'll only find out about the bug when it's live in production and the client is unhappy. Is it really worth the time you're saving now?

Austinfrombostin is pointing the way. Different semantics? Different names. Rule number one is always to write code that says what it does as clearly as possible. Anything else is the path of madness.