From 5450e1d82869af981b0799c5b6e0a34eda6ef354 Mon Sep 17 00:00:00 2001 From: Braulio Bhavamitra Date: Mon, 16 Jun 2014 09:36:39 -0300 Subject: [PATCH] Fix double submission load (remove ||=) --- plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb | 32 +++++++++----------------------- plugins/custom_forms/lib/custom_forms_plugin/helper.rb | 21 +++------------------ plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb | 18 ++---------------- plugins/custom_forms/lib/custom_forms_plugin/submission.rb | 34 +++++++++++++++++++++++++++++++++- plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb | 4 ++-- 5 files changed, 49 insertions(+), 60 deletions(-) diff --git a/plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb b/plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb index e1138e7..f19fb32 100644 --- a/plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb +++ b/plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb @@ -6,40 +6,26 @@ class CustomFormsPluginProfileController < ProfileController @form = CustomFormsPlugin::Form.find(params[:id]) if user - @submission ||= CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) + @submission = CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) @submission ||= CustomFormsPlugin::Submission.new(:form_id => @form.id, :profile_id => user.id) else - @submission ||= CustomFormsPlugin::Submission.new(:form_id => @form.id) + @submission = CustomFormsPlugin::Submission.new(:form_id => @form.id) end # build the answers - @submission.answers.push(*(answers = build_answers(params[:submission], @form))) if params[:submission] + @answers = if params[:submission] then @submission.build_answers params[:submission] else @submission.answers end if request.post? begin raise 'Submission already present!' if user.present? && CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) raise 'Form expired!' if @form.expired? - # @submission.answers for some reason has the same answer twice - failed_answers = answers.select {|answer| !answer.valid? } - - if failed_answers.empty? - # Save the submission - ActiveRecord::Base.transaction do - if !user - @submission.author_name = params[:author_name] - @submission.author_email = params[:author_email] - end - @submission.save! - end - else - @submission.errors.clear - failed_answers.each do |answer| - answer.valid? - answer.errors.each do |attribute, msg| - @submission.errors.add(answer.field.id.to_s.to_sym, msg) - end - end + if !user + @submission.author_name = params[:author_name] + @submission.author_email = params[:author_email] + end + + if not @submission.save raise 'Submission failed: answers not valid' end session[:notice] = _('Submission saved') diff --git a/plugins/custom_forms/lib/custom_forms_plugin/helper.rb b/plugins/custom_forms/lib/custom_forms_plugin/helper.rb index 7f158b2..6bab99d 100644 --- a/plugins/custom_forms/lib/custom_forms_plugin/helper.rb +++ b/plugins/custom_forms/lib/custom_forms_plugin/helper.rb @@ -1,4 +1,7 @@ module CustomFormsPlugin::Helper + + protected + def html_for_field(builder, association, klass) new_object = klass.new builder.fields_for(association, new_object, :child_index => "new_#{association}") do |f| @@ -117,22 +120,4 @@ module CustomFormsPlugin::Helper type_for_options(field.class) == 'select_field' && field.select_field_type == 'check_box' end - def build_answers(submission, form) - answers = [] - form.fields.each do |field| - final_value = '' - if submission.has_key?(field.id.to_s) - value = submission[field.id.to_s] - if value.kind_of?(String) - final_value = value - elsif value.kind_of?(Array) - final_value = value.join(',') - elsif value.kind_of?(Hash) - final_value = value.map {|option, present| present == '1' ? option : nil}.compact.join(',') - end - end - answers << CustomFormsPlugin::Answer.new(:field => field, :value => final_value) - end - answers - end end diff --git a/plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb b/plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb index fa13893..3df8180 100644 --- a/plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb +++ b/plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb @@ -11,23 +11,9 @@ class CustomFormsPlugin::MembershipSurvey < Task form = CustomFormsPlugin::Form.find(form_id) raise 'Form expired' if form.expired? - answers = build_answers(submission, form) s = CustomFormsPlugin::Submission.create!(:form => form, :profile => target) - s.answers.push(*answers) - - failed_answers = answers.select {|answer| !answer.valid? } - if failed_answers.empty? - s.save! - else - s.errors.clear - answers.each do |answer| - answer.valid? - answer.errors.each do |attribute, msg| - s.errors.add(answer.field.id.to_s.to_sym, msg) - end - end - raise ActiveRecord::RecordInvalid, s - end + s.build_answers submission + s.save! end def title diff --git a/plugins/custom_forms/lib/custom_forms_plugin/submission.rb b/plugins/custom_forms/lib/custom_forms_plugin/submission.rb index 752872b..83fe2ec 100644 --- a/plugins/custom_forms/lib/custom_forms_plugin/submission.rb +++ b/plugins/custom_forms/lib/custom_forms_plugin/submission.rb @@ -2,12 +2,14 @@ class CustomFormsPlugin::Submission < Noosfero::Plugin::ActiveRecord belongs_to :form, :class_name => 'CustomFormsPlugin::Form' belongs_to :profile - has_many :answers, :class_name => 'CustomFormsPlugin::Answer', :dependent => :destroy + # validation is done manually, see below + has_many :answers, :class_name => 'CustomFormsPlugin::Answer', :dependent => :destroy, :validate => false validates_presence_of :form validates_presence_of :author_name, :author_email, :if => lambda {|submission| submission.profile.nil?} validates_uniqueness_of :author_email, :scope => :form_id, :allow_nil => true validates_format_of :author_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda {|submission| !submission.author_email.blank?}) + validate :check_answers def self.human_attribute_name(attrib) if /\d+/ =~ attrib and (f = CustomFormsPlugin::Field.find_by_id(attrib.to_i)) @@ -17,5 +19,35 @@ class CustomFormsPlugin::Submission < Noosfero::Plugin::ActiveRecord end end + def build_answers submission + self.form.fields.each do |field| + next unless value = submission[field.id.to_s] + + final_value = '' + if value.kind_of?(String) + final_value = value + elsif value.kind_of?(Array) + final_value = value.join(',') + elsif value.kind_of?(Hash) + final_value = value.map {|option, present| present == '1' ? option : nil}.compact.join(',') + end + + self.answers.build :field => field, :value => final_value + end + + self.answers + end + + protected + + def check_answers + self.answers.each do |answer| + answer.valid? + answer.errors.each do |attribute, msg| + self.errors.add answer.field.id.to_s.to_sym, msg + end + end + end + end diff --git a/plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb b/plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb index 947404c..8043114 100644 --- a/plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb +++ b/plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb @@ -36,8 +36,8 @@ class CustomFormsPlugin::SubmissionTest < ActiveSupport::TestCase form = CustomFormsPlugin::Form.create!(:name => 'Free Software', :profile => fast_create(Profile)) field = CustomFormsPlugin::Field.create!(:name => 'License', :form => form) submission = CustomFormsPlugin::Submission.create!(:form => form, :profile => fast_create(Profile)) - a1 = CustomFormsPlugin::Answer.create!(:field => field, :submission => submission) - a2 = CustomFormsPlugin::Answer.create!(:field => field, :submission => submission) + a1 = submission.answers.create!(:field => field, :submission => submission) + a2 = submission.answers.create!(:field => field, :submission => submission) assert_includes submission.answers, a1 assert_includes submission.answers, a2 -- libgit2 0.21.2