Rails method to match users according to language fluency level

Question

This code works fine, but I'm still learning, and feel like there is a more Railsy way to write this. Is there a good rails ActiveRecord method I should learn more about? Please let me know if you know a shorter, more direct way to run this same code.

Scenario: A user can have many languages, and level is an attribute of languages. I want to iterate through languages_users and find all the languages of a user, and then find where another user has 2 of the same languages, but at opposite levels.

user.rb

class User < ApplicationRecord
 has_many :languages_users
 has_many :languages, :through => :languages_users
end

controller

#Fluent languages and nonfluent languages can change depending on this situation, but here is good example:
@fluent_languages = LanguagesUser.where(user: current_user.id).where("level > 4")
@nonfluent_language = LanguagesUser.where(user: current_user.id).where("level < 5")
email_matches(@fluent_languages, @nonfluent_languages)
def email_matches(fluent_languages, nonfluent_languages)
 @fluent_languages = fluent_languages
 @nonfluent_languages = nonfluent_languages
 #array of user's fluent language_ids
 @fluent_langs = []
 @fluent_langs << @fluent_languages
 #array of user's nonfluent language_ids
 @nonfluent_langs = []
 @nonfluent_languages.each do |lang|
 @nonfluent_langs << lang.language_id
 end
 @fluent_les = LanguagesUser.select(:user_id).where(language_id: @fluent_langs, level: 1..4)
 @fluent_users_ids = []
 @fluent_les.each do |le|
 @fluent_users_ids << le.user_id
 end
 @nonfluent_les = LanguagesUser.select(:user_id).where(language_id: @nonfluent_langs, level: 5)
 @nonfluent_user_ids = []
 @nonfluent_les.each do |le|
 @nonfluent_user_ids << le.user_id
 end
 @matches = @fluent_users_ids & @nonfluent_user_ids
 @new_matches = User.where(id: @matches, status: "Active").order('last_seen DESC').limit(10)
 @new_matches.each do |user|
 UserMailer.new_match(user, current_user).deliver_later
 end
end

Let me know if I can provide anything else.

• 200_success: Welcome to Code Review! I have edited your post, especially the title, to better conform to the How to Ask guidelines.

Answer 1

This is a somewhat shorter more concise version of your code. Taking advantages of some or Rails's magic and conventions.

def email_matches(current_user)
 fluent_ids = current_user.languages.where("level > 4").pluck(:language_id)
 nonfluent_ids = current_user.languages.where("level < 5").pluck(:language_id)
 fluent_user_ids = LanguagesUser.where(language_id: fluent_ids, level: 1..4)
 .pluck(:user_id)
 nonfluent_user_ids = LanguagesUser.where(language_id: nonfluent_ids, level: 5)
 .pluck(:user_id)
 matches = fluent_user_ids & nonfluent_user_ids
 new_matches = User.where(id: matches, status: "Active").order(last_seen: :desc).limit(10)
 new_matches.each do |user|
 UserMailer.new_match(user, current_user).deliver_later
 end
end

This takes advantage of the has_many :languages in your User model. Also uses the pluck method which returns an array of language_ids

current_user.languages.where.where("level > 4").pluck(:language_id)

There is however a way to do this all in maybe 1-2 lines of SQL, maybe someone else will provide that magic here.

Answer 2

I think you should be closer to MVC here. All this code should be in model, so from controller you just need to command user email matches like @user.email_matches. Also I would use standard Rails tools for creating conditioned relations between models.

 class User < ApplicationRecord
 has_many :languages_users
 has_many :languages, :through => :languages_users
 has_many :fluent_languages_users, -> { :fluent }, class_name: 'LanguagesUser'
 has_many :fluent_languages, through: :fluent_languages_users, source: :language
 has_many :non_fluent_languages_users, -> { :non_fluent }, class_name: 'LanguagesUser'
 has_many :non_fluent_languages, through: :non_fluent_languages_users, source: :language
 def email_matches
 fluent_users = non_fluent_languages.map {|language| language.fluent_users}.flatten
 non_fluent_users = fluent_languages.map {|language| language.non_fluent_users}.flatten
 matching_user_ids = (fluent_users & non_fluent_users).map(&:id)
 User.where(id: matching_user_ids, status: 'Active').order(last_seen: :desc).limit(10).each do |user|
 UserMailer.new_match(user, self).deliver_later
 end
 end
 end
 class LanguagesUser < ApplicationRecord
 belongs_to :user
 belongs_to :language
 FLUENT_LEVELS = [5]
 NON_FLUENT_LEVELS = [1,2,3,4]
 scope :fluent, -> { where(level: FLUENT_LEVELS) }
 scope :non_fluent, -> { where(level: NON_FLUENT_LEVELS) }
 end
 class Language < ApplicationRecord
 has_many :languages_users
 has_many :users, :through => :languages_users
 has_many :fluent_languages_users, -> { :fluent }, class_name: 'LanguagesUser'
 has_many :fluent_users, through: :fluent_languages_users, source: :user
 has_many :non_fluent_languages_users, -> { :non_fluent }, class_name: 'LanguagesUser'
 has_many :non_fluent_users, through: :non_fluent_languages_users, source: :user
 end
 @user = User.find(1)
 @user.email_matches

Or if it would be too consuming for SQL querying you can remove those conditional and through relations and just do like this:

class User < ApplicationRecord
 has_many :languages_users
 has_many :languages, :through => :languages_users
 scope :last_seen_active, -> { where(status: 'Active').order(last_seen: :desc)) }
 def email_matches
 fluent_users = users_with_oposite_languages_level(:non_fluent)
 non_fluent_users = users_with_oposite_languages_level(:fluent)
 fluent_users.merge(non_fluent_users).last_seen_active.limit(10).each do |user|
 UserMailer.new_match(user, self).deliver_later
 end
 end
 private
 def users_with_oposite_languages_level(level_type)
 User.where(
 id: languages_users.send(level_type).map(&:language).compact.uniq do |language|
 language.languages_users.send(inverted_level_type(level_type)).map(&:user_id)
 end.flatten.compact.uniq
 )
 end
 def inverted_level_type(level_type)
 :fluent ? :non_fluent : :fluent
 end
end