Commit 5450e1d82869af981b0799c5b6e0a34eda6ef354
1 parent
da6c1271
Exists in
master
and in
28 other branches
Fix double submission load (remove ||=)
This makes possible to move controllers' code to the model, where it should be in first place (ActionItem3168)
Showing
5 changed files
with
49 additions
and
60 deletions
Show diff stats
plugins/custom_forms/controllers/custom_forms_plugin_profile_controller.rb
... | ... | @@ -6,40 +6,26 @@ class CustomFormsPluginProfileController < ProfileController |
6 | 6 | |
7 | 7 | @form = CustomFormsPlugin::Form.find(params[:id]) |
8 | 8 | if user |
9 | - @submission ||= CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) | |
9 | + @submission = CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) | |
10 | 10 | @submission ||= CustomFormsPlugin::Submission.new(:form_id => @form.id, :profile_id => user.id) |
11 | 11 | else |
12 | - @submission ||= CustomFormsPlugin::Submission.new(:form_id => @form.id) | |
12 | + @submission = CustomFormsPlugin::Submission.new(:form_id => @form.id) | |
13 | 13 | end |
14 | 14 | |
15 | 15 | # build the answers |
16 | - @submission.answers.push(*(answers = build_answers(params[:submission], @form))) if params[:submission] | |
16 | + @answers = if params[:submission] then @submission.build_answers params[:submission] else @submission.answers end | |
17 | 17 | |
18 | 18 | if request.post? |
19 | 19 | begin |
20 | 20 | raise 'Submission already present!' if user.present? && CustomFormsPlugin::Submission.find_by_form_id_and_profile_id(@form.id,user.id) |
21 | 21 | raise 'Form expired!' if @form.expired? |
22 | 22 | |
23 | - # @submission.answers for some reason has the same answer twice | |
24 | - failed_answers = answers.select {|answer| !answer.valid? } | |
25 | - | |
26 | - if failed_answers.empty? | |
27 | - # Save the submission | |
28 | - ActiveRecord::Base.transaction do | |
29 | - if !user | |
30 | - @submission.author_name = params[:author_name] | |
31 | - @submission.author_email = params[:author_email] | |
32 | - end | |
33 | - @submission.save! | |
34 | - end | |
35 | - else | |
36 | - @submission.errors.clear | |
37 | - failed_answers.each do |answer| | |
38 | - answer.valid? | |
39 | - answer.errors.each do |attribute, msg| | |
40 | - @submission.errors.add(answer.field.id.to_s.to_sym, msg) | |
41 | - end | |
42 | - end | |
23 | + if !user | |
24 | + @submission.author_name = params[:author_name] | |
25 | + @submission.author_email = params[:author_email] | |
26 | + end | |
27 | + | |
28 | + if not @submission.save | |
43 | 29 | raise 'Submission failed: answers not valid' |
44 | 30 | end |
45 | 31 | session[:notice] = _('Submission saved') | ... | ... |
plugins/custom_forms/lib/custom_forms_plugin/helper.rb
1 | 1 | module CustomFormsPlugin::Helper |
2 | + | |
3 | + protected | |
4 | + | |
2 | 5 | def html_for_field(builder, association, klass) |
3 | 6 | new_object = klass.new |
4 | 7 | builder.fields_for(association, new_object, :child_index => "new_#{association}") do |f| |
... | ... | @@ -117,22 +120,4 @@ module CustomFormsPlugin::Helper |
117 | 120 | type_for_options(field.class) == 'select_field' && field.select_field_type == 'check_box' |
118 | 121 | end |
119 | 122 | |
120 | - def build_answers(submission, form) | |
121 | - answers = [] | |
122 | - form.fields.each do |field| | |
123 | - final_value = '' | |
124 | - if submission.has_key?(field.id.to_s) | |
125 | - value = submission[field.id.to_s] | |
126 | - if value.kind_of?(String) | |
127 | - final_value = value | |
128 | - elsif value.kind_of?(Array) | |
129 | - final_value = value.join(',') | |
130 | - elsif value.kind_of?(Hash) | |
131 | - final_value = value.map {|option, present| present == '1' ? option : nil}.compact.join(',') | |
132 | - end | |
133 | - end | |
134 | - answers << CustomFormsPlugin::Answer.new(:field => field, :value => final_value) | |
135 | - end | |
136 | - answers | |
137 | - end | |
138 | 123 | end | ... | ... |
plugins/custom_forms/lib/custom_forms_plugin/membership_survey.rb
... | ... | @@ -11,23 +11,9 @@ class CustomFormsPlugin::MembershipSurvey < Task |
11 | 11 | form = CustomFormsPlugin::Form.find(form_id) |
12 | 12 | raise 'Form expired' if form.expired? |
13 | 13 | |
14 | - answers = build_answers(submission, form) | |
15 | 14 | s = CustomFormsPlugin::Submission.create!(:form => form, :profile => target) |
16 | - s.answers.push(*answers) | |
17 | - | |
18 | - failed_answers = answers.select {|answer| !answer.valid? } | |
19 | - if failed_answers.empty? | |
20 | - s.save! | |
21 | - else | |
22 | - s.errors.clear | |
23 | - answers.each do |answer| | |
24 | - answer.valid? | |
25 | - answer.errors.each do |attribute, msg| | |
26 | - s.errors.add(answer.field.id.to_s.to_sym, msg) | |
27 | - end | |
28 | - end | |
29 | - raise ActiveRecord::RecordInvalid, s | |
30 | - end | |
15 | + s.build_answers submission | |
16 | + s.save! | |
31 | 17 | end |
32 | 18 | |
33 | 19 | def title | ... | ... |
plugins/custom_forms/lib/custom_forms_plugin/submission.rb
... | ... | @@ -2,12 +2,14 @@ class CustomFormsPlugin::Submission < Noosfero::Plugin::ActiveRecord |
2 | 2 | belongs_to :form, :class_name => 'CustomFormsPlugin::Form' |
3 | 3 | belongs_to :profile |
4 | 4 | |
5 | - has_many :answers, :class_name => 'CustomFormsPlugin::Answer', :dependent => :destroy | |
5 | + # validation is done manually, see below | |
6 | + has_many :answers, :class_name => 'CustomFormsPlugin::Answer', :dependent => :destroy, :validate => false | |
6 | 7 | |
7 | 8 | validates_presence_of :form |
8 | 9 | validates_presence_of :author_name, :author_email, :if => lambda {|submission| submission.profile.nil?} |
9 | 10 | validates_uniqueness_of :author_email, :scope => :form_id, :allow_nil => true |
10 | 11 | validates_format_of :author_email, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda {|submission| !submission.author_email.blank?}) |
12 | + validate :check_answers | |
11 | 13 | |
12 | 14 | def self.human_attribute_name(attrib) |
13 | 15 | if /\d+/ =~ attrib and (f = CustomFormsPlugin::Field.find_by_id(attrib.to_i)) |
... | ... | @@ -17,5 +19,35 @@ class CustomFormsPlugin::Submission < Noosfero::Plugin::ActiveRecord |
17 | 19 | end |
18 | 20 | end |
19 | 21 | |
22 | + def build_answers submission | |
23 | + self.form.fields.each do |field| | |
24 | + next unless value = submission[field.id.to_s] | |
25 | + | |
26 | + final_value = '' | |
27 | + if value.kind_of?(String) | |
28 | + final_value = value | |
29 | + elsif value.kind_of?(Array) | |
30 | + final_value = value.join(',') | |
31 | + elsif value.kind_of?(Hash) | |
32 | + final_value = value.map {|option, present| present == '1' ? option : nil}.compact.join(',') | |
33 | + end | |
34 | + | |
35 | + self.answers.build :field => field, :value => final_value | |
36 | + end | |
37 | + | |
38 | + self.answers | |
39 | + end | |
40 | + | |
41 | + protected | |
42 | + | |
43 | + def check_answers | |
44 | + self.answers.each do |answer| | |
45 | + answer.valid? | |
46 | + answer.errors.each do |attribute, msg| | |
47 | + self.errors.add answer.field.id.to_s.to_sym, msg | |
48 | + end | |
49 | + end | |
50 | + end | |
51 | + | |
20 | 52 | end |
21 | 53 | ... | ... |
plugins/custom_forms/test/unit/custom_forms_plugin/submission_test.rb
... | ... | @@ -36,8 +36,8 @@ class CustomFormsPlugin::SubmissionTest < ActiveSupport::TestCase |
36 | 36 | form = CustomFormsPlugin::Form.create!(:name => 'Free Software', :profile => fast_create(Profile)) |
37 | 37 | field = CustomFormsPlugin::Field.create!(:name => 'License', :form => form) |
38 | 38 | submission = CustomFormsPlugin::Submission.create!(:form => form, :profile => fast_create(Profile)) |
39 | - a1 = CustomFormsPlugin::Answer.create!(:field => field, :submission => submission) | |
40 | - a2 = CustomFormsPlugin::Answer.create!(:field => field, :submission => submission) | |
39 | + a1 = submission.answers.create!(:field => field, :submission => submission) | |
40 | + a2 = submission.answers.create!(:field => field, :submission => submission) | |
41 | 41 | |
42 | 42 | assert_includes submission.answers, a1 |
43 | 43 | assert_includes submission.answers, a2 | ... | ... |