Kyle L. Kyle L. - 7 months ago 21
Ruby Question

How to filter an ActiveRecord query result by comparison to another model instance?

I have a simple ActiveRecord query along the lines of this:

similar_changes = Notification.where(change_owner: 'foo1', change_target: 'foo2', change_cancelled: false)


Each notification object has a field
change_type
and I have another function that checks one Notification's
change_type
with one other Notification for inverse changes (changes that undo each other in the context of my application).

I need to take this Notification's
change_type
and compare it against all others in the array. I have to reference the objects like so:
similar_changes[0]['change_type']
where the first index is each ActiveRecord in the array and the second is the dictionary that specifies which property in the Notification object.

I have a feeling I could do this manually with two nested loops and if statements, but I also know Ruby and I feel like this is something that it should have built in.

Am I wrong, or is there a better way to do this?

Here is the code (note all this code isn't quite finished so bear with me if it's not perfect):

def self.group_similar_changes(owner, target, change_type)
# long query where it selects all rows where change_owner and change_target
# are the same as original
# also where cancelled is false
# determine if cancelled (yaml)
# if cancelled (do nothing)
similar_changes = Notification.where(
change_owner: owner,
change_target: target,
change_cancelled: false
)
similar_changes.each do |change|
cancel_inverse_change(change, change.change_type)
if change.cancelled?
similar_changes.delete(change)
end
end
end
end

def cancel_inverse_change(change, change_type)
if change.inverse?(change_type)
change.cancel
end
end

def inverse?(possible_inverse_change)
is_inverse = false
change_types = YAML.load_file(File.join(NotificationManager::Engine.root, 'config/change_types.yaml'))
if self.change_type == change_types[possible_inverse_change]['inverse']
is_inverse = true
end
return is_inverse
end

Answer

Yes, your loop over similar_changes can be improved.

  • It's confusing to modify the array you're looping over. I don't even know if it's reliable, because I never do it!
  • It's also not idiomatic Ruby to rely on the return value of each. each is normally used to do something to the elements of an Enumerable that already exists, so using its return value seems strange.

I'd write it as

similar_changes.reject do |change|
  cancel_inverse_change(change, change.change)
  change.cancelled?
end