Fix translating Public Body Headings

Fixes submission of form containing both existing and new
translations

7 files changed, 683 insertions, 184 deletions

diff --git a/app/controllers/admin_public_body_headings_controller.rb b/app/controllers/admin_public_body_headings_controller.rb
index e893e760d..a7fe27390 100644
--- a/app/controllers/admin_public_body_headings_controller.rb
+++ b/app/controllers/admin_public_body_headings_controller.rb
@@ -1,22 +1,52 @@
 class AdminPublicBodyHeadingsController < AdminController
 
+    def new
+        @heading = PublicBodyHeading.new
+        @heading.build_all_translations
+    end
+
+    def create
+        I18n.with_locale(I18n.default_locale) do
+            @heading = PublicBodyHeading.new(params[:public_body_heading])
+            if @heading.save
+                flash[:notice] = 'Heading was successfully created.'
+                redirect_to admin_categories_url
+            else
+                @heading.build_all_translations
+                render :action => 'new'
+            end
+        end
+    end
+
     def edit
         @heading = PublicBodyHeading.find(params[:id])
-        render :formats => [:html]
+        @heading.build_all_translations
     end
 
     def update
+        @heading = PublicBodyHeading.find(params[:id])
+
         I18n.with_locale(I18n.default_locale) do
-            @heading = PublicBodyHeading.find(params[:id])
             if @heading.update_attributes(params[:public_body_heading])
-                flash[:notice] = 'Category heading was successfully updated.'
+                flash[:notice] = 'Heading was successfully updated.'
                 redirect_to edit_admin_heading_path(@heading)
             else
+                @heading.build_all_translations
                 render :action => 'edit'
             end
         end
     end
 
+    def destroy
+        @locale = self.locale_from_params
+        I18n.with_locale(@locale) do
+            heading = PublicBodyHeading.find(params[:id])
+            heading.destroy
+            flash[:notice] = "Heading was successfully destroyed."
+            redirect_to admin_categories_url
+        end
+    end
+
     def reorder
         transaction = reorder_headings(params[:headings])
         if transaction[:success]
@@ -35,33 +65,6 @@ class AdminPublicBodyHeadingsController < AdminController
         end
     end
 
-    def new
-        @heading = PublicBodyHeading.new
-        render :formats => [:html]
-    end
-
-    def create
-        I18n.with_locale(I18n.default_locale) do
-            @heading = PublicBodyHeading.new(params[:public_body_heading])
-            if @heading.save
-                flash[:notice] = 'Category heading was successfully created.'
-                redirect_to admin_categories_url
-            else
-                render :action => 'new'
-            end
-        end
-    end
-
-    def destroy
-        @locale = self.locale_from_params()
-        I18n.with_locale(@locale) do
-            heading = PublicBodyHeading.find(params[:id])
-            heading.destroy
-            flash[:notice] = "Category heading was successfully destroyed."
-            redirect_to admin_categories_url
-        end
-    end
-
     protected
 
     def reorder_headings(headings)
diff --git a/app/models/public_body_heading.rb b/app/models/public_body_heading.rb
index f394c37c6..8c160ba8b 100644
--- a/app/models/public_body_heading.rb
+++ b/app/models/public_body_heading.rb
@@ -7,13 +7,15 @@
 #
 
 class PublicBodyHeading < ActiveRecord::Base
-    attr_accessible :name, :display_order, :translated_versions
+    attr_accessible :locale, :name, :display_order, :translated_versions,
+                    :translations_attributes
 
     has_many :public_body_category_links, :dependent => :destroy
     has_many :public_body_categories, :order => :category_display_order, :through => :public_body_category_links
     default_scope order('display_order ASC')
 
     translates :name
+    accepts_nested_attributes_for :translations, :reject_if => :empty_translation_in_params?
 
     validates_uniqueness_of :name, :message => 'Name is already taken'
     validates_presence_of :name, :message => 'Name can\'t be blank'
@@ -36,24 +38,20 @@ class PublicBodyHeading < ActiveRecord::Base
     end
 
     def translated_versions=(translation_attrs)
-        def empty_translation?(attrs)
-            attrs_with_values = attrs.select{ |key, value| value != '' and key != 'locale' }
-            attrs_with_values.empty?
-        end
+        warn "[DEPRECATION] PublicBodyHeading#translated_versions= will be replaced " \
+             "by PublicBodyHeading#translations_attributes= as of release 0.22"
+        self.translations_attributes = translation_attrs
+    end
+
+    def ordered_translations
+        translations.
+          select { |t| I18n.available_locales.include?(t.locale) }.
+            sort_by { |t| I18n.available_locales.index(t.locale) }
+    end
 
-        if translation_attrs.respond_to? :each_value    # Hash => updating
-            translation_attrs.each_value do |attrs|
-                next if empty_translation?(attrs)
-                t = translation_for(attrs[:locale]) || PublicBodyHeading::Translation.new
-                t.attributes = attrs
-                t.save!
-            end
-        else                                            # Array => creating
-            translation_attrs.each do |attrs|
-                next if empty_translation?(attrs)
-                new_translation = PublicBodyHeading::Translation.new(attrs)
-                translations << new_translation
-            end
+    def build_all_translations
+        I18n.available_locales.each do |locale|
+            translations.build(:locale => locale) unless translations.detect{ |t| t.locale == locale }
         end
     end
 
@@ -71,4 +69,13 @@ class PublicBodyHeading < ActiveRecord::Base
         end
     end
 
+    private
+
+    def empty_translation_in_params?(attributes)
+        attrs_with_values = attributes.select do |key, value|
+            value != '' and key.to_s != 'locale'
+        end
+        attrs_with_values.empty? 
+    end
+
 end
diff --git a/app/views/admin_public_body_headings/_form.html.erb b/app/views/admin_public_body_headings/_form.html.erb
index d4e914ca1..7eaa4bff7 100644
--- a/app/views/admin_public_body_headings/_form.html.erb
+++ b/app/views/admin_public_body_headings/_form.html.erb
@@ -1,40 +1,49 @@
-<%= error_messages_for 'heading' %>
+<% if @heading.errors.any? %>
+  <ul>
+  <% @heading.errors.each do |attr, message| %>
+    <% unless attr.to_s.starts_with?('translation') %>
+    <li><%= message %></li>
+    <% end %>
+  <% end %>
+  </ul>
+<% end %>
+
+<% @heading.ordered_translations.each do |translation| %>
+  <% if translation.errors.any? %>
+    <%= locale_name(translation.locale.to_s) || translation.locale.to_s %>
+    <ul>
+    <% translation.errors.each do |attr, message| %>
+      <li><%= message %></li>
+    <% end %>
+    </ul>
+  <% end %>
+<% end %>
+
 
 <!--[form:public_body_heading]-->
 
 <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 %>
+    <% @heading.ordered_translations.each do |translation| %>
+      <li>
+        <a href="#div-locale-<%= translation.locale.to_s %>" data-toggle="tab" >
+          <%= locale_name(translation.locale.to_s) || translation.locale.to_s %>
+        </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_heading'
-            object = @heading
-        else                            # ...but additional locales go "on the side"
-            prefix = "public_body_heading[translated_versions][]"
-            object = @heading.new_record? ?
-                        PublicBodyHeading::Translation.new :
-                        @heading.find_translation_by_locale(locale.to_s) || PublicBodyHeading::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>
-    <%
-    end
-end
-%>
+    <% @heading.ordered_translations.each do |translation| %>
+      <% if translation.locale.to_s == I18n.default_locale.to_s %>
+        <%= fields_for('public_body_heading', @heading) 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_headings/_locale_fields.html.erb b/app/views/admin_public_body_headings/_locale_fields.html.erb
new file mode 100644
index 000000000..3846bfafa
--- /dev/null
+++ b/app/views/admin_public_body_headings/_locale_fields.html.erb
@@ -0,0 +1,9 @@
+<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>
diff --git a/spec/controllers/admin_public_body_headings_controller_spec.rb b/spec/controllers/admin_public_body_headings_controller_spec.rb
index afbe0fa30..ccdfdecfb 100644
--- a/spec/controllers/admin_public_body_headings_controller_spec.rb
+++ b/spec/controllers/admin_public_body_headings_controller_spec.rb
@@ -2,161 +2,466 @@ require 'spec_helper'
 
 describe AdminPublicBodyHeadingsController do
 
-    context 'when showing the form for a new public body category' do
-        it 'should assign a new public body heading to the view' do
+    describe :new do
+
+        it 'responds successfully' do
             get :new
-            assigns[:heading].should be_a(PublicBodyHeading)
+            expect(response).to be_success
         end
 
-        it 'renders the new template' do
+        it 'builds a new PublicBodyHeading' do
             get :new
-            expect(response).to render_template('new')
+            expect(assigns(:heading)).to be_new_record
         end
+
+       it 'builds new translations for all locales' do
+           get :new
+
+           translations = assigns(:heading).translations.map{ |t| t.locale.to_s }.sort
+           available = I18n.available_locales.map{ |l| l.to_s }.sort
+
+           expect(translations).to eq(available)
+       end
+
+       it 'renders the new template' do
+           get :new
+           expect(response).to render_template('new')
+       end
+
     end
 
-    context 'when creating a public body heading' do
-        it "creates a new public body heading in one locale" do
-            n = PublicBodyHeading.count
-            post :create, {
-                :public_body_heading => {
-                    :name => 'New Heading'
-                 }
-            }
-            PublicBodyHeading.count.should == n + 1
-
-            heading = PublicBodyHeading.find_by_name("New Heading")
-            response.should redirect_to(admin_categories_path)
-        end
+    describe :create do
 
-        it 'creates a new public body heading with multiple locales' do
-            n = PublicBodyHeading.count
-            post :create, {
-                :public_body_heading => {
-                    :name => 'New Heading',
-                    :translated_versions => [{ :locale => "es",
-                                               :name => "Mi Nuevo Heading" }]
-                }
-            }
-            PublicBodyHeading.count.should == n + 1
-
-            heading = PublicBodyHeading.find_by_name("New Heading")
-            heading.translations.map {|t| t.locale.to_s}.sort.should == ["en", "es"]
-            I18n.with_locale(:en) do
-                heading.name.should == "New Heading"
+        context 'on success' do
+
+            before(:each) do
+              PublicBodyHeading.destroy_all
+              @params = { :translations_attributes => {
+                            'en' => { :locale => 'en',
+                                      :name => 'New Heading' }
+                          } }
             end
-            I18n.with_locale(:es) do
-                heading.name.should == "Mi Nuevo Heading"
+
+            it 'creates a new heading in the default locale' do
+                expect {
+                  post :create, :public_body_heading => @params
+                }.to change{ PublicBodyHeading.count }.from(0).to(1)
             end
 
-            response.should redirect_to(admin_categories_path)
-        end
+            it 'notifies the admin that the heading was created' do
+                post :create, :public_body_heading => @params
+                expect(flash[:notice]).to eq('Heading was successfully created.')
+            end
+
+            it 'redirects to the categories index' do
+                post :create, :public_body_heading => @params
+                expect(response).to redirect_to(admin_categories_path)
+            end
 
-        it "renders the form if creating the record was unsuccessful" do
-            post :create, :public_body_heading => { :name => '' }
-            expect(response).to render_template('new')
         end
 
-    end
+        context 'on success for multiple locales' do
 
-    context 'when editing a public body heading' do
-        before do
-            @heading = FactoryGirl.create(:public_body_heading)
-        end
+            before(:each) do
+              PublicBodyHeading.destroy_all
+              @params = { :translations_attributes => {
+                            'en' => { :locale => 'en',
+                                      :name => 'New Heading' },
+                            'es' => { :locale => 'es',
+                                      :name => 'Mi Nuevo Heading' }
+                          } }
+            end
 
-        render_views
+            it 'saves the heading' do
+                expect {
+                  post :create, :public_body_heading => @params
+                }.to change{ PublicBodyHeading.count }.from(0).to(1)
+            end
 
-        it "finds the requested heading" do
-            get :edit, :id => @heading.id
-            expect(assigns[:heading]).to eq(@heading)
-        end
+            it 'saves the default locale translation' do
+                post :create, :public_body_heading => @params
 
-        it "renders the edit template" do
-            get :edit, :id => @heading.id
-            expect(assigns[:heading]).to render_template('edit')
-        end
-    end
+                heading = PublicBodyHeading.find_by_name('New Heading')
+
+                I18n.with_locale(:en) do
+                    expect(heading.name).to eq('New Heading')
+                end
+            end
+
+            it 'saves the alternative locale translation' do
+                post :create, :public_body_heading => @params
+
+                heading = PublicBodyHeading.find_by_name('New Heading')
+
+                I18n.with_locale(:es) do
+                    expect(heading.name).to eq('Mi Nuevo Heading')
+                end
+            end
 
-    context 'when updating a public body heading' do
-        before do
-            @heading = FactoryGirl.create(:public_body_heading)
-            @name = @heading.name
         end
 
-        it "saves edits to a public body heading" do
-            post :update, { :id => @heading.id,
-                            :public_body_heading => { :name => "Renamed" } }
-            request.flash[:notice].should include('successful')
-            found_heading = PublicBodyHeading.find(@heading.id)
-            found_heading.name.should == "Renamed"
+        context 'on failure' do
+
+            it 'renders the form if creating the record was unsuccessful' do
+                post :create, :public_body_heading => { :name => '' }
+                expect(response).to render_template('new')
+            end
+
+            it 'is rebuilt with the given params' do
+                post :create, :public_body_heading => { :name => 'Need a description' }
+                expect(assigns(:heading).name).to eq('Need a description')
+            end
+
         end
 
-        it "saves edits to a public body heading in another locale" do
-            I18n.with_locale(:es) do
-                post :update, {
-                    :id => @heading.id,
-                    :public_body_heading => {
-                        :name => @name,
-                        :translated_versions => {
-                            @heading.id => {:locale => "es",
-                                            :name => "Renamed"}
-                            }
-                        }
-                    }
-                request.flash[:notice].should include('successful')
+        context 'on failure for multiple locales' do
+
+            before(:each) do
+                @params = { :translations_attributes => {
+                              'en' => { :locale => 'en',
+                                        :name => 'Need a description' },
+                              'es' => { :locale => 'es',
+                                        :name => 'Mi Nuevo Heading' }
+                            } }
+            end
+            
+            it 'is rebuilt with the default locale translation' do
+                post :create, :public_body_heading => @params
+                expect(assigns(:heading).name).to eq('Need a description')
             end
 
-            heading = PublicBodyHeading.find(@heading.id)
-            I18n.with_locale(:es) do
-               heading.name.should == "Renamed"
+            it 'is rebuilt with the alternative locale translation' do
+                post :create, :public_body_heading => @params
+
+                I18n.with_locale(:es) do
+                    expect(assigns(:heading).name).to eq('Mi Nuevo Heading')
+                end
             end
-            I18n.with_locale(:en) do
-               heading.name.should == @name
+
+        end
+
+    end
+
+    describe :edit do
+
+        before do
+            @heading = FactoryGirl.create(:public_body_heading)
+            I18n.with_locale('es') do
+                @heading.name = 'Los heading'
+                @heading.save!
             end
         end
 
-        it "redirects to the edit page after a successful update" do
-            post :update, { :id => @heading.id,
-                            :public_body_heading => { :name => "Renamed" } }
+        it 'responds successfully' do
+            get :edit, :id => @heading.id
+            expect(response).to be_success
+        end
 
-            expect(response).to redirect_to(edit_admin_heading_path(@heading))
+        it 'finds the requested heading' do
+            get :edit, :id => @heading.id
+            expect(assigns[:heading]).to eq(@heading)
         end
 
-        it "re-renders the edit form after an unsuccessful update" do
-            post :update, { :id => @heading.id,
-                            :public_body_heading => { :name => '' } }
+        it 'builds new translations if the body does not already have a translation in the specified locale' do
+            get :edit, :id => @heading.id
+            expect(assigns[:heading].translations.map(&:locale)).to include(:fr)
+        end
 
+        it 'renders the edit template' do
+            get :edit, :id => @heading.id
             expect(response).to render_template('edit')
         end
 
     end
 
-    context 'when destroying a public body heading' do
+    describe :update do
+
+         before do
+             @heading = FactoryGirl.create(:public_body_heading)
+             I18n.with_locale('es') do
+                 @heading.name = 'Los heading'
+                 @heading.save!
+             end
+             @params = { :translations_attributes => {
+                           'en' => { :id => @heading.translation_for(:en).id,
+                                     :locale => 'en',
+                                     :name => @heading.name(:en) },
+                           'es' => { :id => @heading.translation_for(:es).id,
+                                     :locale => 'es',
+                                     :title => @heading.name(:es) }
+                         } }
+         end
+
+         it 'finds the heading to update' do
+             post :update, :id => @heading.id,
+                           :public_body_category => @params
+             expect(assigns(:heading)).to eq(@heading)
+         end
+
+         context 'on success' do
+
+             before(:each) do
+               @params = { :id => @heading.id,
+                           :public_body_heading => {
+                             :translations_attributes => {
+                               'en' => { :id => @heading.translation_for(:en).id,
+                                         :name => 'Renamed' }
+                             }
+                           }
+                         }
+             end
+
+             it 'saves edits to a public body heading' do
+                 post :update, @params
+                 heading = PublicBodyHeading.find(@heading.id)
+                 expect(heading.name).to eq('Renamed')
+             end
+
+             it 'notifies the admin that the heading was updated' do
+                 post :update, @params
+                 expect(flash[:notice]).to eq('Heading was successfully updated.')
+             end
+
+             it 'redirects to the heading edit page' do
+                 post :update, @params
+                 expect(response).to redirect_to(edit_admin_heading_path(@heading))
+             end
+
+         end
+
+         context 'on success for multiple locales' do
+
+             it 'saves edits to a public body heading in another locale' do
+                 @heading.name(:es).should == 'Los heading'
+                 post :update, :id => @heading.id,
+                               :public_body_heading => {
+                                   :translations_attributes => {
+                                       'en' => { :id => @heading.translation_for(:en).id,
+                                                 :locale => 'en',
+                                                 :name => @heading.name(:en) },
+                                       'es' => { :id => @heading.translation_for(:es).id,
+                                                 :locale => 'es',
+                                                 :name => 'Renamed' }
+                                   }
+                               }
+
+                 heading = PublicBodyHeading.find(@heading.id)
+                 expect(heading.name(:es)).to eq('Renamed')
+                 expect(heading.name(:en)).to eq(@heading.name(:en))
+             end
+
+             it 'adds a new translation' do
+                  @heading.translation_for(:es).destroy
+                  @heading.reload
+
+                  put :update, {
+                      :id => @heading.id,
+                      :public_body_heading => {
+                          :translations_attributes => {
+                              'en' => { :id => @heading.translation_for(:en).id,
+                                        :locale => 'en',
+                                        :name => @heading.name(:en) },
+                              'es' => { :locale => "es",
+                                        :name => "Example Public Body Heading ES" }
+                          }
+                      }
+                  }
+
+                  request.flash[:notice].should include('successful')
+
+                  heading = PublicBodyHeading.find(@heading.id)
+
+                  I18n.with_locale(:es) do
+                     expect(heading.name).to eq('Example Public Body Heading ES')
+                  end
+              end
+
+              it 'adds new translations' do
+                  @heading.translation_for(:es).destroy
+                  @heading.reload
+
+                  post :update, {
+                      :id => @heading.id,
+                      :public_body_heading => {
+                          :translations_attributes => {
+                              'en' => { :id => @heading.translation_for(:en).id,
+                                        :locale => 'en',
+                                        :name => @heading.name(:en) },
+                              'es' => { :locale => "es",
+                                        :name => "Example Public Body Heading ES" },
+                              'fr' => { :locale => "fr",
+                                        :name => "Example Public Body Heading FR" }
+                          }
+                      }
+                  }
+
+                  request.flash[:notice].should include('successful')
+
+                  heading = PublicBodyHeading.find(@heading.id)
+
+                  I18n.with_locale(:es) do
+                     expect(heading.name).to eq('Example Public Body Heading ES')
+                  end
+                  I18n.with_locale(:fr) do
+                     expect(heading.name).to eq('Example Public Body Heading FR')
+                  end
+              end
+
+              it 'updates an existing translation and adds a third translation' do
+                  post :update, {
+                      :id => @heading.id,
+                      :public_body_heading => {
+                          :translations_attributes => {
+                              'en' => { :id => @heading.translation_for(:en).id,
+                                        :locale => 'en',
+                                        :name => @heading.name(:en) },
+                              # Update existing translation
+                              'es' => { :id => @heading.translation_for(:es).id,
+                                        :locale => "es",
+                                        :name => "Renamed Example Public Body Heading ES" },
+                              # Add new translation
+                              'fr' => { :locale => "fr",
+                                        :name => "Example Public Body Heading FR" }
+                          }
+                      }
+                  }
+
+                  request.flash[:notice].should include('successful')
+
+                  heading = PublicBodyHeading.find(@heading.id)
+
+                  I18n.with_locale(:es) do
+                     expect(heading.name).to eq('Renamed Example Public Body Heading ES')
+                  end
+                  I18n.with_locale(:fr) do
+                     expect(heading.name).to eq('Example Public Body Heading FR')
+                  end
+              end
+
+             it "redirects to the edit page after a successful update" do
+                 post :update, :id => @heading.id,
+                               :public_body_heading => {
+                                 :translations_attributes => {
+                                     'en' => { :id => @heading.translation_for(:en).id,
+                                               :locale => 'en',
+                                               :name => @heading.name(:en) }
+                                 } }
+
+                 expect(response).to redirect_to(edit_admin_heading_path(@heading))
+             end
+
+         end
+
+         context 'on failure' do
+
+             it 'renders the form if creating the record was unsuccessful' do
+                 post :update, :id => @heading.id,
+                               :public_body_heading => {
+                                 :translations_attributes => {
+                                     'en' => { :id => @heading.translation_for(:en).id,
+                                               :locale => 'en',
+                                               :name => '' }
+                                 } }
+                 expect(response).to render_template('edit')
+             end
+
+             it 'is rebuilt with the given params' do
+                 post :update, :id => @heading.id,
+                               :public_body_heading => {
+                                 :translations_attributes => {
+                                     'en' => { :id => @heading.translation_for(:en).id,
+                                               :locale => 'en',
+                                               :name => 'Need a description' }
+                                 } }
+                 expect(assigns(:heading).name).to eq('Need a description')
+             end
+
+         end
+
+         context 'on failure for multiple locales' do
+
+             before(:each) do
+                 @params = { :translations_attributes => {
+                               'en' => { :id => @heading.translation_for(:en).id,
+                                         :locale => 'en',
+                                         :name => '' },
+                               'es' => { :id => @heading.translation_for(:es).id,
+                                         :locale => 'es',
+                                         :name => 'Mi Nuevo Heading' }
+                             } }
+             end
+
+             it 'is rebuilt with the default locale translation' do
+                 post :update, :id => @heading.id,
+                               :public_body_heading => @params
+                 expect(assigns(:heading).name(:en)).to eq('')
+             end
+
+             it 'is rebuilt with the alternative locale translation' do
+                 post :update, :id => @heading.id,
+                               :public_body_heading => @params
+
+                 I18n.with_locale(:es) do
+                     expect(assigns(:heading).name).to eq('Mi Nuevo Heading')
+                 end
+             end
+
+         end
+
+     end
+
+    describe :destroy do
+
+        it 'uses the current locale by default' do
+            heading = FactoryGirl.create(:public_body_heading)
+            post :destroy, :id => heading.id
+            expect(assigns(:locale)).to eq(I18n.locale.to_s)
+        end
 
-        before do
-            @heading = FactoryGirl.create(:public_body_heading)
+        it 'sets the locale if the show_locale param is passed' do
+            heading = FactoryGirl.create(:public_body_heading)
+            post :destroy, :id => heading.id, :show_locale => 'es'
+            expect(assigns(:locale)).to eq('es')
         end
 
-        it "destroys a public body heading that has associated categories" do
+        it 'destroys the public body heading' do
+            PublicBodyHeading.destroy_all
+
+            heading = FactoryGirl.create(:public_body_heading)
+
+            expect{
+              post :destroy, :id => heading.id
+            }.to change{ PublicBodyHeading.count }.from(1).to(0)
+        end
+
+        it 'destroys a heading that has associated categories' do
+            PublicBodyHeading.destroy_all
+            PublicBodyCategory.destroy_all
+
+            heading = FactoryGirl.create(:public_body_heading)
             category = FactoryGirl.create(:public_body_category)
             link = FactoryGirl.create(:public_body_category_link,
                                       :public_body_category => category,
-                                      :public_body_heading => @heading,
+                                      :public_body_heading => heading,
                                       :category_display_order => 0)
-            n = PublicBodyHeading.count
-            n_links = PublicBodyCategoryLink.count
 
-            post :destroy, { :id => @heading.id }
-            response.should redirect_to(admin_categories_path)
-            PublicBodyHeading.count.should == n - 1
-            PublicBodyCategoryLink.count.should == n_links - 1
+            expect{
+              post :destroy, :id => heading.id
+            }.to change{ PublicBodyHeading.count }.from(1).to(0)
+        end
+
+        it 'notifies the admin that the heading was destroyed' do
+            heading = FactoryGirl.create(:public_body_heading)
+            post :destroy, :id => heading.id
+            expect(flash[:notice]).to eq('Heading was successfully destroyed.')
         end
 
-        it "destroys an empty public body heading" do
-            n = PublicBodyHeading.count
-            post :destroy, { :id => @heading.id }
-            response.should redirect_to(admin_categories_path)
-            PublicBodyHeading.count.should == n - 1
+        it 'redirects to the categories index' do
+            heading = FactoryGirl.create(:public_body_heading)
+            post :destroy, :id => heading.id
+            expect(response).to redirect_to(admin_categories_path)
         end
+
     end
 
     context 'when reordering public body headings' do
diff --git a/spec/integration/admin_public_body_heading_edit_spec.rb b/spec/integration/admin_public_body_heading_edit_spec.rb
new file mode 100644
index 000000000..6c7a5a74b
--- /dev/null
+++ b/spec/integration/admin_public_body_heading_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 Heading' do
+    before do
+        AlaveteliConfiguration.stub!(:skip_admin_auth).and_return(false)
+
+        confirm(:admin_user)
+        @admin = login(:admin_user)
+        @heading = FactoryGirl.create(:public_body_heading)
+    end
+
+    it 'can edit the default locale' do
+        @admin.visit edit_admin_heading_path(@heading)
+        @admin.fill_in 'public_body_heading_name__en', :with => 'New Heading EN'
+        @admin.click_button 'Save'
+
+        @heading.reload
+        expect(@heading.name).to eq('New Heading EN')
+    end
+
+    it 'can add a translation for a single locale' do
+        expect(@heading.find_translation_by_locale('fr')).to be_nil
+        
+        @admin.visit edit_admin_heading_path(@heading)
+        @admin.fill_in 'public_body_heading_translations_attributes_fr_name__fr', :with => 'New Heading FR'
+        @admin.click_button 'Save'
+
+        @heading.reload
+        I18n.with_locale(:fr) do
+            expect(@heading.name).to eq('New Heading FR')
+        end
+    end
+
+    it 'can add a translation for multiple locales' do
+        # Add FR translation
+        expect(@heading.find_translation_by_locale('fr')).to be_nil
+        @admin.visit edit_admin_heading_path(@heading)
+        @admin.fill_in 'public_body_heading_translations_attributes_fr_name__fr', :with => 'New Heading FR'
+        @admin.click_button 'Save'
+
+        # Add ES translation
+        expect(@heading.find_translation_by_locale('es')).to be_nil
+        @admin.visit edit_admin_heading_path(@heading)
+        @admin.fill_in 'public_body_heading_translations_attributes_es_name__es', :with => 'New Heading ES'
+        @admin.click_button 'Save'
+
+        @heading.reload
+        I18n.with_locale(:fr) do
+            expect(@heading.name).to eq('New Heading FR')
+        end
+
+        I18n.with_locale(:es) do
+            expect(@heading.name).to eq('New Heading ES')
+        end
+    end
+
+end
diff --git a/spec/models/public_body_heading_spec.rb b/spec/models/public_body_heading_spec.rb
index 620f7da9c..be3e7c7d2 100644
--- a/spec/models/public_body_heading_spec.rb
+++ b/spec/models/public_body_heading_spec.rb
@@ -30,6 +30,13 @@ describe PublicBodyHeading do
             heading.valid?
             heading.display_order.should == PublicBodyHeading.next_display_order
         end
+
+        it 'validates the translations' do
+            heading = FactoryGirl.build(:public_body_heading)
+            translation = heading.translations.build
+            expect(heading).to_not be_valid
+        end
+
     end
 
     context 'when setting a display order' do
@@ -43,4 +50,105 @@ describe PublicBodyHeading do
             PublicBodyHeading.next_display_order.should == 1
         end
     end
+
+    describe :save do
+
+      it 'saves translations' do
+          heading = FactoryGirl.build(:public_body_heading)
+          heading.translations_attributes = { :es => { :locale => 'es',
+                                                       :name => 'El Heading' } }
+
+          heading.save
+          expect(PublicBodyHeading.find(heading.id).translations.size).to eq(2)
+      end
+
+    end
+
+    describe :translations_attributes= do
+
+        context 'translation_attrs is a Hash' do
+
+            it 'does not persist translations' do
+                heading = FactoryGirl.create(:public_body_heading)
+                heading.translations_attributes = { :es => { :locale => 'es',
+                                                             :name => 'El Heading' } }
+
+                expect(PublicBodyHeading.find(heading.id).translations.size).to eq(1)
+            end
+
+            it 'creates a new translation' do
+                heading = FactoryGirl.create(:public_body_heading)
+                heading.translations_attributes = { :es => { :locale => 'es',
+                                                             :name => 'El Heading' } }
+                heading.save
+                heading.reload
+                expect(heading.name(:es)).to eq('El Heading')
+            end
+
+            it 'updates an existing translation' do
+                heading = FactoryGirl.create(:public_body_heading)
+                heading.translations_attributes = { 'es' => { :locale => 'es',
+                                                              :name => 'Name' } }
+                heading.save
+
+                heading.translations_attributes = { 'es' => { :id => heading.translation_for(:es).id,
+                                                              :locale => 'es',
+                                                              :name => 'Renamed' } }
+                heading.save
+                expect(heading.name(:es)).to eq('Renamed')
+            end
+
+            it 'updates an existing translation and creates a new translation' do
+                heading = FactoryGirl.create(:public_body_heading)
+                heading.translations.create(:locale => 'es',
+                                             :name => 'Los Heading')
+
+                expect(heading.translations.size).to eq(2)
+
+                heading.translations_attributes = {
+                    'es' => { :id => heading.translation_for(:es).id,
+                              :locale => 'es',
+                              :name => 'Renamed' },
+                    'fr' => { :locale => 'fr',
+                              :name => 'Le Heading' }
+                }
+
+                expect(heading.translations.size).to eq(3)
+                I18n.with_locale(:es) { expect(heading.name).to eq('Renamed') }
+                I18n.with_locale(:fr) { expect(heading.name).to eq('Le Heading') }
+            end
+
+            it 'skips empty translations' do
+                heading = FactoryGirl.create(:public_body_heading)
+                heading.translations.create(:locale => 'es',
+                                             :name => 'Los Heading')
+
+                expect(heading.translations.size).to eq(2)
+
+                heading.translations_attributes = {
+                    'es' => { :id => heading.translation_for(:es).id,
+                              :locale => 'es',
+                              :name => 'Renamed' },
+                    'fr' => { :locale => 'fr' }
+                }
+
+                expect(heading.translations.size).to eq(2)
+            end
+        end
+    end
+end
+
+describe PublicBodyHeading::Translation do
+
+  it 'requires a locale' do
+    translation = PublicBodyHeading::Translation.new
+    translation.valid?
+    expect(translation.errors[:locale]).to eq(["can't be blank"])
+  end
+
+  it 'is valid if all required attributes are assigned' do
+    translation = PublicBodyHeading::Translation.new(:locale => I18n.default_locale)
+    expect(translation).to be_valid
+  end
+
 end