Comments and changes to this ticket

• You flagged this item as spam.
  

  Christian Weyer July 7th, 2008 @ 11:38 PM

  • Tag set to “activerecord, has_many, tested”

  I also ran into this and it sucks like hell. It is a clear violation of the validation system in Rails.

  I see two possible solutions:

  • Delete should only remove the element from the collection and wait to do the database stuff until the parent's save method is called. I agree that a direct deletion might be useful sometimes (but put a warning in the documentation about skipping validations then!), so here is another possibility:
  • Do it like collection.<< and collection.build. When using << the attached object is saved instantly whereas build saves the object when the parents save method is called. Such a thing should exists for deletion, too.

• You flagged this item as spam.
  

  Rob Dupuis August 1st, 2008 @ 10:53 PM

  • Tag changed from “activerecord, has_many, tested” to “activerecord, bug, has_many, tested”

  I'm hitting this exact problem. Does anyone have a workaround for this? Thanks.

• You flagged this item as spam.
  

  Jacob Burkhart August 2nd, 2008 @ 03:37 AM

  The workaround is to delete from the collection and then re-assign to the association:

  for example, instead of:

  group.users.delete(bob)

  you have to do something like:

  g_users = group.users.to_a

  g_users.delete(bob)

  group.users = g_users

  group.save!

  bob.destroy

• You flagged this item as spam.
  

  Rob Dupuis August 2nd, 2008 @ 01:31 PM

  I don't think this works (at least for me). As soon as I do the assignment (group.users = g_users) the sql is instantly executed to update the has_many collection. Therefore the update has been committed before I can validate the new collection.

  Thanks

• You flagged this item as spam.
  

  Zef RosnBrick August 4th, 2008 @ 12:28 AM

  This seems to work for me. It is based on Railscast #75 http://railscasts.com/episodes/75, where Projects have many Tasks. I also give each task a position to allow for reordering (not shown).

  project.rb

  has_many :tasks, :dependent => :destroy

  #validation statements here...

  after_update :save_tasks

  def tasks=(tasks_attributes)

  tasks_attributes[:new_tasks] ||= []

  position = 0

  #For existing tasks

  tasks.each do |task|

  attributes = tasks_attributes[task.id_s]

  unless attributes.nil?

  task.attributes = attributes

  unless task.should_delete

  task.position = position

  position += 1

  end

  end

  end

  #For new tasks

  tasks_attributes[:new_tasks].each do |task_attributes|

  task_attributes[:position] = position

  tasks.build(task_attributes)

  position += 1

  end

  end

  def save_tasks

  tasks.each do |task|

  if (task.should_delete != true)

  task.save(false)

  else

  tasks.delete(task)

  end

  end

  end

  def id_s(element = nil)

  id = self.id || self.object_id

  if element.nil?

  "#{self.class.to_s.underscore}_#{id}"

  else

  "#{self.class.to_s.underscore}_#{id}_#{element}"

  end

  end

  protected

  def validate

  tasks.each_with_index do |task, position|

  errors.add_to_base("Task #{position + 1} Name cannot be blank.") if (task.name.blank?)

  end

  end

  ------------------

  task.rb

  belongs_to :project

  validates_presence_of :name

  attr_accessor :should_delete

  def should_delete

  @should_delete || false

  end

  def should_delete=(bool)

  if (bool == false)

  @should_delete = false

  else

  @should_delete = true

  end

  end

  def id_s(element = nil)

  id = self.id || self.object_id

  if element.nil?

  "#{self.class.to_s.underscore}_#{id}"

  else

  "#{self.class.to_s.underscore}_#{id}_#{element}"

  end

  end

  ------------------

  new.rhtml

  <% form_tag({:controller => Project, :action => "new"}) do %>

  New Project

  <%= error_messages_for :project %>

  <%= label :project, :name, "Name:" %> <%= text_field :project, :name %>

  <%= render :partial => 'task', :collection => @project.tasks %>

  <%= link_to_remote("Add Task", :update => :tasks, :position => :bottom, :url => {:controller => Project, :action => "add_task"}) %>

  <%= submit_tag "Create", :name => 'submit_action' %>

  <%= submit_tag "Cancel", :name => 'submit_action' %>

  <% end %>

  ------------------

  _task.rhtml

  <% unless task.new_record? %>

  <%= label_tag task.id_s("name"), "Task:" %>

  <%= text_field_tag "project[tasks][#{task.id_s}][name]", task.name, :id => task.id_s("name") %>

  <%= check_box_tag "project[tasks][#{task.id_s}][should_delete]", 1, task.should_delete, :id => task.id_s("should_delete") %>

  <%= label_tag task.id_s("should_delete"), "Delete" %>

  <% else %>

  <%= label_tag task.id_s("name"), "Task:" %>

  <%= text_field_tag "project[tasks][new_tasks][][name]", task.name, :id => task.id_s("name") %>

  <%= link_to_function "remove", "removeTask('#{task.id_s}')" %>

  <% end %>

• You flagged this item as spam.
  

  Zef RosnBrick August 4th, 2008 @ 12:31 AM

  This seems to work for me. It is based on Railscast 75 http://railscasts.com/episodes/75, where Projects have many Tasks. I also give each task a position to allow for reordering (not shown).

  project.rb

  
  has_many :tasks, :dependent => :destroy
  
  #validation statements here...
  
  after_update :save_tasks
  
  def tasks=(tasks_attributes)
  	tasks_attributes[:new_tasks] ||= []
  	position = 0
  	
  	#For existing tasks
  	tasks.each do |task|
  		attributes = tasks_attributes[task.id_s]
  		unless attributes.nil?
  			task.attributes = attributes
  			unless task.should_delete
  				task.position = position
  				position += 1
  			end
  		end
  	end
  	
  	#For new tasks
  	tasks_attributes[:new_tasks].each do |task_attributes|
  		task_attributes[:position] = position
  		tasks.build(task_attributes)
  		position += 1
  	end
  end
  
  def save_tasks
   	tasks.each do |task|
  		if (task.should_delete != true)
      			task.save(false)
  		else
  			tasks.delete(task)
  		end
  	end
  end
  
  def id_s(element = nil)
  	id = self.id || self.object_id
  	if element.nil?
    		"#{self.class.to_s.underscore}_#{id}"
      	else
        		"#{self.class.to_s.underscore}_#{id}_#{element}"
      	end
  end
  
  protected
  	
  def validate
  	tasks.each_with_index do |task, position|
  		errors.add_to_base("Task #{position + 1} Name cannot be blank.") if (task.name.blank?)
  	end
  end
  
  

  ------------------

  task.rb

  
  belongs_to :project
  
  validates_presence_of :name
  
  attr_accessor :should_delete
  	
  def should_delete
  	@should_delete || false
  end
  	
  def should_delete=(bool)
  	if (bool == false)
  		@should_delete = false
  	else
  		@should_delete = true
  	end
  end
  	
  def id_s(element = nil)
  	id = self.id || self.object_id
  	if element.nil?
    		"#{self.class.to_s.underscore}_#{id}"
      	else
        		"#{self.class.to_s.underscore}_#{id}_#{element}"
      	end
  end
  
  

  ------------------

  new.rhtml

  
  <% form_tag({:controller => Project, :action => "new"}) do %>
  	<h1>New Project</h1>
  	
  	<%= error_messages_for :project %>
  
  	<div class="form_div">
  	  <%= label :project, :name, "Name:" %> <%= text_field :project, :name %>
  	</div>
  	
  	<div class="form_div">
  		<div id="tasks">
  		 	<%= render :partial => 'task', :collection => @project.tasks %>
  		</div>
  		<%= link_to_remote("Add Task", :update => :tasks, :position => :bottom, :url => {:controller => Project, :action => "add_task"}) %>
  	</div>
  
    <div class="submit_div">
  		<%= submit_tag "Create", :name => 'submit_action' %>
  		<%= submit_tag "Cancel", :name => 'submit_action' %>
  	</div>
  <% end %>
  
  

  ------------------

  _task.rhtml

  
  <div id="<%= task.id_s %>" class="task">
  	<% unless task.new_record? %>
  		<%= label_tag task.id_s("name"), "Task:" %>
  		<%= text_field_tag "project[tasks][#{task.id_s}][name]", task.name, :id => task.id_s("name") %>
  		<%= check_box_tag "project[tasks][#{task.id_s}][should_delete]", 1, task.should_delete, :id => task.id_s("should_delete") %>
  		<%= label_tag task.id_s("should_delete"), "Delete" %>
  	<% else %>
  		<%= label_tag task.id_s("name"), "Task:" %>
  		<%= text_field_tag "project[tasks][new_tasks][][name]", task.name, :id => task.id_s("name") %>
  		<%= link_to_function "remove", "removeTask('#{task.id_s}')" %>
  	<% end %>
  </div>
  
  

• You flagged this item as spam.
  

  Zef RosnBrick August 4th, 2008 @ 02:44 AM

  Here is a better solution to allow for sorting of tasks using scriptaculous drag and drop.

  project_controller.rb

  class ProjectController < ApplicationController
  	def index
  		redirect_to :action => 'list'
  	end
  	
  	def new
  		submit_action = params['submit_action']
  		
  		if (submit_action == nil)
  		  @project = Project.new
  		  @project.tasks.build
  		elsif (submit_action == "Create")
  			@project = Project.new(params[:project])
  		  if @project.save
  		    flash[:notice] = "Successfully created project and tasks."
  		    redirect_to :action => 'list'
  		  end
  		elsif (submit_action == "Cancel")
  			redirect_to :action => 'list'
  		end
  	end
  
  	def edit
  		submit_action = params['submit_action']
  		
  		begin
  	  	@project = Project.find(params[:id], :include => :tasks)
  		rescue ActiveRecord::RecordNotFound
  			flash[:notice] = "Invalid Project."
  			redirect_to :action => 'list'
  		else
  			if (submit_action == nil)
  			elsif (submit_action == "Save")
  			  if @project.update_attributes(params[:project])
  			    flash[:notice] = "Successfully updated project and tasks."
  			    redirect_to :action => 'list'
  			  end
  			elsif (submit_action == "Cancel")
  				redirect_to :action => 'list'
  			end
  		end
  	end
  	
  	def delete
  		submit_action = params['submit_action']
  		
  		begin
  	  	@project = Project.find(params[:id])
  		rescue ActiveRecord::RecordNotFound
  			flash[:notice] = "Invalid Project."
  			redirect_to :action => 'list'
  		else
  			if (submit_action == nil)
  			elsif (submit_action == "Delete")
  				@project.destroy
  			  flash[:notice] = "Successfully deleted project and tasks."
  		    redirect_to :action => 'list'
  			elsif (submit_action == "Cancel")
  				redirect_to :action => 'list'
  			end
  		end
  	end
  		
  	def list
  		@projects = Project.find(:all, :include => :tasks)
  	end
  	
  	def add_task
  		render :partial => "task", :object => Task.new()
  	end
  end
  

  application_helper.rb

  # Methods added to this helper will be available to all templates in the application.
  module ApplicationHelper
  	def label_tag(for_tag, name)
  		"<label for='#{for_tag}'>#{name}</label>"
  	end
  end
  

  project.rb

  class Project < ActiveRecord::Base
    has_many :tasks, :dependent => :destroy
  
    validates_presence_of :name
  	validates_uniqueness_of :name
  
    after_update :save_tasks
  
  	def tasks=(tasks_attributes)
  		existing_tasks = {}
  		new_tasks = []
  		
  		tasks_attributes.each_with_index do |task_attributes, position|
  			task_attributes[:position] = position
  			if (task_attributes.has_key?(:id))
  				id = task_attributes.delete(:id)
  				existing_tasks[id] = task_attributes
  			else
  				new_tasks.push(task_attributes)
  			end
  		end
  		
  		#For existing tasks
  		tasks.each do |task|
  			attributes = existing_tasks[task.id.to_s]
  			unless attributes.nil?
  				task.attributes = attributes
  			end
  		end
  		
  		#For new tasks
  		new_tasks.each do |task_attributes|
  			tasks.build(task_attributes)
  		end
  	end
  	  
    def save_tasks
      tasks.each do |task|
  			logger.info("#{task.id}: #{task.should_delete}")
  			if (task.should_delete != true)
        	task.save(false)
  			else
  				tasks.delete(task)
  			end
      end
    end
  	
  	def id_s(element = nil)
  		id = self.id || self.object_id
      if element.nil?
    	  "#{self.class.to_s.underscore}_#{id}"
      else
        "#{self.class.to_s.underscore}_#{id}_#{element}"
      end
    end
  
  	protected
  	
  	def validate
  		tasks.each_with_index do |task, position|
  			errors.add_to_base("Task #{position + 1} Name cannot be blank.") if (task.name.blank?)
  		end
  	end
  end
  

  task.rb

  class Task < ActiveRecord::Base
    belongs_to :project
  
    validates_presence_of :name
  
  	attr_accessor :should_delete
  	
  	def <=> other_task
  		position <=> other_task.position
  	end
  	
  	def should_delete
  		@should_delete || false
  	end
  	
  	def should_delete=(bool)
  		if (bool == false)
  			@should_delete = false
  		else
  			@should_delete = true
  		end
  	end
  	
  	def id_s(element = nil)
  		id = self.id || self.object_id
      if element.nil?
    	  "#{self.class.to_s.underscore}_#{id}"
      else
        "#{self.class.to_s.underscore}_#{id}_#{element}"
      end
    end
  end
  

  new.rhtml

  <% form_tag({:controller => Project, :action => "new"}) do %>
  	<h1>New Project</h1>
  	
  	<%= error_messages_for :project %>
  
  	<div class="form_div">
  	  <%= label :project, :name, "Name:" %> <%= text_field :project, :name %>
  	</div>
  	
  	<div class="form_div">
  		<div id="tasks">
  		 	<%= render :partial => 'task', :collection => @project.tasks.sort %>
  		</div>
  		<%= javascript_tag("createTaskSortable()") %>
  		<%= link_to_remote("Add Task", :update => :tasks, :position => :bottom, :url => {:controller => Project, :action => "add_task"}, :complete => "createTaskSortable()") %>
  	</div>
  
    <div class="submit_div">
  		<%= submit_tag "Create", :name => 'submit_action' %>
  		<%= submit_tag "Cancel", :name => 'submit_action' %>
  	</div>
  <% end %>
  

  edit.rhtml

  <% form_tag({:controller => Project, :action => "edit", :id => @project.id}) do %>
  	<h1>Edit Project</h1>
  	
  	<%= error_messages_for :project %>
  
  	<div class="form_div">
  	  <%= label :project, :name, "Name:" %> <%= text_field :project, :name %>
  	</div>
  	
  	<div class="form_div">
  		<div id="tasks">
  		 	<%= render :partial => 'task', :collection => @project.tasks.sort %>
  		</div>
  		<%= javascript_tag("createTaskSortable()") %>
  		<%= link_to_remote("Add Task", :update => :tasks, :position => :bottom, :url => {:controller => Project, :action => "add_task"}, :complete => "createTaskSortable()") %>
  	</div>
  
    <div class="submit_div">
  		<%= submit_tag "Save", :name => 'submit_action' %>
  		<%= submit_tag "Cancel", :name => 'submit_action' %>
  	</div>
  <% end %>
  

  _task.rhtml

  <div id="<%= task.id_s %>" class="task">
  	<%= image_tag("drag.png", :class => "drag_handle", :id => task.id_s("drag_handle"), :style => "cursor: move;") %>
  	<% unless task.new_record? %>
  		<%= hidden_field_tag "project[tasks][][id]", task.id, :id => task.id_s("id") %>
  	
  		<%= label_tag task.id_s("name"), "Task:" %>	<%= text_field_tag "project[tasks][][name]", task.name, :id => task.id_s("name") %>
  		
  		<%= check_box_tag "project[tasks][][should_delete]", 1, task.should_delete, :id => task.id_s("should_delete") %> <%= label_tag task.id_s("should_delete"), "Delete" %>
  	<% else %>
  		<%= label_tag task.id_s("name"), "Task:" %> <%= text_field_tag "project[tasks][][name]", task.name, :id => task.id_s("name") %>
  		
  		<%= link_to_function "remove", "removeTask('#{task.id_s}')" %>
  	<% end %>
  </div>
  

• 

  josh November 22nd, 2008 @ 07:50 PM

  • State changed from “new” to “stale”

  Staling out, please let me know if its still an issue.

• You flagged this item as spam.
  

  Jacob Burkhart February 9th, 2009 @ 08:01 PM

  Hi Josh,

  Just tested this again with Rails 2.3... Same code I attached with my original ticket. I still think this is a problem.

• You flagged this item as spam.
  

  Jacob Burkhart February 9th, 2009 @ 08:02 PM

  • Assigned user set to “josh”

  I wouldn't call this ticket stale... perhaps you really mean 'wontfix' ?