From 8e947a4d6808440b314ed8bf70448e81f152c4bc Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 29 Jan 2015 15:49:34 +0000 Subject: [PATCH] Fix translating Public Body Categories Fixes submission of form containing both existing and new translations --- ...admin_public_body_categories_controller.rb | 6 + app/models/public_body_category.rb | 21 +++- .../_form.html.erb | 54 +++----- .../_locale_fields.html.erb | 15 +++ ..._public_body_categories_controller_spec.rb | 117 +++++++++++++++++- .../admin_public_body_category_edit_spec.rb | 58 +++++++++ spec/models/public_body_category_spec.rb | 60 +++++++++ 7 files changed, 287 insertions(+), 44 deletions(-) create mode 100644 app/views/admin_public_body_categories/_locale_fields.html.erb create mode 100644 spec/integration/admin_public_body_category_edit_spec.rb diff --git a/app/controllers/admin_public_body_categories_controller.rb b/app/controllers/admin_public_body_categories_controller.rb index 5e305dde37..b4907dfa47 100644 --- a/app/controllers/admin_public_body_categories_controller.rb +++ b/app/controllers/admin_public_body_categories_controller.rb @@ -7,11 +7,17 @@ def index def new @category = PublicBodyCategory.new + I18n.available_locales.each do |locale| + @category.translations.build(:locale => locale) + end render :formats => [:html] end def edit @category = PublicBodyCategory.find(params[:id]) + I18n.available_locales.each do |locale| + @category.translations.find_or_initialize_by_locale(locale) + end @tagged_public_bodies = PublicBody.find_by_tag(@category.category_tag) end diff --git a/app/models/public_body_category.rb b/app/models/public_body_category.rb index c313e57348..be853a00ae 100644 --- a/app/models/public_body_category.rb +++ b/app/models/public_body_category.rb @@ -10,12 +10,15 @@ class PublicBodyCategory < ActiveRecord::Base attr_accessible :locale, :category_tag, :title, :description, - :translated_versions, :display_order + :translated_versions, :translations_attributes, + :display_order has_many :public_body_category_links, :dependent => :destroy has_many :public_body_headings, :through => :public_body_category_links translates :title, :description + accepts_nested_attributes_for :translations + validates_uniqueness_of :category_tag, :message => 'Tag is already taken' validates_presence_of :title, :message => "Title can't be blank" validates_presence_of :category_tag, :message => "Tag can't be blank" @@ -63,9 +66,9 @@ def translated_versions 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 @@ -76,6 +79,12 @@ def empty_translation?(attrs) t.save! end else # Array => creating + warn "[DEPRECATION] PublicBodyCategory#translations_attributes= " \ + "will no longer accept an Array as of release 0.22. " \ + "Use Hash arguments instead. See " \ + "spec/models/public_body_category_spec.rb and " \ + "app/views/admin_public_body_categories/_form.html.erb for more " \ + "details." translation_attrs.each do |attrs| next if empty_translation?(attrs) new_translation = PublicBodyCategory::Translation.new(attrs) @@ -83,6 +92,12 @@ def empty_translation?(attrs) end end end + + def translated_versions=(translation_attrs) + warn "[DEPRECATION] PublicBodyCategory#translated_versions= will be replaced " \ + "by PublicBodyCategory#translations_attributes= as of release 0.22" + self.translations_attributes = translation_attrs + end end diff --git a/app/views/admin_public_body_categories/_form.html.erb b/app/views/admin_public_body_categories/_form.html.erb index 1f033ac9bb..6861795fc5 100644 --- a/app/views/admin_public_body_categories/_form.html.erb +++ b/app/views/admin_public_body_categories/_form.html.erb @@ -4,43 +4,27 @@
+
-<% - 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_category' - object = @category - else # ...but additional locales go "on the side" - prefix = "public_body_category[translated_versions][]" - object = @category.new_record? ? - PublicBodyCategory::Translation.new : - @category.find_translation_by_locale(locale.to_s) || PublicBodyCategory::Translation.new - end -%> - <%= fields_for prefix, object do |t| %> -
-
- <%= t.hidden_field :locale, :value => locale.to_s %> - -
- <%= t.text_field :title, :id => form_tag_id(t.object_name, :title, locale), :class => "span4" %> -
-
-
- -
- <%= t.text_field :description, :id => form_tag_id(t.object_name, :description, locale), :class => "span4" %> -
-
-
- <% - end -end -%> + <% @category.translations.each do |translation| %> + <% if translation.locale.to_s == I18n.default_locale.to_s %> + <%= fields_for 'public_body_category', @category 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 %>
diff --git a/app/views/admin_public_body_categories/_locale_fields.html.erb b/app/views/admin_public_body_categories/_locale_fields.html.erb new file mode 100644 index 0000000000..0d9a2f2072 --- /dev/null +++ b/app/views/admin_public_body_categories/_locale_fields.html.erb @@ -0,0 +1,15 @@ +
+
+ <%= t.hidden_field :locale, :value => locale.to_s %> + +
+ <%= t.text_field :title, :id => form_tag_id(t.object_name, :title, locale), :class => "span4" %> +
+
+
+ +
+ <%= t.text_field :description, :id => form_tag_id(t.object_name, :description, locale), :class => "span4" %> +
+
+
diff --git a/spec/controllers/admin_public_body_categories_controller_spec.rb b/spec/controllers/admin_public_body_categories_controller_spec.rb index 4c641bd757..8cb9683ba5 100644 --- a/spec/controllers/admin_public_body_categories_controller_spec.rb +++ b/spec/controllers/admin_public_body_categories_controller_spec.rb @@ -21,6 +21,15 @@ expect(response).to render_template('new') end + it "builds new translations for all locales" do + get :new + + translations = assigns[:category].translations.map{ |t| t.locale.to_s }.sort + available = I18n.available_locales.map{ |l| l.to_s }.sort + + expect(translations).to eq(available) + end + end context 'when creating a public body category' do @@ -58,14 +67,16 @@ :title => 'New Category', :category_tag => 'new_test_category', :description => 'New category for testing stuff', - :translated_versions => [{ :locale => "es", - :title => "Mi Nuevo Category" }] + :translations_attributes => { + 'es' => { :locale => "es", + :title => "Mi Nuevo Category" } + } } } PublicBodyCategory.count.should == n + 1 category = PublicBodyCategory.find_by_title("New Category") - category.translations.map {|t| t.locale.to_s}.sort.should == ["en", "es"] + #category.translations.map {|t| t.locale.to_s}.sort.should == ["en", "es"] I18n.with_locale(:en) do category.title.should == "New Category" end @@ -99,6 +110,11 @@ expect(assigns[:category]).to eq(@category) end + it "builds new translations if the body does not already have a translation in the specified locale" do + get :edit, :id => @category.id + expect(assigns[:category].translations.map(&:locale)).to include(:fr) + end + it "renders the edit template" do get :edit, :id => @category.id expect(assigns[:category]).to render_template('edit') @@ -158,9 +174,9 @@ :id => @category.id, :public_body_category => { :title => "Category", - :translated_versions => { - @category.id => {:locale => "es", - :title => "Renamed"} + :translations_attributes => { + 'es' => { :locale => "es", + :title => "Renamed" } } } } @@ -176,6 +192,95 @@ end end + it 'adds a new translation' do + @category.translation_for(:es).destroy + @category.reload + + put :update, { + :id => @category.id, + :public_body_category => { + :title => @category.title, + :description => @category.description, + :translations_attributes => { + 'es' => { :locale => "es", + :title => "Example Public Body Category ES", + :description => @category.description } + } + } + } + + request.flash[:notice].should include('successful') + + pbc = PublicBodyCategory.find(@category.id) + + I18n.with_locale(:es) do + expect(pbc.title).to eq('Example Public Body Category ES') + end + end + + it 'adds new translations' do + @category.translation_for(:es).destroy + @category.reload + + post :update, { + :id => @category.id, + :public_body_category => { + :title => @category.title, + :description => @category.description, + :translations_attributes => { + 'es' => { :locale => "es", + :title => "Example Public Body Category ES", + :description => @category.description }, + 'fr' => { :locale => "fr", + :title => "Example Public Body Category FR", + :description => @category.description } + } + } + } + + request.flash[:notice].should include('successful') + + pbc = PublicBodyCategory.find(@category.id) + + I18n.with_locale(:es) do + expect(pbc.title).to eq('Example Public Body Category ES') + end + I18n.with_locale(:fr) do + expect(pbc.title).to eq('Example Public Body Category FR') + end + end + + it 'updates an existing translation and adds a third translation' do + post :update, { + :id => @category.id, + :public_body_category => { + :title => @category.title, + :description => @category.description, + :translations_attributes => { + # Update existing translation + 'es' => { :locale => "es", + :title => "Renamed Example Public Body Category ES", + :description => @category.description }, + # Add new translation + 'fr' => { :locale => "fr", + :title => "Example Public Body Category FR", + :description => @category.description } + } + } + } + + request.flash[:notice].should include('successful') + + pbc = PublicBodyCategory.find(@category.id) + + I18n.with_locale(:es) do + expect(pbc.title).to eq('Renamed Example Public Body Category ES') + end + I18n.with_locale(:fr) do + expect(pbc.title).to eq('Example Public Body Category FR') + end + end + it "does not save edits to category_tag if the category has associated bodies" do body = FactoryGirl.create(:public_body, :tag_string => @tag) post :update, { :id => @category.id, diff --git a/spec/integration/admin_public_body_category_edit_spec.rb b/spec/integration/admin_public_body_category_edit_spec.rb new file mode 100644 index 0000000000..32fbab2921 --- /dev/null +++ b/spec/integration/admin_public_body_category_edit_spec.rb @@ -0,0 +1,58 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +require File.expand_path(File.dirname(__FILE__) + '/alaveteli_dsl') + +describe 'Editing a Public Body Category' do + before do + AlaveteliConfiguration.stub!(:skip_admin_auth).and_return(false) + + confirm(:admin_user) + @admin = login(:admin_user) + @category = FactoryGirl.create(:public_body_category) + end + + it 'can edit the default locale' do + @admin.visit edit_admin_category_path(@category) + @admin.fill_in 'public_body_category_title__en', :with => 'New Category EN' + @admin.click_button 'Save' + + @category.reload + expect(@category.title).to eq('New Category EN') + end + + it 'can add a translation for a single locale' do + expect(@category.find_translation_by_locale('fr')).to be_nil + + @admin.visit edit_admin_category_path(@category) + @admin.fill_in 'public_body_category_translations_attributes_fr_title__fr', :with => 'New Category FR' + @admin.click_button 'Save' + + @category.reload + I18n.with_locale(:fr) do + expect(@category.title).to eq('New Category FR') + end + end + + it 'can add a translation for multiple locales' do + # Add FR translation + expect(@category.find_translation_by_locale('fr')).to be_nil + @admin.visit edit_admin_category_path(@category) + @admin.fill_in 'public_body_category_translations_attributes_fr_title__fr', :with => 'New Category FR' + @admin.click_button 'Save' + + # Add ES translation + expect(@category.find_translation_by_locale('es')).to be_nil + @admin.visit edit_admin_category_path(@category) + @admin.fill_in 'public_body_category_translations_attributes_es_title__es', :with => 'New Category ES' + @admin.click_button 'Save' + + @category.reload + I18n.with_locale(:fr) do + expect(@category.title).to eq('New Category FR') + end + + I18n.with_locale(:es) do + expect(@category.title).to eq('New Category ES') + end + end + +end diff --git a/spec/models/public_body_category_spec.rb b/spec/models/public_body_category_spec.rb index 96fe5686b4..f9963dcaee 100644 --- a/spec/models/public_body_category_spec.rb +++ b/spec/models/public_body_category_spec.rb @@ -64,4 +64,64 @@ category.errors[:description].should == ["Description can't be blank"] end end + + 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) + PublicBodyCategory.new().translations_attributes = attrs + end + + it 'updates an existing translation' do + category = FactoryGirl.create(:public_body_category) + params = { 'es' => { :locale => 'es', + :title => 'Renamed' } } + + category.translations_attributes = params + I18n.with_locale(:es) { expect(category.title).to eq('Renamed') } + end + + it 'updates an existing translation and creates a new translation' do + category = FactoryGirl.create(:public_body_category) + category.translations.create(:locale => 'es', + :title => 'Los Category', + :description => 'ES Description') + + expect(category.translations.size).to eq(2) + + category.translations_attributes = { + 'es' => { :locale => 'es', + :title => 'Renamed' }, + 'fr' => { :locale => 'fr', + :title => 'Le Category' } + } + + expect(category.translations.size).to eq(3) + I18n.with_locale(:es) { expect(category.title).to eq('Renamed') } + I18n.with_locale(:fr) { expect(category.title).to eq('Le Category') } + end + + it 'skips empty translations' do + category = FactoryGirl.create(:public_body_category) + category.translations.create(:locale => 'es', + :title => 'Los Category', + :description => 'ES Description') + + expect(category.translations.size).to eq(2) + + category.translations_attributes = { + 'es' => { :locale => 'es', + :title => 'Renamed' }, + 'fr' => { :locale => 'fr' } + } + + expect(category.translations.size).to eq(2) + end + + end + end + end