1

I wondering what would be the best practice to perform next task.

I have a search results to display from index action. Every individual record displays in the pop up through show action.

What I would love to do is to execute pop up if there is only one record found.

Here what I already tried.

def index
 @companies = Company.search(params[:query]).results
 @count = @companies.total
 if @count == 1
   return
   render company_path       
 end

end

Seems like return, redirect_to or render aren't play well in one action.

Any other thought of doing it?

UPDATE added show action

def show
 sleep 1/2
 client = Elasticsearch::Client.new host:'127.0.0.1:9200', log: true
 response = client.search index: 'companies', body: {query: { match: {_id: params[:id]} } }
 @company = response['hits']['hits'][0]['_source']
   respond_to do |format|
     format.html # show.html.erb
     format.js # show.js.erb
     format.json { render json: @company }
   end
  # more code 
end
Misha
  • 1,876
  • 2
  • 17
  • 24
  • 1
    Why do you have a `return` in the conditional statement? `render company_path` will never be reached. – Sculper Aug 10 '15 at 17:47
  • will this help `return redirect_to @companies.first if @count == 1` – Athar Aug 10 '15 at 17:51
  • @Sculper I have a **return** since I have to display results first then execute pop up – Misha Aug 10 '15 at 18:14
  • Thanks @Athar. That would probably be good solution, but in my my show action I am executing second query. I will add my show action into question. – Misha Aug 10 '15 at 18:19
  • okay if you render the view of show page for company the url will remain the `/companies` which does not looks nice, but if you redirect to show action there is an overhead of extra search query. you can decide which one you should go with. if you need to render the show page without going to the action. i guess you might need to add this `@company = @companies.first` after this `if @companies.count == 1` and then `render company_path` and return after render company_path. – Athar Aug 10 '15 at 18:29

3 Answers3

1

The return is definitely killing you, but you're trying to render / redirect to a path for a specific resource without specifying the resource. I've taken a stab at something that might work a bit better for you:

class MyController
  before_action :find_companies, only: :index
  before_action :find_company, only: :show
  before_action :show_company_if_matched, only: :index

  def index
    # do whatever you were doing here...
  end

  def show
    respond_to do |format|
      format.html # show.html.erb
      format.js # show.js.erb
      format.json { render json: @company }
    end
    # more code 
  end

  private

  def find_companies
    @companies = Company.search(params[:query]).results
  end

  def find_company
    client = Elasticsearch::Client.new host:'127.0.0.1:9200', log: true
    response = client.search index: 'companies', body: {query: { match: {_id: params[:id]} } }
    @company = response['hits']['hits'][0]['_source']
  end

  def show_company_if_matched
    redirect_to company_path(@comapnies.first) if @companies.total == 1
  end
end

EDIT: Updated to include show action

Ian Selby
  • 3,241
  • 1
  • 25
  • 18
  • Thank you. It great approach, If I wouldn't execute second query in show action. Check my updated code. – Misha Aug 10 '15 at 18:36
0

Remove the return from your controller. If I've understood your question, this should result in the behavior you're looking for:

 if @count == 1
   render company_path  
 else
   # Do something else
 end

If there is subsequent code in the controller that you do not want to execute, you can render and return as follows:

 if @count == 1
   render company_path and return
 else
   # Do something else
 end
Sculper
  • 756
  • 2
  • 12
  • 24
0

This is correct syntax :

def index
 @companies = Company.search(params[:query]).results
 @count = @companies.total
 if @count == 1
   render company_path # no params ?
   return
 else
   redirect_to root_path
   return
 end
end

Use return after render or redirect it's good practice, because in some cases 'render' or 'redirect_to' do not do 'return' (cf: best practice ruby)

Gearnode
  • 693
  • 7
  • 21
  • I don't doubt you, but do you have a citation for that? My understanding is that the most "railsy" approach is to not explicitly return from a controller unless there is more control flow after redirects or renders. – Sculper Aug 10 '15 at 18:08
  • Yes, the [airbnb best practice](https://github.com/airbnb/ruby). This rule comes from one of their conferences but I don't know which one sorry. – Gearnode Aug 10 '15 at 18:16