Rails - Sorting a model using order

Question

I have a Question model, which I am getting a selection of items from (@rejected). I then want to calculate an :approval_rating attribute for each item and sort the collection by that attribute. I'm trying to do so with the following:

def index
  ...
  @rejected = @questions.where("ques_num=? AND rejected=?", params[:ques_num], true) 
    if @rejected   
      @rejected = @rejected.map do |q|
        upvotes = q.get_upvotes.size
        downvotes = q.get_downvotes.size
        total_votes = Float(upvotes + downvotes)
        q.approval_rating = (upvotes/total_votes*100).round(2)
        q.save
      end
      @rejected = @rejected.order('approval_rating DESC')
    end
  end

This is giving me the error when attempting to sort: undefined method `order' for [true, true, true]:Array

I also tried this with

@rejected = @rejected.sort_by(&:approval_ratings)

and got the error: undefined method `approval_rating' for true:TrueClass

What am I doing incorrectly here?

Answer 1

Your code creates an Array of the value true (see below)

def index
  ...
  @rejected = @questions.where("ques_num=? AND rejected=?", params[:ques_num], true) 
  if @rejected   
    @rejected = @rejected.map do |q|
      upvotes = q.get_upvotes.size
      downvotes = q.get_downvotes.size
      total_votes = Float(upvotes + downvotes)
      q.approval_rating = (upvotes/total_votes*100).round(2)
      q.save # <= this is what gets added to the array from map, which is 'true'
    end
    @rejected = @rejected.order('approval_rating DESC')
  end
end

And true:TrueClass doesn't respond to approval_rating.

There are some other problems with your code. This:

@questions.where("ques_num=? AND rejected=?", params[:ques_num], true) 

will always return an ActiveRecord::Relation, so this:

if @rejected

will always return true.

You probably want to consider using each instead of map. But, that will still return an Array, I believe. So you still won't be able to do @rejected.order('approval_rating DESC')

I think you should be able to do:

def index
  ...
  @rejected = @questions.
                where("ques_num=? AND rejected=?", params[:ques_num], true).
                each do |q|
                  upvotes     = q.get_upvotes.size
                  downvotes   = q.get_downvotes.size
                  total_votes = Float(upvotes + downvotes)
                  q.approval_rating.update(
                    approval_rating: (upvotes/total_votes*100).round(2)
                  )
                end.
                sort_by(&:approval_rating)
end

I'm not 100% sure your sort order will be in the right direction, so you may need to check that.

Personally, it seems to me that you would be better off to put the approval_rating logic in the Question model. Perhaps something like:

class Question < ApplicationRecord

  def update_approval_rating
    upvotes     = get_upvotes.size
    downvotes   = get_downvote.size
    total_votes = Float(upvotes+downvotes)
    update(
      approval_rating: (upvotes/total_votes*100).round(2)
    )
  end

end

In which case you could do

def index
  ...
  @rejected = @questions.
                where("ques_num=? AND rejected=?", params[:ques_num], true).
                each{|q| q.update_approval_rating}.
                sort_by(&:approval_rating)
end

It seems to me like you might consider setting approval_rating every time a Question gets a vote (in your QuestionsController presumably). Then you don't have to deal with setting approval_rating in the index action. Besides, index should be idempotent, so modifying records there seems smelly (IMO).

Then, you could end up with something more like:

def index
  ...
  @rejected = @questions.
                where("ques_num=? AND rejected=?", params[:ques_num], true).
                order(approval_rating: :desc)
end

Ah... So much better.

You could even add some class methods on to your Question model, something like:

class Question < ApplicationRecord
  class << self 

    def for_question_number(ques_num)
      where(ques_num: ques_num)
    end 

    def rejected
      where(rejected: true)
    end

    def best_first
      order(approval_rating: :desc)
    end

  end
end

In which case you could do:

def index
  ...
  @rejected = @questions.
                for_question_number(params[:ques_num]).
                rejected.
                best_first
end

If the future you decides to determine what is "best" some other way at some point, then you can make the changes to the best_first method in your Question model and your index action is non-the-wiser.

And now it's starting to feel like rainbows and lollipops in here.

Answer 2

You're using map which returns an array. If you want to apply AR query methods such order, you need to supply ActiveRecord::Relation class. The simplest solution here - perform another query to sort the relation based on the modified field or sort @rejected with ruby using sort_by.

def index
  ...
  @rejected = @questions.where("ques_num=? AND rejected=?", params[:ques_num], true) 
  if @rejected.present?   
     @rejected.each do |q|
        upvotes = q.get_upvotes.size
        downvotes = q.get_downvotes.size
        total_votes = Float(upvotes + downvotes)
        q.approval_rating = (upvotes/total_votes*100).round(2)
        q.save
      end
      @rejected = @questions.where("ques_num=? AND rejected=?", params[:ques_num], true).order('approval_rating DESC')
    end
  end
end

Note: Your @rejected always == true (Empty relation [] treats as true). Use @rejected.present?

Perhaps it would be better to write the code from iterator to the SQL/AR statement