PiRSquared17 / activescaffold

Automatically exported from code.google.com/p/activescaffold
MIT License
0 stars 0 forks source link

Custom validation gets run twice when using nested scaffold #722

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Have the following models
def Company
 has_many :subscriptions
end

def Subscription

 belongs_to :company

 # custom validation, makes sure that start/expiry dates don't overlap
 def validate
  if company && company.subscriptions.all(:conditions => ['NOT (start_date
> ? OR expiry_date < ?)', expiry_date, start_date]).count != 0
   errors.add_to_base('Subscription must not overlap another subscription')
  end
 end

2. In CompaniesController I have the following to enable nested scaffold
for subscriptions:

config.nested.add_link("Subscriptions", [:subscriptions])

3. Attempting to create a new subscription from regular Subscription form
works like a charm with validation working properly. But as soon as another
valid (having non-overlapping date) subscription is added via the nested
scaffold from Companies, the validation fails. Although a new subscription
is in fact added to the database.

What is the expected output? What do you see instead?
In nested scaffold expect to see a successful subscription creation and
return back to nested listing. Instead it creates the subscription record
in the database, but then re-displays the subscription form with validation
error (which shouldn't happen). In the logs I can see the same validation
query gets executed once before the INSERT and after the INSERT via nested
scaffold.

What version (or revision) of the product are you using?
Rails 2.3.5, AS master from github, and  render_component plugin.

If this bug causes an exception, please paste at least the first 20 lines
below.
No exceptions. Just that validation gets executed twice with nested scaffold.

Original issue reported on code.google.com by netv...@gmail.com on 16 Dec 2009 at 12:30

GoogleCodeExporter commented 9 years ago

Rails log for nested scaffold:

-- validation query before insert, which confirms non-overlapping date
SELECT * FROM `subscriptions` WHERE (NOT (start_date > '2006-12-31' OR 
expiry_date <
'2006-12-14')) AND (`subscriptions`.company_id = 212) ORDER BY start_date
  Subscription Create (0.4ms)

-- successful insert into subscriptions
 INSERT INTO `subscriptions` (`start_date`, `membership_type_id`, `company_id`,
`expiry_date`) VALUES('2006-12-14', 2, 212, '2006-12-31')  

-- and then the same validation just before a COMMIT, which will now fail as the
record was already inserted
SELECT * FROM `subscriptions` WHERE (NOT (start_date > '2006-12-31' OR 
expiry_date <
'2006-12-14')) AND (`subscriptions`.company_id = 212) ORDER BY start_date

With regular scaffold, there is no secondary validation query before a COMMIT.

Original comment by netv...@gmail.com on 16 Dec 2009 at 12:39

GoogleCodeExporter commented 9 years ago
You can add id != #{self.id} to the conditions (like validates_uniqueness_of 
does)

Original comment by sergio.c...@gmail.com on 17 Dec 2009 at 8:55

GoogleCodeExporter commented 9 years ago
The new validate method as per your suggestion:

def validate
  if company && company.subscriptions.all(:conditions => ['(NOT (start_date > ? OR
expiry_date < ?)) AND subscriptions.id != ?', expiry_date, start_date,
self.id]).count != 0
    errors.add_to_base('Subscription must not overlap another subscription')
  end
end

This still produces the same result in nested scaffold. That is a subscription 
record
gets added but validation gets tripped up as if nothing got added due to an 
error.
Also because self.id is undefined for a new record, this allows to add 
overlapping
subscriptions on creation of a new record. However the validation works if 
editing
existing record (coz self.id is defined).

Hmmm... I'm really stumped by this one.

Original comment by netv...@gmail.com on 17 Dec 2009 at 10:24

GoogleCodeExporter commented 9 years ago
You're a genius Sergio!

After more thought about your proposed fix I have come up with this validation 
method
that appears to work in all situations. Posting here in case someone else runs 
into a
similar situation.

def validate
  if company
    cond = ['(NOT (start_date > ? OR expiry_date < ?))', expiry_date, start_date]

   # if self.id is defined apply it to conditions array 
   if !self.id.nil?
      cond[0] += ' AND id != ?'
      cond << self.id
    end

    if company.subscriptions.all(:conditions => cond).count != 0
      errors.add_to_base('Subscription must not overlap another subscription')
    end
  end
end

This thing is a quick hack to make it work. Any suggestions on how to improve 
this
are greatly appreciated.

Original comment by netv...@gmail.com on 17 Dec 2009 at 10:48

GoogleCodeExporter commented 9 years ago
I'm not sure it's a hack, it's the same as validates_uniqueness_of does (Have 
you
looked at the code of that method?), although validates_uniqueness_of uses 
"unless
new_record?" instead of "if !self.id.nil?"

Original comment by sergio.c...@gmail.com on 18 Dec 2009 at 11:44