Ajuda para diminuir minha action create - Ruby on Br

90 semanas atrás Mawcor 48 posts	Boa tarde pessoal, queria uma ajuda para diminuir minha action. Essa action é responsavel por criar as minhas tasks no banco. def create @find = @service.tasks.all puts "Entrou em Create" if @find.size == 0 #Caso não exista dados no banco puts "Entrou em find == 0 (Banco esta vazio)(criar task normalmente)" @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id respond_to do \|format\| if @task.save flash[:notice] = 'Contrato criado com sucesso!' format.html { redirect_to alltasks_path } format.xml { render :xml => @task, :status => :created, :location => @task } else format.html { render :action => "new" } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } end end else puts "Entrou em find diferente 0 (Existe dados no banco)" @find.each do \|x\| #verifica se o current_user tem task ja criadas no banco if x.customer_id == current_user.id puts "Current_user ja soliciou um serviço" if x.service_id == @service.id puts "Achou service solicitado pelo user #{current_user.name}" flash[:error] = 'Erro - A operação não pode ser concluída - Esse Serviço já consta como Contratado' redirect_to alltasks_path else #current_user ja solicitou um serviço, mas é diferente do serviço atual) puts "Entrou no Else de find diferente de 0" @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id respond_to do \|format\| if @task.save flash[:notice] = 'Contrato criado com sucesso!' format.html { redirect_to alltasks_path } format.xml { render :xml => @task, :status => :created, :location => @task } else format.html { render :action => "new" } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } end end end else puts "Esse current_user nao tem serviço solicitado ainda (Criar task normalmente)" @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id respond_to do \|format\| if @task.save flash[:notice] = 'Contrato criado com sucesso!' format.html { redirect_to alltasks_path } format.xml { render :xml => @task, :status => :created, :location => @task } else format.html { render :action => "new" } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } end end end end puts "Acabou o find diferente de 0" end end Ps. Só programei java antes de ruby/rails. Obrigado pela ajuda e ensino.

90 semanas atrás Mereghost 1476 posts	Mawcor: Dá pra melhorar bastante aí, já que tem repetição a dar com pau. Mas sem conhecer a estrutura da app fica um pouco difícil. Dá pra postar ao menos como os modelos envolvidos estão relacionados? def create tasks = @service.tasks if tasks.empty? @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id else tasks.each do \|x\| #verifica se o current_user tem task ja criadas no banco if x.customer_id == current_user.id if x.service_id == @service.id flash[:error] = 'Erro...' redirect_to alltasks_path else #current_user ja solicitou um serviço, mas é diferente do serviço atual) @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id end else @task = @service.tasks.build(params[:task]) @task.firm_id = @service.account.id @task.customer_id = current_user.id @task.service_id = @service.id end end end respond_to do \|format\| if @task.save flash[:notice] = 'Contrato criado com sucesso!' format.html { redirect_to alltasks_path } format.xml { render :xml => @task, :status => :created, :location => @task } else format.html { render :action => "new" } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } end end end

90 semanas atrás Mawcor 48 posts	Opa, vou postar como esta meus models entao: class Task < ActiveRecord::Base belongs_to :firm, :class_name => "User" belongs_to :customer, :class_name => "User" belongs_to :service endclass Service < ActiveRecord::Base belongs_to :account has_many :tasks, :dependent=>:destroy end Vlw!

90 semanas atrás nofxx 1960 posts	Bro, com uns controller desse nem desce pro play q ninguem vai qrer brincar com vc. Principio/norma/convenção básica e irrefutável: Controllers magros, models gordos. Controller ñ precisava nem ter mudado, deixe toda a lógica nos models.

90 semanas atrás Mawcor 48 posts	Vlw pela dica nofx, mas me ajuda a dar um enxugada no codigo tbm?! Abraço! edit: Pow mereghost. Voce diminuiu 40% do meu codigo! rsrsrsrs show de bola amigo! Vivendo e aprendendo.

90 semanas atrás Urubatan 165 posts	Algumas sugestões que melhorariam o código na minha opinião: O serviço retornar tasks que não pertencem a ele não iria acontecer, então esta verificação no código não é útil. Adicionar uma lista de tasks no modelo User (se é este o nome do modelo do current_user), assim o código fica bem mais simples e a lista de tarefas a ser iterada deve diminuir mover o create para dentro do service, um create_task para este serviço parece bem lógico, pdoeria receber o current_user e o hash de parâmetros … Pelo código que esta no post, tu adiciona a task sempre com os mesmos parâmetros, a única excessão é se o usuário já tem uma task para aquele serviço então o código poderia ficar assim class Task < ActiveRecord::Base belongs_to :firm, :class_name => "User" belongs_to :customer, :class_name => "User" belongs_to :service named_scope :from_user, lambda { \|user_id\| { :conditions => {:customer_id => user_id} } } end class Service < ActiveRecord::Base belongs_to :account has_many :tasks, :dependent => :destroy def build_task(user,hash) #verifica se este usuário ainda não tem nenhuma task para este serviço if tasks.from_user(user.id).empty? task = tasks.build(hash) task.firm = account task.customer = current_user task else nil end end end #dentro do controller @task = @service.build_task current_user, params[:task] if !@task flash[:error] = 'Erro - A operação não pode ser concluída - Esse Serviço já consta como Contratado' redirect_to alltasks_path else respond_to do \|format\| if @task.save flash[:notice] = 'Contrato criado com sucesso!' format.html { redirect_to alltasks_path } format.xml { render :xml => @task, :status => :created, :location => @task } else format.html { render :action => "new" } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } end end end E só pra constar, se tiver mais de uma task na lista, o teu código original já ta quebrado, ele teria que verificar em toda a lista para verificar se o usuário já tem tasks criadas ou já solicitou serviços, e não fazer esta verificação e criar uma task se nenhuma das duas verificações for verdadeira a cada iteração do loop.

90 semanas atrás Urubatan 165 posts	PS.: que porquera é esta de line breaks no final de cada linha do código que escrevi?

90 semanas atrás Mawcor 48 posts	Obrigado pela ajuda Urubatan. Realmente o código fica bem mais legivel assim e mais MVC tbm. A proposito, comprei seu livro, estou lendo ele ainda. Muito bom.

88 semanas atrás Mawcor 48 posts	Fui tentar implementar o codigo acima no meu projeto. Mas surgiu um problema. Não sei como chamar esse novo método criado no service_controller. Atualmente estou criando um novo task pela view new de task, que chama automaticamente a action create do task_controller. <% form_for([@service, @task]) do \|f\| %> <%= f.error_messages %> <p> <%= f.label :firm, "Empresa: #{@service.account.name}" %><br /> <%= f.text_field :firm %> </p> <p> <%= f.label :customer, "Cliente: #{current_user.name}" %><br /> <%= f.text_field :customer %> </p> <p> <%= f.label :service, "Serviço: #{@service.name}" %><br /> <%= f.text_field :service %> </p> <p> <%= f.submit button,:confirm => "Deseja contratar o serviço de #{@service.name}?" %> </p> <% end %> Como chamar essa nova action que esta no service_controller quando eu aperto o botão submit?