From 5b776d1daf8f6bc42f95dbfc86c20409e03ba4a2 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Sat, 17 Jan 2015 21:43:25 +0000 Subject: Add an extra locale to the test environment config --- config/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 -- cgit v1.2.3 From 4519fd2b3e3e899f785810a7f7c9536c7dd90409 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Sat, 17 Jan 2015 21:43:58 +0000 Subject: Test adding multiple translations to Public Bodies --- spec/integration/admin_public_body_edit_spec.rb | 56 +++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 spec/integration/admin_public_body_edit_spec.rb 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..83174fa80 --- /dev/null +++ b/spec/integration/admin_public_body_edit_spec.rb @@ -0,0 +1,56 @@ +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 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_translated_versions__name__fr', :with => 'New Quango FR' + @admin.click_button 'Save' + + pb = PublicBody.find_by_name('New Quango') + I18n.with_locale(:fr) do + expect(pb.name).to eq('New Quango FR') + end + end + + it 'can add a translation for multiple locales' do + # 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_translated_versions__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_translated_versions__name__es', :with => 'New Quango ES' + @admin.click_button 'Save' + + pb = PublicBody.find_by_name('New Quango') + + 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 -- cgit v1.2.3 From 8c48ad992b0285fa8da81c6b673635757d2c8ef9 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 19 Jan 2015 10:35:47 +0000 Subject: Rewrite complex ternary logic --- app/views/admin_public_body/_form.html.erb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 2da13ab01..24bb03ad6 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -16,9 +16,12 @@ 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 + object = if @public_body.new_record? + PublicBody::Translation.new + else + @public_body.find_translation_by_locale(locale.to_s) + end + object ||= PublicBody::Translation.new end %> <%= fields_for prefix, object do |t| %> -- cgit v1.2.3 From 2dc3dc7ede4b1a0468849317975de5a4a2083d4e Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 19 Jan 2015 10:39:57 +0000 Subject: each instead of for --- app/views/admin_public_body/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 24bb03ad6..623922b08 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -10,7 +10,7 @@
<% - for locale in I18n.available_locales do + I18n.available_locales.each do |locale| if locale==I18n.default_locale # The default locale is submitted as part of the bigger object... prefix = 'public_body' object = @public_body -- cgit v1.2.3 From b23c9e8ba7fd87c7d41aac785fb5c132031b4d78 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 19 Jan 2015 11:34:36 +0000 Subject: Extract view logic --- app/helpers/admin_public_body_helper.rb | 22 ++++++++ app/views/admin_public_body/_form.html.erb | 26 ++------- spec/helpers/admin_public_body_helper_spec.rb | 79 +++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 app/helpers/admin_public_body_helper.rb create mode 100644 spec/helpers/admin_public_body_helper_spec.rb diff --git a/app/helpers/admin_public_body_helper.rb b/app/helpers/admin_public_body_helper.rb new file mode 100644 index 000000000..5139bd49f --- /dev/null +++ b/app/helpers/admin_public_body_helper.rb @@ -0,0 +1,22 @@ +module AdminPublicBodyHelper + + def public_body_form_object(public_body, locale) + 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 = if public_body.new_record? + PublicBody::Translation.new + else + public_body.find_translation_by_locale(locale.to_s) + end + object ||= PublicBody::Translation.new + end + + { :object => object, :prefix => prefix } + end + +end diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 623922b08..6b573ac9e 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -9,22 +9,10 @@ <% end %>
-<% - I18n.available_locales.each do |locale| - 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 = if @public_body.new_record? - PublicBody::Translation.new - else - @public_body.find_translation_by_locale(locale.to_s) - end - object ||= PublicBody::Translation.new - end -%> - <%= fields_for prefix, object do |t| %> +<% I18n.available_locales.each do |locale| %> + <% context = public_body_form_object(@public_body, locale) %> + + <%= fields_for context[:prefix], context[:object] do |t| %>
<%= t.hidden_field :locale, :value => locale.to_s %> @@ -63,10 +51,8 @@
-<% - end - end -%> + <% end %> +<% end %>
diff --git a/spec/helpers/admin_public_body_helper_spec.rb b/spec/helpers/admin_public_body_helper_spec.rb new file mode 100644 index 000000000..defb1b359 --- /dev/null +++ b/spec/helpers/admin_public_body_helper_spec.rb @@ -0,0 +1,79 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe AdminPublicBodyHelper do + + include AdminPublicBodyHelper + + describe :public_body_form_object do + + context 'in the default locale' do + + before(:each) do + @locale = I18n.default_locale + @public_body = Factory.create(:public_body) + end + + it 'provides the original object' do + object = public_body_form_object(@public_body, @locale)[:object] + expect(object).to eq(@public_body) + end + + it 'provides the prefix public_body' do + prefix = public_body_form_object(@public_body, @locale)[:prefix] + expect(prefix).to eq('public_body') + end + + end + + context 'in an alternative locale' do + + it 'provides the prefix public_body[translated_versions][]' do + public_body = FactoryGirl.build(:public_body) + locale = :es + prefix = public_body_form_object(public_body, locale)[:prefix] + expect(prefix).to eq('public_body[translated_versions][]') + end + + context 'when the PublicBody is new' do + + it 'builds a new PublicBody::Translation' do + public_body = FactoryGirl.build(:public_body) + locale = :es + + object = public_body_form_object(public_body, locale)[:object] + + expect(object).to be_instance_of(PublicBody::Translation) + expect(object).to be_new_record + end + + end + + context 'when the PublicBody has been persisted' do + + it 'finds an existing PublicBody::Translation for the locale' do + public_body = public_bodies(:geraldine_public_body) + locale = :es + translation = public_body.find_translation_by_locale(locale) + + object = public_body_form_object(public_body, locale)[:object] + + expect(object).to eq(translation) + end + + it 'builds a new PublicBody::Translation if the record does not have one for that locale' do + public_body = FactoryGirl.create(:public_body) + locale = :es + + object = public_body_form_object(public_body, locale)[:object] + + expect(object).to be_instance_of(PublicBody::Translation) + expect(object).to be_new_record + end + + end + + end + + end + +end -- cgit v1.2.3 From a3dc8ee09b4b0a3661e623ff2665c005fbe8405e Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 19 Jan 2015 16:29:14 +0000 Subject: Add specs for PublicBody#translated_versions= Also fixes #empty_translation? to check for String and Symbol keys named 'locale'. Specs use a pre-change check to assert difference. For some reason rspec change matcher fails with 'nil is not a symbol'. --- app/models/public_body.rb | 2 +- spec/models/public_body_spec.rb | 110 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/app/models/public_body.rb b/app/models/public_body.rb index a9cdfeab2..a434cce35 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -153,7 +153,7 @@ class PublicBody < ActiveRecord::Base def translated_versions=(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 diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 225958cac..bdfaab6bf 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -28,6 +28,116 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +describe PublicBody do + + describe :translated_versions= 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().translated_versions = attrs + end + + it 'updates an existing translation' do + body = public_bodies(:geraldine_public_body) + translation = body.translation_for(:es) + params = { translation.id.to_sym => { :locale => 'es', + :name => 'Renamed' } } + + body.translated_versions = 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.translated_versions = { + translation.id.to_sym => { + :locale => 'es', + :name => 'Renamed' + }, + :new_translation => { + :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.translated_versions = { + translation.id.to_sym => { + :locale => 'es', + :name => 'Renamed' + }, + :empty_translation => { + :locale => 'es' + } + } + + 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().translated_versions = 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.translated_versions = [ { + :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.translated_versions = [ + { :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', -- cgit v1.2.3 From 9f2db30774d159eb143497584de65f0d7a68ee40 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 20 Jan 2015 12:03:34 +0000 Subject: Add AdminPublicBodyController#update specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers core functionality. Could do with extracting multiple assertions in to individual specs. Strange that you need to call `reload` on the PublicBody instance when testing “updates an existing translation and adds a third translation” --- .../admin_public_body_controller_spec.rb | 106 +++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 095d23245..8aebbc9ed 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -196,6 +196,111 @@ 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 => [ + { :locale => "es", + :name => "El Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + ] + } + } + + request.flash[:notice].should include('successful') + + 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 => [ + { :locale => "es", + :name => "El Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' }, + { :locale => "fr", + :name => "Le Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + ] + } + } + + request.flash[:notice].should include('successful') + + 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) + + 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 => [ + # Update existing translation + { :locale => "es", + :name => "Renamed Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' }, + # Add new translation + { :locale => "fr", + :name => "Le Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + ] + } + } + + pb.reload + + request.flash[:notice].should include('successful') + + 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) @@ -226,6 +331,7 @@ describe AdminPublicBodyController, "when updating a public body" do I18n.with_locale(:en) do pb.name.should == "Department for Humpadinking" end + end context 'when the body is being updated as a result of a change request' do -- cgit v1.2.3 From 7c7b008a0f2c6937b6bf02ab26134bb90aae19ee Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 20 Jan 2015 12:26:30 +0000 Subject: Fix submission of form containing both existing and new translations --- app/helpers/admin_public_body_helper.rb | 4 +- app/models/public_body.rb | 4 +- app/views/admin_public_body/_form.html.erb | 55 +++---------- .../admin_public_body/_locale_fields.html.erb | 49 ++++++++++++ .../admin_public_body_controller_spec.rb | 91 ++++++++++++---------- spec/helpers/admin_public_body_helper_spec.rb | 4 +- spec/integration/admin_public_body_edit_spec.rb | 12 +-- spec/models/public_body_spec.rb | 42 ++++------ 8 files changed, 140 insertions(+), 121 deletions(-) create mode 100644 app/views/admin_public_body/_locale_fields.html.erb diff --git a/app/helpers/admin_public_body_helper.rb b/app/helpers/admin_public_body_helper.rb index 5139bd49f..97f656ddb 100644 --- a/app/helpers/admin_public_body_helper.rb +++ b/app/helpers/admin_public_body_helper.rb @@ -7,11 +7,11 @@ module AdminPublicBodyHelper object = public_body else # ...but additional locales go "on the side" - prefix = 'public_body[translated_versions][]' + prefix = :translations object = if public_body.new_record? PublicBody::Translation.new else - public_body.find_translation_by_locale(locale.to_s) + public_body.find_translation_by_locale(locale.to_s) end object ||= PublicBody::Translation.new end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index a434cce35..d3544e13c 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.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) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 6b573ac9e..177873984 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -9,53 +9,22 @@ <% end %>
-<% I18n.available_locales.each do |locale| %> - <% context = public_body_form_object(@public_body, locale) %> - - <%= fields_for context[:prefix], context[:object] do |t| %> -
-
- <%= t.hidden_field :locale, :value => locale.to_s %> - -
- <%= t.text_field :name, :id => form_tag_id(t.object_name, :name, locale), :class => "span4" %> -
-
-
- -
- <%= t.text_field :short_name, :id => form_tag_id(t.object_name, :short_name, locale), :class => "span2" %> -

<%=_("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")%>

-
-
-
- -
- <%= t.text_field :request_email, :id => form_tag_id(t.object_name, :request_email, locale), :class => "span3" %> -

<%=_("set to blank (empty string) if can't find an address; these emails are public as anyone can view with a CAPTCHA")%>

-
-
-
- -
- <%= t.text_field :publication_scheme, :size => 60, :id => form_tag_id(t.object_name, :publication_scheme, locale), :class => "span3" %> -
-
-
- -
- <%= t.text_area :notes, :rows => 3, :id => form_tag_id(t.object_name, :notes, locale), :class => "span6" %> -

- HTML, for users to consider when making FOI requests to the authority -

-
-
-
+ <% I18n.available_locales.each do |locale| %> + <% context = public_body_form_object(@public_body, locale) %> + <% if locale.to_s == I18n.default_locale.to_s %> + <%= fields_for('public_body', context[:object]) do |t| %> + <%= render :partial => 'locale_fields', :locals => {:t => t, :locale => locale}%> + <% end %> + <% else %> + <%= f.fields_for(context[:prefix], context[:object], :child_index => locale) do |t| %> + <%= render :partial => 'locale_fields' , :locals => {:t => t, :locale => locale}%> + <% end %> + <% end %> <% end %> -<% end %>
+

Common Fields

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 @@ + +
+
+ <%= t.hidden_field :locale, :value => locale.to_s %> + +
+ <%= t.text_field :name, :id => form_tag_id(t.object_name, :name, locale), :class => "span4" %> +
+
+ +
+ +
+ <%= t.text_field :short_name, :id => form_tag_id(t.object_name, :short_name, locale), :class => "span2" %> +

+ <%=_("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")%> +

+
+
+ +
+ +
+ <%= t.text_field :request_email, :id => form_tag_id(t.object_name, :request_email, locale), :class => "span3" %> +

+ <%=_("set to blank (empty string) if can't find an address; these emails are public as anyone can view with a CAPTCHA")%> +

+
+
+ +
+ +
+ <%= t.text_field :publication_scheme, :size => 60, :id => form_tag_id(t.object_name, :publication_scheme, locale), :class => "span3" %> +
+
+ +
+ +
+ <%= t.text_area :notes, :rows => 3, :id => form_tag_id(t.object_name, :notes, locale), :class => "span6" %> +

+ HTML, for users to consider when making FOI requests to the authority +

+
+
+ +
+ diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 8aebbc9ed..5b8ed6c55 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -88,11 +88,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 @@ -209,17 +211,19 @@ describe AdminPublicBodyController, "when updating a public body" do :tag_string => "some tags", :request_email => 'edited@localhost', :last_edit_comment => 'From test code', - :translations_attributes => [ - { :locale => "es", - :name => "El Department for Humpadinking", - :short_name => "", - :request_email => 'edited@localhost' } - ] + :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 @@ -238,21 +242,23 @@ describe AdminPublicBodyController, "when updating a public body" do :tag_string => "some tags", :request_email => 'edited@localhost', :last_edit_comment => 'From test code', - :translations_attributes => [ - { :locale => "es", - :name => "El Department for Humpadinking", - :short_name => "", - :request_email => 'edited@localhost' }, - { :locale => "fr", - :name => "Le Department for Humpadinking", - :short_name => "", - :request_email => 'edited@localhost' } - ] + :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 @@ -265,7 +271,7 @@ describe AdminPublicBodyController, "when updating a public body" do it 'updates an existing translation and adds a third translation' do pb = public_bodies(:humpadink_public_body) - post :update, { + put :update, { :id => pb.id, :public_body => { :name => "Department for Humpadinking", @@ -273,25 +279,25 @@ describe AdminPublicBodyController, "when updating a public body" do :tag_string => "some tags", :request_email => 'edited@localhost', :last_edit_comment => 'From test code', - :translations_attributes => [ + :translations_attributes => { # Update existing translation - { :locale => "es", - :name => "Renamed Department for Humpadinking", - :short_name => "", - :request_email => 'edited@localhost' }, + 'es' => { :locale => "es", + :name => "Renamed Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' }, # Add new translation - { :locale => "fr", - :name => "Le Department for Humpadinking", - :short_name => "", - :request_email => 'edited@localhost' } - ] + 'fr' => { :locale => "fr", + :name => "Le Department for Humpadinking", + :short_name => "", + :request_email => 'edited@localhost' } + } } } - pb.reload - 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 @@ -299,6 +305,7 @@ describe AdminPublicBodyController, "when updating a public body" do 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 @@ -313,11 +320,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' } } } } @@ -325,11 +332,13 @@ 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 diff --git a/spec/helpers/admin_public_body_helper_spec.rb b/spec/helpers/admin_public_body_helper_spec.rb index defb1b359..c135ef348 100644 --- a/spec/helpers/admin_public_body_helper_spec.rb +++ b/spec/helpers/admin_public_body_helper_spec.rb @@ -27,11 +27,11 @@ describe AdminPublicBodyHelper do context 'in an alternative locale' do - it 'provides the prefix public_body[translated_versions][]' do + it 'provides the prefix :translations' do public_body = FactoryGirl.build(:public_body) locale = :es prefix = public_body_form_object(public_body, locale)[:prefix] - expect(prefix).to eq('public_body[translated_versions][]') + expect(prefix).to eq(:translations) end context 'when the PublicBody is new' do diff --git a/spec/integration/admin_public_body_edit_spec.rb b/spec/integration/admin_public_body_edit_spec.rb index 83174fa80..d75ded6fe 100644 --- a/spec/integration/admin_public_body_edit_spec.rb +++ b/spec/integration/admin_public_body_edit_spec.rb @@ -19,9 +19,9 @@ describe 'Editing a Public Body' do 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_translated_versions__name__fr', :with => 'New Quango FR' + @admin.fill_in 'public_body_translations_attributes_fr_name__fr', :with => 'New Quango FR' @admin.click_button 'Save' pb = PublicBody.find_by_name('New Quango') @@ -30,17 +30,17 @@ describe 'Editing a Public Body' do end end - it 'can add a translation for multiple locales' do + it 'can add a translation for multiple locales', :focus => true do # 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_translated_versions__name__fr', :with => 'New Quango FR' + @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_translated_versions__name__es', :with => 'New Quango ES' + @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 = PublicBody.find_by_name('New Quango') diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index bdfaab6bf..f12582f21 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -30,23 +30,23 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PublicBody do - describe :translated_versions= 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().translated_versions = attrs + PublicBody.new().translations_attributes = attrs end it 'updates an existing translation' do body = public_bodies(:geraldine_public_body) translation = body.translation_for(:es) - params = { translation.id.to_sym => { :locale => 'es', - :name => 'Renamed' } } + params = { 'es' => { :locale => 'es', + :name => 'Renamed' } } - body.translated_versions = params + body.translations_attributes = params I18n.with_locale(:es) { expect(body.name).to eq('Renamed') } end @@ -56,15 +56,11 @@ describe PublicBody do expect(body.translations.size).to eq(2) - body.translated_versions = { - translation.id.to_sym => { - :locale => 'es', - :name => 'Renamed' - }, - :new_translation => { - :locale => 'fr', - :name => 'Le Geraldine Quango' - } + body.translations_attributes = { + 'es' => { :locale => 'es', + :name => 'Renamed' }, + 'fr' => { :locale => 'fr', + :name => 'Le Geraldine Quango' } } expect(body.translations.size).to eq(3) @@ -78,14 +74,10 @@ describe PublicBody do expect(body.translations.size).to eq(2) - body.translated_versions = { - translation.id.to_sym => { - :locale => 'es', - :name => 'Renamed' - }, - :empty_translation => { - :locale => 'es' - } + body.translations_attributes = { + 'es' => { :locale => 'es', + :name => 'Renamed' }, + 'fr' => { :locale => 'fr' } } expect(body.translations.size).to eq(2) @@ -98,7 +90,7 @@ describe PublicBody do it 'takes the correct code path for an Array' do attrs = [] attrs.should_receive(:each) - PublicBody.new().translated_versions = attrs + PublicBody.new().translations_attributes = attrs end it 'creates a new translation' do @@ -108,7 +100,7 @@ describe PublicBody do expect(body.translations.size).to eq(1) - body.translated_versions = [ { + body.translations_attributes = [ { :locale => 'es', :name => 'Renamed' } @@ -125,7 +117,7 @@ describe PublicBody do expect(body.translations.size).to eq(1) - body.translated_versions = [ + body.translations_attributes = [ { :locale => 'empty' } ] -- cgit v1.2.3 From 40dd8b05787e665d3d5534acfb6208e236240698 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 22 Jan 2015 15:05:11 +0000 Subject: Build available locales in the controller Removes logic from views and obsoletes AdminPublicBodyHelper#public_body_form_object --- app/controllers/admin_public_body_controller.rb | 10 +++ app/helpers/admin_public_body_helper.rb | 22 ------ app/views/admin_public_body/_form.html.erb | 26 ++++--- .../admin_public_body_controller_spec.rb | 15 ++++ spec/helpers/admin_public_body_helper_spec.rb | 79 ---------------------- spec/integration/admin_public_body_edit_spec.rb | 19 +++++- 6 files changed, 57 insertions(+), 114 deletions(-) delete mode 100644 app/helpers/admin_public_body_helper.rb delete mode 100644 spec/helpers/admin_public_body_helper_spec.rb 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/helpers/admin_public_body_helper.rb b/app/helpers/admin_public_body_helper.rb deleted file mode 100644 index 97f656ddb..000000000 --- a/app/helpers/admin_public_body_helper.rb +++ /dev/null @@ -1,22 +0,0 @@ -module AdminPublicBodyHelper - - def public_body_form_object(public_body, locale) - 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 = :translations - object = if public_body.new_record? - PublicBody::Translation.new - else - public_body.find_translation_by_locale(locale.to_s) - end - object ||= PublicBody::Translation.new - end - - { :object => object, :prefix => prefix } - end - -end diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 177873984..d01421089 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -4,23 +4,27 @@
+
- <% I18n.available_locales.each do |locale| %> - <% context = public_body_form_object(@public_body, locale) %> - <% if locale.to_s == I18n.default_locale.to_s %> - <%= fields_for('public_body', context[:object]) do |t| %> - <%= render :partial => 'locale_fields', :locals => {:t => t, :locale => locale}%> + <% @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(context[:prefix], context[:object], :child_index => locale) do |t| %> - <%= render :partial => 'locale_fields' , :locals => {:t => t, :locale => locale}%> + <%= f.fields_for(:translations, translation, :child_index => translation.locale) do |t| %> + <%= render :partial => 'locale_fields' , :locals => { :t => t, :locale => translation.locale } %> <% end %> <% end %> - <% end %> + <% end %>
diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 5b8ed6c55..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 @@ -161,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 diff --git a/spec/helpers/admin_public_body_helper_spec.rb b/spec/helpers/admin_public_body_helper_spec.rb deleted file mode 100644 index c135ef348..000000000 --- a/spec/helpers/admin_public_body_helper_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') - -describe AdminPublicBodyHelper do - - include AdminPublicBodyHelper - - describe :public_body_form_object do - - context 'in the default locale' do - - before(:each) do - @locale = I18n.default_locale - @public_body = Factory.create(:public_body) - end - - it 'provides the original object' do - object = public_body_form_object(@public_body, @locale)[:object] - expect(object).to eq(@public_body) - end - - it 'provides the prefix public_body' do - prefix = public_body_form_object(@public_body, @locale)[:prefix] - expect(prefix).to eq('public_body') - end - - end - - context 'in an alternative locale' do - - it 'provides the prefix :translations' do - public_body = FactoryGirl.build(:public_body) - locale = :es - prefix = public_body_form_object(public_body, locale)[:prefix] - expect(prefix).to eq(:translations) - end - - context 'when the PublicBody is new' do - - it 'builds a new PublicBody::Translation' do - public_body = FactoryGirl.build(:public_body) - locale = :es - - object = public_body_form_object(public_body, locale)[:object] - - expect(object).to be_instance_of(PublicBody::Translation) - expect(object).to be_new_record - end - - end - - context 'when the PublicBody has been persisted' do - - it 'finds an existing PublicBody::Translation for the locale' do - public_body = public_bodies(:geraldine_public_body) - locale = :es - translation = public_body.find_translation_by_locale(locale) - - object = public_body_form_object(public_body, locale)[:object] - - expect(object).to eq(translation) - end - - it 'builds a new PublicBody::Translation if the record does not have one for that locale' do - public_body = FactoryGirl.create(:public_body) - locale = :es - - object = public_body_form_object(public_body, locale)[:object] - - expect(object).to be_instance_of(PublicBody::Translation) - expect(object).to be_new_record - end - - end - - end - - end - -end diff --git a/spec/integration/admin_public_body_edit_spec.rb b/spec/integration/admin_public_body_edit_spec.rb index d75ded6fe..613793dd4 100644 --- a/spec/integration/admin_public_body_edit_spec.rb +++ b/spec/integration/admin_public_body_edit_spec.rb @@ -17,6 +17,15 @@ describe 'Editing a Public Body' do @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 @@ -24,13 +33,17 @@ describe 'Editing a Public Body' do @admin.fill_in 'public_body_translations_attributes_fr_name__fr', :with => 'New Quango FR' @admin.click_button 'Save' - pb = PublicBody.find_by_name('New Quango') + 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) @@ -43,7 +56,9 @@ describe 'Editing a Public Body' do @admin.fill_in 'public_body_translations_attributes_es_name__es', :with => 'New Quango ES' @admin.click_button 'Save' - pb = PublicBody.find_by_name('New Quango') + pb = @body.reload + + expect(pb.name).to eq('New Quango EN') I18n.with_locale(:fr) do expect(pb.name).to eq('New Quango FR') -- cgit v1.2.3 From 13e58d96656aec11c0bfa11624480777e31a4a75 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 22 Jan 2015 15:06:03 +0000 Subject: Remove unused index --- app/views/admin_public_body/_form.html.erb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index d01421089..c765c116e 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -4,9 +4,9 @@
-

Common Fields

-- cgit v1.2.3 From dd8d6c08319b965f833c61bcc8f270dd36d5db9d Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 22 Jan 2015 15:56:41 +0000 Subject: Deprecate PublicBody#translations_attributes=(Array) Array handling will no longer be supported in release 0.22 --- app/models/public_body.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/models/public_body.rb b/app/models/public_body.rb index d3544e13c..829625cac 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -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) -- cgit v1.2.3 From 1a8f912700430d58ad0827082adb2692e4f02e88 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 5 Feb 2015 10:31:56 +0000 Subject: Bump Alaveteli version number --- config/initializers/alaveteli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index ee9ad8316..9167a0c8e 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -10,7 +10,7 @@ load "debug_helpers.rb" load "util.rb" # Application version -ALAVETELI_VERSION = '0.20.0.5' +ALAVETELI_VERSION = '0.20.0.6' # Add new inflection rules using the following format # (all these examples are active by default): -- cgit v1.2.3