'Move logic into controller in a Rails app
I have defined this variable within my view
<% value_one.each do |s| %>
<% document = current_user.documents.where(skill_id: s.id ) %>
<% end %>
So I'm finding the users documents based on the skill_id (s).
But I am wondering how i can move this logic into the controller and assign it to say @documents. I am unsure on how to pass the params s.id to the controller though?
Am I thinking about this wrong, or is there a simple way to do this?
Update
Based on the answer below my controller looks like so
def index
@skills = Skill.all
@documents = current_user.documents.where(skill_id: @skills.map(&:id)).all
end
I now have 1 single query (thankfully)
Document Load (0.5ms) SELECT "documents".* FROM "documents" WHERE "documents"."user_id" = $1 AND "documents"."skill_id" IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70) [["user_id", 1]]
Solution 1:[1]
You're querying your documents in the least efficient way. If your value_one has 100 items, you're performing 100 queries.
Instead, you should do a single query for all the items in question:
# Assuming 'value_one' is available....
@documents = current_user.documents.where(skill_id: value_one.map(&:id)).all
You will have to actually store/map the objects to whatever values are in value_one. A simple solution would be to retain your current loop and find the object from the already loaded list of objects:
<% value_one.each do |s| %>
<% document = @documents.select { |d| d.skill_id == s.id } %>
<% end %>
Solution 2:[2]
I think you need a function with one parameter. Assigning it to a global variable will not work, since you need that parameter for the query. So I'd say you could do
def userDocumentsForSkill(sid)
return @current_user.documents.where(skill_id: sid)
end
... or similar and in your template:
<% value_one.each do |s| %>
<% document = userDocumentsForSkill(s.id) %>
<% end %>
Not tested this, but I hope you get the idea.
Solution 3:[3]
You shouldn't have ANY logic in the view.
Assuming that you have connection between document and skill (skill has_many documents) you can do document.includes(:skill) or joins(:skill). You can add additional conditions here.
This will generate only one query to the database, and your view will be cleared from any logic.
If you want me to provide full code please update question with controller for that particular action.
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
| Solution | Source |
|---|---|
| Solution 1 | user229044 |
| Solution 2 | Sebastian Wramba |
| Solution 3 | Miknash |
