diff options
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 10 | ||||
| -rw-r--r-- | app/models/public_body.rb | 19 | ||||
| -rw-r--r-- | app/views/admin_public_body/_form.html.erb | 77 | ||||
| -rw-r--r-- | app/views/admin_public_body/_locale_fields.html.erb | 49 | ||||
| -rw-r--r-- | config/test.yml | 2 | ||||
| -rw-r--r-- | spec/controllers/admin_public_body_controller_spec.rb | 154 | ||||
| -rw-r--r-- | spec/integration/admin_public_body_edit_spec.rb | 71 | ||||
| -rw-r--r-- | spec/models/public_body_spec.rb | 102 |
8 files changed, 410 insertions, 74 deletions
diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index f7a80476c..baa5a1d22 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -83,6 +83,11 @@ class AdminPublicBodyController < AdminController def new @public_body = PublicBody.new + + I18n.available_locales.each do |locale| + @public_body.translations.build(:locale => locale) + end + if params[:change_request_id] @change_request = PublicBodyChangeRequest.find(params[:change_request_id]) end @@ -120,6 +125,11 @@ class AdminPublicBodyController < AdminController def edit @public_body = PublicBody.find(params[:id]) + + I18n.available_locales.each do |locale| + @public_body.translations.find_or_initialize_by_locale(locale) + end + if params[:change_request_id] @change_request = PublicBodyChangeRequest.find(params[:change_request_id]) end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index a9cdfeab2..829625cac 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -64,6 +64,7 @@ class PublicBody < ActiveRecord::Base } translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme + accepts_nested_attributes_for :translations # Default fields available for importing from CSV, in the format # [field_name, 'short description of field (basic html allowed)'] @@ -151,12 +152,11 @@ class PublicBody < ActiveRecord::Base translations end - def translated_versions=(translation_attrs) + def translations_attributes=(translation_attrs) def empty_translation?(attrs) - attrs_with_values = attrs.select{ |key, value| value != '' and key != 'locale' } + attrs_with_values = attrs.select{ |key, value| value != '' and key.to_s != 'locale' } attrs_with_values.empty? end - if translation_attrs.respond_to? :each_value # Hash => updating translation_attrs.each_value do |attrs| next if empty_translation?(attrs) @@ -166,6 +166,13 @@ class PublicBody < ActiveRecord::Base t.save! end else # Array => creating + warn "[DEPRECATION] PublicBody#translations_attributes= " \ + "will no longer accept an Array as of release 0.22. " \ + "Use Hash arguments instead. See " \ + "spec/models/public_body_spec.rb and " \ + "app/views/admin_public_body/_form.html.erb for more " \ + "details." + translation_attrs.each do |attrs| next if empty_translation?(attrs) new_translation = PublicBody::Translation.new(attrs) @@ -175,6 +182,12 @@ class PublicBody < ActiveRecord::Base end end + def translated_versions=(translation_attrs) + warn "[DEPRECATION] PublicBody#translated_versions= will be replaced " \ + "by PublicBody#translations_attributes= as of release 0.22" + self.translations_attributes = translation_attrs + end + def set_default_publication_scheme # Make sure publication_scheme gets the correct default value. # (This would work automatically, were publication_scheme not a translated attribute) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 2da13ab01..c765c116e 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -4,66 +4,27 @@ <div id="div-locales"> <ul class="locales nav nav-tabs"> - <% I18n.available_locales.each_with_index do |locale, i| %> - <li><a href="#div-locale-<%=locale.to_s%>" data-toggle="tab" ><%=locale_name(locale.to_s) || _("Default locale")%></a></li> - <% end %> + <% @public_body.translations.each do |translation| %> + <li> + <a href="#div-locale-<%= translation.locale.to_s %>" data-toggle="tab"> + <%= locale_name(translation.locale.to_s) || _("Default locale") %> + </a> + </li> + <% end %> </ul> + <div class="tab-content"> -<% - for locale in I18n.available_locales do - 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.find_translation_by_locale(locale.to_s) || PublicBody::Translation.new - end -%> - <%= fields_for prefix, object do |t| %> - <div class="tab-pane" id="div-locale-<%=locale.to_s%>"> - <div class="control-group"> - <%= t.hidden_field :locale, :value => locale.to_s %> - <label for="<%= form_tag_id(t.object_name, :name, locale) %>" class="control-label">Name</label> - <div class="controls"> - <%= t.text_field :name, :id => form_tag_id(t.object_name, :name, locale), :class => "span4" %> - </div> - </div> - <div class="control-group"> - <label for="<%= form_tag_id(t.object_name, :short_name, locale) %>", class="control-label"><%=_("Short name")%></label> - <div class="controls"> - <%= t.text_field :short_name, :id => form_tag_id(t.object_name, :short_name, locale), :class => "span2" %> - <p class="help-block"><%=_("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")%></p> - </div> - </div> - <div class="control-group"> - <label for="<%= form_tag_id(t.object_name, :request_email, locale) %>" class="control-label"><%=_("Request email")%></label> - <div class="controls"> - <%= t.text_field :request_email, :id => form_tag_id(t.object_name, :request_email, locale), :class => "span3" %> - <p class="help-block"><%=_("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")%></p> - </div> - </div> - <div class="control-group"> - <label for="<%= form_tag_id(t.object_name, :publication_scheme, locale) %>" class="control-label"><%=_("Publication scheme URL")%></label> - <div class="controls"> - <%= t.text_field :publication_scheme, :size => 60, :id => form_tag_id(t.object_name, :publication_scheme, locale), :class => "span3" %> - </div> - </div> - <div class="control-group"> - <label for="<%= form_tag_id(t.object_name, :notes, locale) %>" class="control-label"><%=_("Public notes")%></label> - <div class="controls"> - <%= t.text_area :notes, :rows => 3, :id => form_tag_id(t.object_name, :notes, locale), :class => "span6" %> - <p class="help-block"> - HTML, for users to consider when making FOI requests to the authority - </p> - </div> - </div> - </div> -<% - end - end -%> + <% @public_body.translations.each do |translation| %> + <% if translation.locale.to_s == I18n.default_locale.to_s %> + <%= fields_for('public_body', @public_body) do |t| %> + <%= render :partial => 'locale_fields', :locals => { :t => t, :locale => translation.locale } %> + <% end %> + <% else %> + <%= f.fields_for(:translations, translation, :child_index => translation.locale) do |t| %> + <%= render :partial => 'locale_fields' , :locals => { :t => t, :locale => translation.locale } %> + <% end %> + <% end %> + <% end %> </div> </div> diff --git a/app/views/admin_public_body/_locale_fields.html.erb b/app/views/admin_public_body/_locale_fields.html.erb new file mode 100644 index 000000000..cbe008fe2 --- /dev/null +++ b/app/views/admin_public_body/_locale_fields.html.erb @@ -0,0 +1,49 @@ + +<div class="tab-pane" id="div-locale-<%=locale.to_s%>"> + <div class="control-group"> + <%= t.hidden_field :locale, :value => locale.to_s %> + <label for="<%= form_tag_id(t.object_name, :name, locale) %>" class="control-label">Name</label> + <div class="controls"> + <%= t.text_field :name, :id => form_tag_id(t.object_name, :name, locale), :class => "span4" %> + </div> + </div> + + <div class="control-group"> + <label for="<%= form_tag_id(t.object_name, :short_name, locale) %>" class="control-label"><%=_("Short name")%></label> + <div class="controls"> + <%= t.text_field :short_name, :id => form_tag_id(t.object_name, :short_name, locale), :class => "span2" %> + <p class="help-block"> + <%=_("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")%> + </p> + </div> + </div> + + <div class="control-group"> + <label for="<%= form_tag_id(t.object_name, :request_email, locale) %>" class="control-label"><%=_("Request email")%></label> + <div class="controls"> + <%= t.text_field :request_email, :id => form_tag_id(t.object_name, :request_email, locale), :class => "span3" %> + <p class="help-block"> + <%=_("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")%> + </p> + </div> + </div> + + <div class="control-group"> + <label for="<%= form_tag_id(t.object_name, :publication_scheme, locale) %>" class="control-label"><%=_("Publication scheme URL")%></label> + <div class="controls"> + <%= t.text_field :publication_scheme, :size => 60, :id => form_tag_id(t.object_name, :publication_scheme, locale), :class => "span3" %> + </div> + </div> + + <div class="control-group"> + <label for="<%= form_tag_id(t.object_name, :notes, locale) %>" class="control-label"><%=_("Public notes")%></label> + <div class="controls"> + <%= t.text_area :notes, :rows => 3, :id => form_tag_id(t.object_name, :notes, locale), :class => "span6" %> + <p class="help-block"> + HTML, for users to consider when making FOI requests to the authority + </p> + </div> + </div> + +</div> + diff --git a/config/test.yml b/config/test.yml index 599e1e81a..4a5de704f 100644 --- a/config/test.yml +++ b/config/test.yml @@ -27,7 +27,7 @@ BLOG_FEED: 'http://www.mysociety.org/category/projects/whatdotheyknow/feed/' TWITTER_USERNAME: 'alaveteli_foi' # Locales we wish to support in this app, space-delimited -AVAILABLE_LOCALES: 'en es en_GB' +AVAILABLE_LOCALES: 'en es fr en_GB' DEFAULT_LOCALE: 'en' # if 'true', respect the user's choice of language in the browser diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 095d23245..f176150da 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -44,6 +44,15 @@ describe AdminPublicBodyController, 'when showing the form for a new public body assigns[:public_body].should be_a(PublicBody) end + it "builds new translations for all locales" do + get :new + + translations = assigns[:public_body].translations.map{ |t| t.locale.to_s }.sort + available = I18n.available_locales.map{ |l| l.to_s }.sort + + expect(translations).to eq(available) + end + context 'when passed a change request id as a param' do render_views @@ -88,11 +97,13 @@ describe AdminPublicBodyController, "when creating a public body" do :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' }] - } + :translations_attributes => { + 'es' => { :locale => "es", + :name => "Mi Nuevo Quango", + :short_name => "", + :request_email => 'newquango@localhost' } + } + } } PublicBody.count.should == n + 1 @@ -159,6 +170,12 @@ describe AdminPublicBodyController, "when editing a public body" do response.should render_template('edit') end + it "builds new translations if the body does not already have a translation in the specified locale" do + public_body = FactoryGirl.create(:public_body) + get :edit, :id => public_body.id + expect(assigns[:public_body].translations.map(&:locale)).to include(:fr) + end + context 'when passed a change request id as a param' do render_views @@ -196,6 +213,116 @@ describe AdminPublicBodyController, "when updating a public body" do pb.name.should == "Renamed" end + it 'adds a new translation' do + pb = public_bodies(:humpadink_public_body) + pb.translation_for(:es).destroy + pb.reload + + post :update, { + :id => pb.id, + :public_body => { + :name => "Department for Humpadinking", + :short_name => "", + :tag_string => "some tags", + :request_email => 'edited@localhost', + :last_edit_comment => 'From test code', + :translations_attributes => { + 'es' => { :locale => "es", + :name => "El Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + } + } + } + + request.flash[:notice].should include('successful') + + pb = PublicBody.find(public_bodies(:humpadink_public_body).id) + + I18n.with_locale(:es) do + expect(pb.name).to eq('El Department for Humpadinking') + end + end + + it 'adds new translations' do + pb = public_bodies(:humpadink_public_body) + pb.translation_for(:es).destroy + pb.reload + + post :update, { + :id => pb.id, + :public_body => { + :name => "Department for Humpadinking", + :short_name => "", + :tag_string => "some tags", + :request_email => 'edited@localhost', + :last_edit_comment => 'From test code', + :translations_attributes => { + 'es' => { :locale => "es", + :name => "El Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' }, + 'fr' => { :locale => "fr", + :name => "Le Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + } + } + } + + request.flash[:notice].should include('successful') + + pb = PublicBody.find(public_bodies(:humpadink_public_body).id) + + I18n.with_locale(:es) do + expect(pb.name).to eq('El Department for Humpadinking') + end + + I18n.with_locale(:fr) do + expect(pb.name).to eq('Le Department for Humpadinking') + end + end + + it 'updates an existing translation and adds a third translation' do + pb = public_bodies(:humpadink_public_body) + + put :update, { + :id => pb.id, + :public_body => { + :name => "Department for Humpadinking", + :short_name => "", + :tag_string => "some tags", + :request_email => 'edited@localhost', + :last_edit_comment => 'From test code', + :translations_attributes => { + # Update existing translation + 'es' => { :locale => "es", + :name => "Renamed Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' }, + # Add new translation + 'fr' => { :locale => "fr", + :name => "Le Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + } + } + } + + request.flash[:notice].should include('successful') + + pb = PublicBody.find(public_bodies(:humpadink_public_body).id) + + I18n.with_locale(:es) do + expect(pb.name).to eq('Renamed Department for Humpadinking') + end + + I18n.with_locale(:fr) do + expect(pb.name).to eq('Le Department for Humpadinking') + end + + end + it "saves edits to a public body in another locale" do I18n.with_locale(:es) do pb = PublicBody.find(id=3) @@ -208,11 +335,11 @@ describe AdminPublicBodyController, "when updating a public body" do :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'} + :translations_attributes => { + 'es' => { :locale => "es", + :name => "Renamed", + :short_name => "", + :request_email => 'edited@localhost' } } } } @@ -220,12 +347,15 @@ describe AdminPublicBodyController, "when updating a public body" do end pb = PublicBody.find(public_bodies(:humpadink_public_body).id) + I18n.with_locale(:es) do - pb.name.should == "Renamed" + expect(pb.name).to eq('Renamed') end + I18n.with_locale(:en) do - pb.name.should == "Department for Humpadinking" + expect(pb.name).to eq('Department for Humpadinking') end + end context 'when the body is being updated as a result of a change request' do diff --git a/spec/integration/admin_public_body_edit_spec.rb b/spec/integration/admin_public_body_edit_spec.rb new file mode 100644 index 000000000..613793dd4 --- /dev/null +++ b/spec/integration/admin_public_body_edit_spec.rb @@ -0,0 +1,71 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +require File.expand_path(File.dirname(__FILE__) + '/alaveteli_dsl') + +describe 'Editing a Public Body' do + before do + AlaveteliConfiguration.stub!(:skip_admin_auth).and_return(false) + + confirm(:admin_user) + @admin = login(:admin_user) + + PublicBody.create(:name => 'New Quango', + :short_name => '', + :request_email => 'newquango@localhost', + :last_edit_editor => 'test', + :last_edit_comment => 'testing') + + @body = PublicBody.find_by_name('New Quango') + end + + it 'can edit the default locale' do + @admin.visit admin_body_edit_path(@body) + @admin.fill_in 'public_body_name__en', :with => 'New Quango EN' + @admin.click_button 'Save' + + pb = @body.reload + expect(pb.name).to eq('New Quango EN') + end + + it 'can add a translation for a single locale' do + expect(@body.find_translation_by_locale('fr')).to be_nil + + @admin.visit admin_body_edit_path(@body) + @admin.fill_in 'public_body_translations_attributes_fr_name__fr', :with => 'New Quango FR' + @admin.click_button 'Save' + + pb = @body.reload + I18n.with_locale(:fr) do + expect(pb.name).to eq('New Quango FR') + end + end + + it 'can add a translation for multiple locales', :focus => true do + @admin.visit admin_body_edit_path(@body) + @admin.fill_in 'public_body_name__en', :with => 'New Quango EN' + @admin.click_button 'Save' + + # Add FR translation + expect(@body.find_translation_by_locale('fr')).to be_nil + @admin.visit admin_body_edit_path(@body) + @admin.fill_in 'public_body_translations_attributes_fr_name__fr', :with => 'New Quango FR' + @admin.click_button 'Save' + + # Add ES translation + expect(@body.find_translation_by_locale('es')).to be_nil + @admin.visit admin_body_edit_path(@body) + @admin.fill_in 'public_body_translations_attributes_es_name__es', :with => 'New Quango ES' + @admin.click_button 'Save' + + pb = @body.reload + + expect(pb.name).to eq('New Quango EN') + + I18n.with_locale(:fr) do + expect(pb.name).to eq('New Quango FR') + end + + I18n.with_locale(:es) do + expect(pb.name).to eq('New Quango ES') + end + end +end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 225958cac..f12582f21 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -28,6 +28,108 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +describe PublicBody do + + describe :translations_attributes= do + + context 'translation_attrs is a Hash' do + + it 'takes the correct code path for a Hash' do + attrs = {} + attrs.should_receive(:each_value) + PublicBody.new().translations_attributes = attrs + end + + it 'updates an existing translation' do + body = public_bodies(:geraldine_public_body) + translation = body.translation_for(:es) + params = { 'es' => { :locale => 'es', + :name => 'Renamed' } } + + body.translations_attributes = params + I18n.with_locale(:es) { expect(body.name).to eq('Renamed') } + end + + it 'updates an existing translation and creates a new translation' do + body = public_bodies(:geraldine_public_body) + translation = body.translation_for(:es) + + expect(body.translations.size).to eq(2) + + body.translations_attributes = { + 'es' => { :locale => 'es', + :name => 'Renamed' }, + 'fr' => { :locale => 'fr', + :name => 'Le Geraldine Quango' } + } + + expect(body.translations.size).to eq(3) + I18n.with_locale(:es) { expect(body.name).to eq('Renamed') } + I18n.with_locale(:fr) { expect(body.name).to eq('Le Geraldine Quango') } + end + + it 'skips empty translations' do + body = public_bodies(:geraldine_public_body) + translation = body.translation_for(:es) + + expect(body.translations.size).to eq(2) + + body.translations_attributes = { + 'es' => { :locale => 'es', + :name => 'Renamed' }, + 'fr' => { :locale => 'fr' } + } + + expect(body.translations.size).to eq(2) + end + + end + + context 'translation_attrs is an Array' do + + it 'takes the correct code path for an Array' do + attrs = [] + attrs.should_receive(:each) + PublicBody.new().translations_attributes = attrs + end + + it 'creates a new translation' do + body = public_bodies(:geraldine_public_body) + body.translation_for(:es).destroy + body.reload + + expect(body.translations.size).to eq(1) + + body.translations_attributes = [ { + :locale => 'es', + :name => 'Renamed' + } + ] + + expect(body.translations.size).to eq(2) + I18n.with_locale(:es) { expect(body.name).to eq('Renamed') } + end + + it 'skips empty translations' do + body = public_bodies(:geraldine_public_body) + body.translation_for(:es).destroy + body.reload + + expect(body.translations.size).to eq(1) + + body.translations_attributes = [ + { :locale => 'empty' } + ] + + expect(body.translations.size).to eq(1) + end + + end + + end + +end + describe PublicBody, " using tags" do before do @public_body = PublicBody.new(:name => 'Aardvark Monitoring Service', |