diff options
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 72 | ||||
| -rw-r--r-- | app/models/public_body.rb | 45 | ||||
| -rw-r--r-- | app/views/admin_public_body/_form.rhtml | 32 | ||||
| -rw-r--r-- | app/views/admin_public_body/edit.rhtml | 6 | ||||
| -rw-r--r-- | app/views/admin_public_body/new.rhtml | 6 | ||||
| -rw-r--r-- | spec/controllers/admin_public_body_controller_spec.rb | 39 |
6 files changed, 102 insertions, 98 deletions
diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index 61e4aec54..bd85f6eed 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -82,77 +82,37 @@ class AdminPublicBodyController < AdminController end def new - @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do - @public_body = PublicBody.new - render - end - end - - def update_localized_attributes(public_body, params) - I18n.available_locales.each do |locale| - PublicBody.with_locale(locale) do - unless (attr_values = params["public_body_#{locale}"]).nil? - # 'publication_scheme' can't be null in the current DB schema - attr_values[:publication_scheme] = attr_values[:publication_scheme] || '' - public_body.attributes = attr_values - public_body.save! - end - end - end + @public_body = PublicBody.new + render end def create - # Start creating the public body in the default locale PublicBody.with_locale(I18n.default_locale) do - begin - ActiveRecord::Base.transaction do - params[:public_body][:last_edit_editor] = admin_http_auth_user() - @public_body = PublicBody.new(params[:public_body]) - @public_body.save! - - # Next, save the translations in the additional locales - update_localized_attributes(@public_body, params) - - flash[:notice] = 'PublicBody was successfully created.' - redirect_to admin_url('body/show/' + @public_body.id.to_s) - end - rescue ActiveRecord::RecordInvalid + params[:public_body][:last_edit_editor] = admin_http_auth_user() + @public_body = PublicBody.new(params[:public_body]) + if @public_body.save + flash[:notice] = 'PublicBody was successfully created.' + redirect_to admin_url('body/show/' + @public_body.id.to_s) + else render :action => 'new' end end end def edit - PublicBody.with_locale(I18n.default_locale) do - @public_body = PublicBody.find(params[:id]) - @public_body.last_edit_comment = "" - end - - # Additional locales (could remove the default, but won't make a difference) - I18n.available_locales.each do |locale| - instance_variable_set("@public_body_#{locale}", @public_body.translations.select{|t| t.locale==locale}.first) - end - + @public_body = PublicBody.find(params[:id]) + @public_body.last_edit_comment = "" render end def update - # Start updating the public body in the default locale PublicBody.with_locale(I18n.default_locale) do - begin - ActiveRecord::Base.transaction do - params[:public_body][:last_edit_editor] = admin_http_auth_user() - @public_body = PublicBody.find(params[:id]) - @public_body.update_attributes!(params[:public_body]) - - # Next, update the translations in the additional locales - update_localized_attributes(@public_body, params) - - flash[:notice] = 'PublicBody was successfully updated.' - redirect_to admin_url('body/show/' + @public_body.id.to_s) - end - rescue ActiveRecord::RecordInvalid + params[:public_body][:last_edit_editor] = admin_http_auth_user() + @public_body = PublicBody.find(params[:id]) + if @public_body.update_attributes(params[:public_body]) + flash[:notice] = 'PublicBody was successfully updated.' + redirect_to admin_url('body/show/' + @public_body.id.to_s) + else render :action => 'edit' end end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 2c7f80240..a476db8c0 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -38,7 +38,7 @@ class PublicBody < ActiveRecord::Base validates_uniqueness_of :short_name, :message => N_("Short name is already taken"), :if => Proc.new { |pb| pb.short_name != "" } validates_uniqueness_of :name, :message => N_("Name is already taken") - + has_many :info_requests, :order => 'created_at desc' has_many :track_things, :order => 'created_at desc' @@ -46,6 +46,40 @@ class PublicBody < ActiveRecord::Base translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme + # Convenience methods for creating/editing translations via forms + def translation(locale) + self.translations.find_by_locale(locale) + end + + # XXX - Don't like repeating this! + def calculate_cached_fields(t) + t.first_letter = t.name.scan(/^./mu)[0].upcase + short_long_name = t.name + short_long_name = t.short_name if t.short_name and !t.short_name.empty? + t.url_name = MySociety::Format.simplify_url_part(short_long_name, 'body') + end + + def translated_versions + translations + end + + def translated_versions=(translation_attrs) + if translation_attrs.respond_to? :each_value # Hash => updating + translation_attrs.each_value do |attrs| + t = translation(attrs[:locale]) || PublicBody::Translation.new + t.attributes = attrs + calculate_cached_fields(t) + t.save! + end + else # Array => creating + translation_attrs.each do |attrs| + new_translation = PublicBody::Translation.new(attrs) + calculate_cached_fields(new_translation) + translations << new_translation + end + end + end + # Make sure publication_scheme gets the correct default value. # (This would work automatically, were publication_scheme not a translated attribute) def after_initialize @@ -191,7 +225,6 @@ class PublicBody < ActiveRecord::Base # When name or short name is changed, also change the url name def short_name=(short_name) - globalize.write(self.class.locale || I18n.locale, :short_name, short_name) self[:short_name] = short_name self.update_url_name @@ -204,15 +237,15 @@ class PublicBody < ActiveRecord::Base end def update_url_name - url_name = MySociety::Format.simplify_url_part(self.short_or_long_name, 'body') - self.url_name = url_name + self.url_name = MySociety::Format.simplify_url_part(self.short_or_long_name, 'body') end + # Return the short name if present, or else long name def short_or_long_name - if self.short_name.nil? # can happen during construction + if self.short_name.nil? || self.short_name.empty? # 'nil' can happen during construction self.name else - self.short_name.empty? ? self.name : self.short_name + self.short_name end end diff --git a/app/views/admin_public_body/_form.rhtml b/app/views/admin_public_body/_form.rhtml index 1fb573994..98749154c 100644 --- a/app/views/admin_public_body/_form.rhtml +++ b/app/views/admin_public_body/_form.rhtml @@ -25,40 +25,52 @@ <% for locale in I18n.available_locales do - object_name = locale==:en ? 'public_body' : "public_body_#{locale.to_s}" + if locale==I18n.default_locale # The default locale is submitted as part of the bigger object... + prefix = 'public_body' + object = @public_body + else # ...but additional locales go "on the side" + prefix = "public_body[translated_versions][]" + object = @public_body.new_record? ? + PublicBody::Translation.new : + @public_body.translation(locale.to_s) || PublicBody::Translation.new + end + + fields_for prefix, object do |t| %> <div> <p>Locale: <%= locale_name(locale.to_s) %></p> - + <%= t.hidden_field :locale, :value => locale.to_s %> + <p><label for="public_body_name">Name</label><br/> - <%= text_field object_name, 'name', :size => 60 %></p> + <%= t.text_field :name, :size => 60 %></p> <p><label for="public_body_short_name">Short name <small>(only put in abbreviations which are really used, otherwise leave blank. Short or long name is used in the URL - don't worry about breaking URLs through renaming, as the history is used to redirect)</small></label><br/> - <%= text_field object_name, 'short_name', :size => 60 %></p> + <%= t.text_field :short_name, :size => 60 %></p> <p><label for="public_body_request_email">Request email <small>(set to <strong>blank</strong> (empty string) if can't find an address; these emails are <strong>public</strong> as anyone can view with a CAPTCHA)</small></label><br/> - <%= text_field object_name, 'request_email', :size => 40 %></p> + <%= t.text_field :request_email, :size => 40 %></p> <p><label for="public_body_publication_scheme">Publication scheme URL</label><br/> - <%= text_field object_name, 'publication_scheme', :size => 60 %></p> + <%= t.text_field :publication_scheme, :size => 60 %></p> <p><label for="public_body_notes">Public notes</label> <small>(HTML, for users to consider when making FOI requests to the authority)</small><br/> - <%= text_area object_name, 'notes', :rows => 3, :cols => 60 %></p> + <%= t.text_area :notes, :rows => 3, :cols => 60 %></p> </div> <% + end end %> <h3>Common Fields</h3> <p><label for="public_body_tag_string">Tags <small>(space separated; see list of tags on the right; also <strong>not_apply</strong> if FOI and EIR no longer apply to authority, <strong>eir_only</strong> if EIR but not FOI applies to authority, <strong>defunct</strong> if the authority no longer exists; charity:NUMBER if a registered charity)</small></label><br/> -<%= text_field 'public_body', 'tag_string', :size => 60 %></p> +<%= f.text_field :tag_string, :size => 60 %></p> <p><label for="public_body_home_page">Home page <small>(of whole authority, not just their FOI page; set to <strong>blank</strong> (empty string) to guess it from the email)</small></label><br/> -<%= text_field 'public_body', 'home_page', :size => 60 %></p> +<%= f.text_field :home_page, :size => 60 %></p> <p><label for="public_body_last_edit_comment"><strong>Comment</strong> for this edit</label> <small>(put URL or other source of new info)</small><br/> -<%= text_area 'public_body', 'last_edit_comment', :rows => 3, :cols => 60 %></p> +<%= f.text_area :last_edit_comment, :rows => 3, :cols => 60 %></p> <!--[eoform:public_body]--> diff --git a/app/views/admin_public_body/edit.rhtml b/app/views/admin_public_body/edit.rhtml index 005ec93ce..ae4066dc7 100644 --- a/app/views/admin_public_body/edit.rhtml +++ b/app/views/admin_public_body/edit.rhtml @@ -2,9 +2,9 @@ <h1><%=@title%></h1> -<% form_tag '../update/' + @public_body.id.to_s do %> - <%= render :partial => 'form' %> - <p><%= submit_tag 'Save', :accesskey => 's' %></p> +<% form_for @public_body, :url => {:action => 'update'} do |f| %> + <%= render :partial => 'form', :locals => {:f => f} %> + <p><%= f.submit 'Save', :accesskey => 's' %></p> <% end %> <p> diff --git a/app/views/admin_public_body/new.rhtml b/app/views/admin_public_body/new.rhtml index 95208b5b3..fe0b5b670 100644 --- a/app/views/admin_public_body/new.rhtml +++ b/app/views/admin_public_body/new.rhtml @@ -2,9 +2,9 @@ <h1><%=@title%></h1> -<% form_tag 'create' do %> - <%= render :partial => 'form' %> - <p><%= submit_tag "Create" %></p> +<% form_for :public_body, @public_body, :url => {:action => "create"} do |f| %> + <%= render :partial => 'form', :locals => {:f => f} %> + <p><%= f.submit "Create" %></p> <% end %> <p> diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 8c0980db4..53db4f412 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -113,7 +113,7 @@ describe AdminPublicBodyController, "when administering public bodies with i18n" get :edit, {:id => 3, :locale => :en} # When editing a body, the controller returns all available translations - assigns[:public_body_es].name.should == 'El Department for Humpadinking' + assigns[:public_body].translation("es").name.should == 'El Department for Humpadinking' assigns[:public_body].name.should == 'Department for Humpadinking' response.should render_template('edit') end @@ -124,8 +124,16 @@ describe AdminPublicBodyController, "when administering public bodies with i18n" pb.name.should == "El Department for Humpadinking" post :update, { :id => 3, - :public_body => { :name => "Department for Humpadinking", :short_name => "", :tag_string => "some tags", :request_email => 'edited@localhost', :last_edit_comment => 'From test code' }, - :public_body_es => { :name => "Renamed", :short_name => "", :tag_string => "some tags", :request_email => 'edited@localhost', :last_edit_comment => 'From test code' } + :public_body => { + :name => "Department for Humpadinking", + :short_name => "", + :tag_string => "some tags", + :request_email => 'edited@localhost', + :last_edit_comment => 'From test code', + :translated_versions => { + 3 => {:locale => "es", :name => "Renamed",:short_name => "", :request_email => 'edited@localhost'} + } + } } response.flash[:notice].should include('successful') end @@ -171,8 +179,10 @@ describe AdminPublicBodyController, "when creating public bodies with i18n" do it "creates a new public body with multiple locales" do PublicBody.count.should == 2 post :create, { - :public_body => { :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' }, - :public_body_es => { :name => "Nuevo Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' } + :public_body => { + :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code', + :translated_versions => [{ :locale => "es", :name => "Mi Nuevo Quango", :short_name => "", :request_email => 'newquango@localhost' }] + } } PublicBody.count.should == 3 @@ -181,25 +191,14 @@ describe AdminPublicBodyController, "when creating public bodies with i18n" do PublicBody.with_locale(:en) do body.name.should == "New Quango" body.url_name.should == "new_quango" + body.first_letter.should == "N" end PublicBody.with_locale(:es) do - body.name.should == "Nuevo Quango" - body.url_name.should == "nuevo_quango" + body.name.should == "Mi Nuevo Quango" + body.url_name.should == "mi_nuevo_quango" + body.first_letter.should == "M" end response.should redirect_to(:controller=>'admin_public_body', :action=>'show', :id=>body.id) end - - # when submitting the 'new' form, we should ignore a locale if the user hasn't set any value in it - it "doesn't create the public body if anything fails" do - PublicBody.count.should == 2 - post :create, { - :public_body => { :name => "New Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' }, - :public_body_fr => { :name => "Neuf Quango", :short_name => "", :tag_string => "blah", :request_email => 'newquango@localhost', :last_edit_comment => 'From test code' }, - :public_body_es => { :name => "" }, # invalid - } - response.should render_template('new') - PublicBody.count.should == 2 - I18n.locale.should == :en # don't mess up the previous locale - end end |