Fix translating Public Body Categories

Fixes submission of form containing both existing and new translations

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
 

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,13 +79,25 @@ 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)
                 translations << new_translation
             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
 
 

app/views/admin_public_body_categories/_form.html.erb

@@ -4,43 +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 %>
+    <% @category.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">
-<%
-    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| %>
-    <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, :title, locale) %>" class="control-label">Title</label>
-            <div class="controls">
-                <%= t.text_field :title, :id => form_tag_id(t.object_name, :title, locale), :class => "span4" %>
-            </div>
-        </div>
-        <div class="control-group">
-            <label for="<%= form_tag_id(t.object_name, :description, locale) %>" class="control-label">Description</label>
-            <div class="controls">
-                <%= t.text_field :description, :id => form_tag_id(t.object_name, :description, locale), :class => "span4" %>
-            </div>
-        </div>
-    </div>
-    <%
-    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 %>
   </div>
 </div>
 

app/views/admin_public_body_categories/_locale_fields.html.erb

@@ -0,0 +1,15 @@
+<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, :title, locale) %>" class="control-label">Title</label>
+        <div class="controls">
+            <%= t.text_field :title, :id => form_tag_id(t.object_name, :title, locale), :class => "span4" %>
+        </div>
+    </div>
+    <div class="control-group">
+        <label for="<%= form_tag_id(t.object_name, :description, locale) %>" class="control-label">Description</label>
+        <div class="controls">
+            <%= t.text_field :description, :id => form_tag_id(t.object_name, :description, locale), :class => "span4" %>
+        </div>
+    </div>
+</div>

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,

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