2015-01-30 2 views
1

Я определил эту переменную в моей точки зренияMove логика в Rails контроллера

<% value_one.each do |s| %> 
    <% document = current_user.documents.where(skill_id: s.id) %> 
<% end %> 

Так им найти документы пользователей на основе skill_id (ов).

Но мне интересно, как я могу переместить эту логику в контроллер и назначить ее, чтобы сказать @documents. Я не уверен, как передать параметры s.id на контроллер?

Я думаю об этом неправильно? или есть простой способ сделать это

Update

На основании ответа ниже мой контроллер выглядит так

def index 
    @skills = Skill.all 
    @documents = current_user.documents.where(skill_id: @skills.map(&:id)).all 
end 

теперь у меня есть 1 один запрос (к счастью)

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]] 

Спасибо

+0

Возможно, было бы более целесообразно поместить эту логику в модель вместо контроллера. –

ответ

2

Вы запрашиваете свои документы наименее эффективным способом. Если ваш value_one имеет 100 элементов, вы выполняете 100 запросов.

Вместо этого вы должны сделать один запрос для всех элементов вопрос:

# Assuming 'value_one' is available.... 
@documents = current_user.documents.where(skill_id: value_one.map(&:id)).all 

Вам придется фактически магазин/отобразить объекты любые значения, которые в value_one. Простым решением было бы сохранить текущий цикл и найти объект из уже загруженного списка объектов:

<% value_one.each do |s| %> 
    <% document = @documents.select { |d| d.skill_id == s.id } %> 
<% end %> 
+0

Да, я заметил, что мой запрос для 'document' очень низок, поэтому я хочу улучшить его и узнать о лучшей практике в этой ситуации. Спасибо за вашу помощь. – Richlewis

+0

ok im get 'неопределенная локальная переменная или метод value_one', но я знаю, что вы упомянули о том, что value_one доступен. Как бы я это сделал? – Richlewis

+0

О, подождите, я думаю, я понимаю, что вы имеете в виду сейчас. Ive обновил мой вопрос, чтобы показать мой текущий контроллер, если вы можете подтвердить, что это правильно (хотя, глядя на запрос сейчас, я думаю, что это правильно) – Richlewis

0

Я думаю, вам нужна функция с одним параметром. Присвоение его глобальной переменной не будет работать, так как вам нужен этот параметр для запроса. Так что я бы сказал, что вы могли бы сделать

def userDocumentsForSkill(sid) 
    return @current_user.documents.where(skill_id: sid) 
end 

... или подобное и в шаблоне:

<% value_one.each do |s| %> 
    <% document = userDocumentsForSkill(s.id) %> 
<% end %> 

Не тестировался, но я надеюсь, что вы получите эту идею.

0

У вас не должно быть ЛЮБОЙ логики в представлении.

Предполагая, что у вас есть связь между документом и навыком (skill has_many documents), вы можете сделать document.includes(:skill) или joins(:skill). Здесь вы можете добавить дополнительные условия.

Это будет генерировать только один запрос к базе данных, и ваше представление будет очищено от любой логики.

Если вы хотите, чтобы я предоставил полный код, пожалуйста, обновите вопрос с помощью контроллера для этого конкретного действия.

Смежные вопросы