Fix translating Public Bodies

5 files changed, 476 insertions, 338 deletions

diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb
index d188f109d..7de27121a 100644
--- a/app/controllers/admin_public_body_controller.rb
+++ b/app/controllers/admin_public_body_controller.rb
@@ -23,10 +23,7 @@ class AdminPublicBodyController < AdminController
 
     def new
         @public_body = PublicBody.new
-
-        I18n.available_locales.each do |locale|
-            @public_body.translations.build(:locale => locale)
-        end
+        @public_body.build_all_translations
 
         if params[:change_request_id]
             @change_request = PublicBodyChangeRequest.find(params[:change_request_id])
@@ -58,6 +55,7 @@ class AdminPublicBodyController < AdminController
                 flash[:notice] = 'PublicBody was successfully created.'
                 redirect_to admin_body_url(@public_body)
             else
+                @public_body.build_all_translations
                 render :action => 'new'
             end
         end
@@ -65,10 +63,7 @@ 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
+        @public_body.build_all_translations
 
         if params[:change_request_id]
             @change_request = PublicBodyChangeRequest.find(params[:change_request_id])
@@ -99,6 +94,7 @@ class AdminPublicBodyController < AdminController
                 flash[:notice] = 'PublicBody was successfully updated.'
                 redirect_to admin_body_url(@public_body)
             else
+                @public_body.build_all_translations
                 render :action => 'edit'
             end
         end
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 0e90a3c16..232c0ffa1 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -64,7 +64,7 @@ class PublicBody < ActiveRecord::Base
     }
 
     translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme
-    accepts_nested_attributes_for :translations
+    accepts_nested_attributes_for :translations, :reject_if => :empty_translation_in_params?
 
     # Default fields available for importing from CSV, in the format
     # [field_name, 'short description of field (basic html allowed)']
@@ -152,33 +152,15 @@ class PublicBody < ActiveRecord::Base
         translations
     end
 
-    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)
-                t = translation_for(attrs[:locale]) || PublicBody::Translation.new
-                t.attributes = attrs
-                calculate_cached_fields(t)
-                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)
-                calculate_cached_fields(new_translation)
-                translations << new_translation
-            end
+    def ordered_translations
+        translations.
+          select { |t| I18n.available_locales.include?(t.locale) }.
+            sort_by { |t| I18n.available_locales.index(t.locale) }
+    end
+
+    def build_all_translations
+        I18n.available_locales.each do |locale|
+            translations.build(:locale => locale) unless translations.detect{ |t| t.locale == locale }
         end
     end
 
@@ -791,6 +773,13 @@ class PublicBody < ActiveRecord::Base
       end
     end
 
+    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
+
     def request_email_if_requestable
         # Request_email can be blank, meaning we don't have details
         if self.is_requestable?
diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb
index c765c116e..cf0c0e3de 100644
--- a/app/views/admin_public_body/_form.html.erb
+++ b/app/views/admin_public_body/_form.html.erb
@@ -1,20 +1,39 @@
-<%= error_messages_for 'public_body' %>
+<% if @public_body.errors.any? %>
+  <ul>
+  <% @public_body.errors.each do |attr, message| %>
+    <% unless attr.to_s.starts_with?('translation') %>
+    <li><%= message %></li>
+    <% end %>
+  <% end %>
+  </ul>
+<% end %>
+
+<% @public_body.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]-->
 
 <div id="div-locales">
   <ul class="locales nav nav-tabs">
-    <% @public_body.translations.each do |translation| %>
+    <% @public_body.ordered_translations.each do |translation| %>
       <li>
         <a href="#div-locale-<%= translation.locale.to_s %>" data-toggle="tab">
-          <%= locale_name(translation.locale.to_s) || _("Default locale") %>
+          <%= locale_name(translation.locale.to_s) || translation.locale.to_s %>
         </a>
       </li>
     <% end %>
   </ul>
 
   <div class="tab-content">
-    <% @public_body.translations.each do |translation| %>
+    <% @public_body.ordered_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 } %>
diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb
index 3ab58317a..50a373d9d 100644
--- a/spec/controllers/admin_public_body_controller_spec.rb
+++ b/spec/controllers/admin_public_body_controller_spec.rb
@@ -39,9 +39,14 @@ end
 
 describe AdminPublicBodyController, 'when showing the form for a new public body' do
 
+    it 'responds successfully' do
+        get :new
+        expect(response).to be_success
+    end
+
     it 'should assign a new public body to the view' do
         get :new
-        assigns[:public_body].should be_a(PublicBody)
+        expect(assigns(:public_body)).to be_new_record
     end
 
     it "builds new translations for all locales" do
@@ -53,6 +58,11 @@ describe AdminPublicBodyController, 'when showing the form for a new public body
         expect(translations).to eq(available)
     end
 
+    it 'renders the new template' do
+        get :new
+        expect(response).to render_template('new')
+    end
+
     context 'when passed a change request id as a param' do
         render_views
 
@@ -76,51 +86,127 @@ end
 describe AdminPublicBodyController, "when creating a public body" do
     render_views
 
-    it "creates a new public body in one locale" do
-        n = PublicBody.count
-        post :create, { :public_body => { :name => "New Quango",
-                                          :short_name => "",
-                                          :tag_string => "blah",
-                                          :request_email => 'newquango@localhost',
-                                          :last_edit_comment => 'From test code' } }
-        PublicBody.count.should == n + 1
+    context 'on success' do
+
+        before(:each) do
+          @params = { :public_body => { :name => 'New Quango',
+                                        :short_name => 'nq',
+                                        :request_email => 'newquango@localhost',
+                                        :tag_string => 'spec',
+                                        :last_edit_comment => 'From test code' } }
+        end
+
+        it 'creates a new body in the default locale' do
+            # FIXME: Can't call PublicBody.destroy_all because database
+            # database contstraints prevent them being deleted.
+            existing = PublicBody.count
+            expected = existing + 1
+            expect {
+              post :create, @params
+            }.to change{ PublicBody.count }.from(existing).to(expected)
+        end
+
+        it 'notifies the admin that the body was created' do
+            post :create, @params
+            expect(flash[:notice]).to eq('PublicBody was successfully created.')
+        end
+
+        it 'redirects to the admin page of the body' do
+            post :create, @params
+            expect(response).to redirect_to(admin_body_path(assigns(:public_body)))
+        end
 
-        body = PublicBody.find_by_name("New Quango")
-        response.should redirect_to(:controller=>'admin_public_body', :action=>'show', :id=>body.id)
     end
 
-    it "creates a new public body with multiple locales" do
-        n = PublicBody.count
-        post :create, {
-            :public_body => { :name => "New Quango",
-                              :short_name => "",
-                              :tag_string => "blah",
-                              :request_email => 'newquango@localhost',
-                              :last_edit_comment => 'From test code',
-                              :translations_attributes => {
-                                  'es' => { :locale => "es",
-                                            :name => "Mi Nuevo Quango",
-                                            :short_name => "",
-                                            :request_email => 'newquango@localhost' }
-                              }
-            }
-        }
-        PublicBody.count.should == n + 1
+    context 'on success for multiple locales' do
+
+        before(:each) do
+          @params = { :public_body => { :name => 'New Quango',
+                                        :short_name => 'nq',
+                                        :request_email => 'newquango@localhost',
+                                        :tag_string => 'spec',
+                                        :last_edit_comment => 'From test code',
+                                        :translations_attributes => {
+                                          'es' => { :locale => 'es',
+                                                    :name => 'Los Quango' }
+                                        } } }
+        end
+
+        it 'saves the body' do
+          # FIXME: Can't call PublicBody.destroy_all because database
+          # database contstraints prevent them being deleted.
+          existing = PublicBody.count
+          expected = existing + 1
+            expect {
+              post :create, @params
+            }.to change{ PublicBody.count }.from(existing).to(expected)
+        end
+
+        it 'saves the default locale translation' do
+            post :create, @params
 
-        body = PublicBody.find_by_name("New Quango")
-        body.translations.map {|t| t.locale.to_s}.sort.should == ["en", "es"]
-        I18n.with_locale(:en) do
-            body.name.should == "New Quango"
-            body.url_name.should == "new_quango"
-            body.first_letter.should == "N"
+            body = PublicBody.find_by_name('New Quango')
+
+            I18n.with_locale(:en) do
+                expect(body.name).to eq('New Quango')
+            end
+        end
+
+        it 'saves the alternative locale translation' do
+            post :create, @params
+
+            body = PublicBody.find_by_name('New Quango')
+
+            I18n.with_locale(:es) do
+                expect(body.name).to eq('Los Quango')
+            end
+        end
+
+    end
+
+    context 'on failure' do
+
+        it 'renders the form if creating the record was unsuccessful' do
+            post :create, :public_body => { :name => '',
+                                            :translations_attributes => {} }
+            expect(response).to render_template('new')
         end
-        I18n.with_locale(:es) do
-            body.name.should == "Mi Nuevo Quango"
-            body.url_name.should == "mi_nuevo_quango"
-            body.first_letter.should == "M"
+
+        it 'is rebuilt with the given params' do
+            post :create, :public_body => { :name => '',
+                                            :request_email => 'newquango@localhost',
+                                            :translations_attributes => {} }
+            expect(assigns(:public_body).request_email).to eq('newquango@localhost')
+        end
+
+    end
+
+    context 'on failure for multiple locales' do
+
+        before(:each) do
+            @params = { :public_body => { :name => '',
+                                          :request_email => 'newquango@localhost',
+                                          :translations_attributes => {
+                                            'es' => { :locale => 'es',
+                                                      :name => 'Los Quango' }
+                                          } } }
+        end
+        
+        it 'is rebuilt with the default locale translation' do
+            post :create, @params
+            expect(assigns(:public_body)).to_not be_persisted
+            expect(assigns(:public_body).request_email).to eq('newquango@localhost')
+        end
+
+        it 'is rebuilt with the alternative locale translation' do
+            post :create, @params
+
+            expect(assigns(:public_body)).to_not be_persisted
+            I18n.with_locale(:es) do
+                expect(assigns(:public_body).name).to eq('Los Quango')
+            end
         end
 
-        response.should redirect_to(:controller=>'admin_public_body', :action=>'show', :id=>body.id)
     end
 
     context 'when the body is being created as a result of a change request' do
@@ -157,10 +243,34 @@ end
 describe AdminPublicBodyController, "when editing a public body" do
     render_views
 
-    it "edits a public body" do
-        get :edit, :id => 2
+    before do
+        @body = FactoryGirl.create(:public_body)
+        I18n.with_locale('es') do
+            @body.name = 'Los Body'
+            @body.save!
+        end
+    end
+
+    it 'responds successfully' do
+        get :edit, :id => @body.id
+        expect(response).to be_success
+    end
+
+    it 'finds the requested body' do
+        get :edit, :id => @body.id
+        expect(assigns[:public_body]).to eq(@body)
     end
 
+    it 'builds new translations if the body does not already have a translation in the specified locale' do
+        get :edit, :id => @body.id
+        expect(assigns[:public_body].translations.map(&:locale)).to include(:fr)
+    end
+
+    it 'renders the edit template' do
+        get :edit, :id => @body.id
+        expect(response).to render_template('edit')
+    end
+  
     it "edits a public body in another locale" do
         get :edit, {:id => 3, :locale => :en}
 
@@ -170,12 +280,6 @@ 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
 
@@ -201,159 +305,200 @@ end
 describe AdminPublicBodyController, "when updating a public body" do
     render_views
 
-    it "saves edits to a public body" do
-        public_bodies(:humpadink_public_body).name.should == "Department for Humpadinking"
-        post :update, { :id => 3, :public_body => { :name => "Renamed",
-                                                    :short_name => "",
-                                                    :tag_string => "some tags",
-                                                    :request_email => 'edited@localhost',
-                                                    :last_edit_comment => 'From test code' } }
-        request.flash[:notice].should include('successful')
-        pb = PublicBody.find(public_bodies(:humpadink_public_body).id)
-        pb.name.should == "Renamed"
-    end
-
-    it 'adds a new translation' do
-        pb = public_bodies(:humpadink_public_body)
-        pb.translation_for(:es).destroy
-        pb.reload
-
-        post :update, {
-            :id => pb.id,
-            :public_body => {
-                :name => "Department for Humpadinking",
-                :short_name => "",
-                :tag_string => "some tags",
-                :request_email => 'edited@localhost',
-                :last_edit_comment => 'From test code',
-                :translations_attributes => {
-                    'es' => { :locale => "es",
-                              :name => "El Department for Humpadinking",
-                              :short_name => "",
-                              :request_email => 'edited@localhost' }
-                }
-            }
-        }
-
-        request.flash[:notice].should include('successful')
-
-        pb = PublicBody.find(public_bodies(:humpadink_public_body).id)
-
-        I18n.with_locale(:es) do
-           expect(pb.name).to eq('El Department for Humpadinking')
-        end
-    end
-
-    it 'adds new translations' do
-        pb = public_bodies(:humpadink_public_body)
-        pb.translation_for(:es).destroy
-        pb.reload
-
-        post :update, {
-            :id => pb.id,
-            :public_body => {
-                :name => "Department for Humpadinking",
-                :short_name => "",
-                :tag_string => "some tags",
-                :request_email => 'edited@localhost',
-                :last_edit_comment => 'From test code',
-                :translations_attributes => {
-                    'es' => { :locale => "es",
-                              :name => "El Department for Humpadinking",
-                              :short_name => "",
-                              :request_email => 'edited@localhost' },
-                    'fr' => { :locale => "fr",
-                              :name => "Le Department for Humpadinking",
-                              :short_name => "",
-                              :request_email => 'edited@localhost' }
-                }
-            }
-        }
-
-        request.flash[:notice].should include('successful')
-
-        pb = PublicBody.find(public_bodies(:humpadink_public_body).id)
-
-        I18n.with_locale(:es) do
-           expect(pb.name).to eq('El Department for Humpadinking')
-        end
-
-        I18n.with_locale(:fr) do
-           expect(pb.name).to eq('Le Department for Humpadinking')
-        end
-    end
-
-    it 'updates an existing translation and adds a third translation' do
-        pb = public_bodies(:humpadink_public_body)
-
-        put :update, {
-            :id => pb.id,
-            :public_body => {
-                :name => "Department for Humpadinking",
-                :short_name => "",
-                :tag_string => "some tags",
-                :request_email => 'edited@localhost',
-                :last_edit_comment => 'From test code',
-                :translations_attributes => {
-                    # Update existing translation
-                    'es' => { :locale => "es",
-                              :name => "Renamed Department for Humpadinking",
-                              :short_name => "",
-                              :request_email => 'edited@localhost' },
-                    # Add new translation
-                    'fr' => { :locale => "fr",
-                              :name => "Le Department for Humpadinking",
-                              :short_name => "",
-                              :request_email => 'edited@localhost' }
-                }
-            }
-        }
-
-        request.flash[:notice].should include('successful')
-
-        pb = PublicBody.find(public_bodies(:humpadink_public_body).id)
-
-        I18n.with_locale(:es) do
-           expect(pb.name).to eq('Renamed Department for Humpadinking')
-        end
-
-        I18n.with_locale(:fr) do
-           expect(pb.name).to eq('Le Department for Humpadinking')
-        end
-
-    end
-
-    it "saves edits to a public body in another locale" do
-        I18n.with_locale(:es) do
-            pb = PublicBody.find(id=3)
-            pb.name.should == "El Department for Humpadinking"
-            post :update, {
-                :id => 3,
-                :public_body => {
-                    :name => "Department for Humpadinking",
-                    :short_name => "",
-                    :tag_string => "some tags",
-                    :request_email => 'edited@localhost',
-                    :last_edit_comment => 'From test code',
-                    :translations_attributes => {
-                        'es' => { :locale => "es",
-                                  :name => "Renamed",
-                                  :short_name => "",
-                                  :request_email => 'edited@localhost' }
-                        }
-                    }
-                }
-            request.flash[:notice].should include('successful')
-        end
-
-        pb = PublicBody.find(public_bodies(:humpadink_public_body).id)
-
-        I18n.with_locale(:es) do
-           expect(pb.name).to eq('Renamed')
-        end
-
-        I18n.with_locale(:en) do
-           expect(pb.name).to eq('Department for Humpadinking')
+    before do
+        @body = FactoryGirl.create(:public_body)
+        I18n.with_locale('es') do
+            @body.name = 'Los Quango'
+            @body.save!
+        end
+
+        @params = { :id => @body.id,
+                    :public_body => { :name => 'Renamed',
+                                      :short_name => @body.short_name,
+                                      :request_email => @body.request_email,
+                                      :tag_string => @body.tag_string,
+                                      :last_edit_comment => 'From test code',
+                                      :translations_attributes => {
+                                        'es' => { :id => @body.translation_for(:es).id,
+                                                  :locale => 'es',
+                                                  :title => @body.name(:es) }
+                                      } } }
+    end
+
+    it 'finds the heading to update' do
+        post :update, @params
+        expect(assigns(:heading)).to eq(@heading)
+    end
+
+    context 'on success' do
+
+        it 'saves edits to a public body heading' do
+            post :update, @params
+            body = PublicBody.find(@body.id)
+            expect(body.name).to eq('Renamed')
+        end
+
+        it 'notifies the admin that the body was updated' do
+            post :update, @params
+            expect(flash[:notice]).to eq('PublicBody was successfully updated.')
+        end
+
+        it 'redirects to the admin body page' do
+            post :update, @params
+            expect(response).to redirect_to(admin_body_path(@body))
+        end
+
+    end
+
+    context 'on success for multiple locales' do
+
+        it 'saves edits to a public body heading in another locale' do
+            @body.name(:es).should == 'Los Quango'
+            post :update, :id => @body.id,
+                          :public_body => {
+                              :name => @body.name(:en),
+                              :translations_attributes => {
+                                'es' => { :id => @body.translation_for(:es).id,
+                                          :locale => 'es',
+                                          :name => 'Renamed' }
+                              }
+                          }
+
+            body = PublicBody.find(@body.id)
+            expect(body.name(:es)).to eq('Renamed')
+            expect(body.name(:en)).to eq(@body.name(:en))
+        end
+
+        it 'adds a new translation' do
+             @body.translation_for(:es).destroy
+             @body.reload
+
+             put :update, {
+                 :id => @body.id,
+                 :public_body => {
+                     :name => @body.name(:en),
+                     :translations_attributes => {
+                         'es' => { :locale => "es",
+                                   :name => "Example Public Body ES" }
+                     }
+                 }
+             }
+
+             request.flash[:notice].should include('successful')
+
+             body = PublicBody.find(@body.id)
+
+             I18n.with_locale(:es) do
+                expect(body.name).to eq('Example Public Body ES')
+             end
+         end
+
+         it 'adds new translations' do
+             @body.translation_for(:es).destroy
+             @body.reload
+
+             post :update, {
+                 :id => @body.id,
+                 :public_body => {
+                     :name => @body.name(:en),
+                     :translations_attributes => {
+                         'es' => { :locale => "es",
+                                   :name => "Example Public Body ES" },
+                         'fr' => { :locale => "fr",
+                                   :name => "Example Public Body FR" }
+                     }
+                 }
+             }
+
+             request.flash[:notice].should include('successful')
+
+             body = PublicBody.find(@body.id)
+
+             I18n.with_locale(:es) do
+                expect(body.name).to eq('Example Public Body ES')
+             end
+             I18n.with_locale(:fr) do
+                expect(body.name).to eq('Example Public Body FR')
+             end
+         end
+
+         it 'updates an existing translation and adds a third translation' do
+             post :update, {
+                 :id => @body.id,
+                 :public_body => {
+                     :name => @body.name(:en),
+                     :translations_attributes => {
+                         # Update existing translation
+                         'es' => { :id => @body.translation_for(:es).id,
+                                   :locale => "es",
+                                   :name => "Renamed Example Public Body ES" },
+                         # Add new translation
+                         'fr' => { :locale => "fr",
+                                   :name => "Example Public Body FR" }
+                     }
+                 }
+             }
+
+             request.flash[:notice].should include('successful')
+
+             body = PublicBody.find(@body.id)
+
+             I18n.with_locale(:es) do
+                expect(body.name).to eq('Renamed Example Public Body ES')
+             end
+             I18n.with_locale(:fr) do
+                expect(body.name).to eq('Example Public Body FR')
+             end
+         end
+
+    end
+
+    context 'on failure' do
+
+        it 'renders the form if creating the record was unsuccessful' do
+            post :update, :id => @body.id,
+                          :public_body => {
+                            :name => '',
+                            :translations_attributes => {}
+                          }
+            expect(response).to render_template('edit')
+        end
+
+        it 'is rebuilt with the given params' do
+            post :update, :id => @body.id,
+                          :public_body => {
+                            :name => '',
+                            :request_email => 'updated@localhost',
+                            :translations_attributes => {}
+                          }
+            expect(assigns(:public_body).request_email).to eq('updated@localhost')
+        end
+
+    end
+
+    context 'on failure for multiple locales' do
+
+        before(:each) do
+            @params = { :id => @body.id,
+                        :public_body => { :name => '',
+                                          :translations_attributes => {
+                                            'es' => { :id => @body.translation_for(:es).id,
+                                                      :locale => 'es',
+                                                      :name => 'Mi Nuevo Body' }
+                                          } } }
+        end
+
+        it 'is rebuilt with the default locale translation' do
+            post :update, @params
+            expect(assigns(:public_body).name(:en)).to eq('')
+        end
+
+        it 'is rebuilt with the alternative locale translation' do
+            post :update, @params
+
+            I18n.with_locale(:es) do
+                expect(assigns(:public_body).name).to eq('Mi Nuevo Body')
+            end
         end
 
     end
diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb
index d91021315..7b55efda1 100644
--- a/spec/models/public_body_spec.rb
+++ b/spec/models/public_body_spec.rb
@@ -30,106 +30,81 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 
 describe PublicBody do
 
-    describe :translations_attributes= do
-
-        context 'translation_attrs is a Hash' do
-
-            it 'takes the correct code path for a Hash' do
-                attrs = {}
-                attrs.should_receive(:each_value)
-                PublicBody.new().translations_attributes = attrs
-            end
-
-            it 'updates an existing translation' do
-                body = public_bodies(:geraldine_public_body)
-                translation = body.translation_for(:es)
-                params = { 'es' => { :locale => 'es',
-                                     :name => 'Renamed' } }
-
-                body.translations_attributes = params
-                I18n.with_locale(:es) { expect(body.name).to eq('Renamed') }
-            end
-
-            it 'updates an existing translation and creates a new translation' do
-                body = public_bodies(:geraldine_public_body)
-                translation = body.translation_for(:es)
-
-                expect(body.translations.size).to eq(2)
-
-                body.translations_attributes = {
-                    'es' => { :locale => 'es',
-                              :name => 'Renamed' },
-                    'fr' => { :locale => 'fr',
-                              :name => 'Le Geraldine Quango' }
-                }
-
-                expect(body.translations.size).to eq(3)
-                I18n.with_locale(:es) { expect(body.name).to eq('Renamed') }
-                I18n.with_locale(:fr) { expect(body.name).to eq('Le Geraldine Quango') }
-            end
-
-            it 'skips empty translations' do
-                body = public_bodies(:geraldine_public_body)
-                translation = body.translation_for(:es)
-
-                expect(body.translations.size).to eq(2)
-
-                body.translations_attributes = {
-                    'es' => { :locale => 'es',
-                              :name => 'Renamed' },
-                    'fr' => { :locale => 'fr' }
-                }
-
-                expect(body.translations.size).to eq(2)
-            end
-
-        end
-
-        context 'translation_attrs is an Array' do
-
-            it 'takes the correct code path for an Array' do
-                attrs = []
-                attrs.should_receive(:each)
-                PublicBody.new().translations_attributes = attrs
-            end
-
-            it 'creates a new translation' do
-                body = public_bodies(:geraldine_public_body)
-                body.translation_for(:es).destroy
-                body.reload
-
-                expect(body.translations.size).to eq(1)
-
-                body.translations_attributes = [ {
-                        :locale => 'es',
-                        :name => 'Renamed'
-                    }
-                ]
-
-                expect(body.translations.size).to eq(2)
-                I18n.with_locale(:es) { expect(body.name).to eq('Renamed') }
-            end
-
-            it 'skips empty translations' do
-                body = public_bodies(:geraldine_public_body)
-                body.translation_for(:es).destroy
-                body.reload
-
-                expect(body.translations.size).to eq(1)
-
-                body.translations_attributes = [
-                    { :locale => 'empty' }
-                ]
-
-                expect(body.translations.size).to eq(1)
-            end
-
-        end
-
-    end
-
+  describe :translations_attributes= do
+
+      context 'translation_attrs is a Hash' do
+
+          it 'does not persist translations' do
+              body = FactoryGirl.create(:public_body)
+              body.translations_attributes = { :es => { :locale => 'es',
+                                                        :name => 'El Body' } }
+
+              expect(PublicBody.find(body.id).translations.size).to eq(1)
+          end
+
+          it 'creates a new translation' do
+              body = FactoryGirl.create(:public_body)
+              body.translations_attributes = { :es => { :locale => 'es',
+                                                        :name => 'El Body' } }
+              body.save
+              body.reload
+              expect(body.name(:es)).to eq('El Body')
+          end
+
+          it 'updates an existing translation' do
+              body = FactoryGirl.create(:public_body)
+              body.translations_attributes = { 'es' => { :locale => 'es',
+                                                         :name => 'El Body' } }
+              body.save
+
+              body.translations_attributes = { 'es' => { :id => body.translation_for(:es).id,
+                                                         :locale => 'es',
+                                                         :name => 'Renamed' } }
+              body.save
+              expect(body.name(:es)).to eq('Renamed')
+          end
+
+          it 'updates an existing translation and creates a new translation' do
+              body = FactoryGirl.create(:public_body)
+              body.translations.create(:locale => 'es',
+                                       :name => 'El Body')
+
+              expect(body.translations.size).to eq(2)
+
+              body.translations_attributes = {
+                  'es' => { :id => body.translation_for(:es).id,
+                            :locale => 'es',
+                            :name => 'Renamed' },
+                  'fr' => { :locale => 'fr',
+                            :name => 'Le Body' }
+              }
+
+              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 Body') }
+          end
+
+          it 'skips empty translations' do
+              body = FactoryGirl.create(:public_body)
+              body.translations.create(:locale => 'es',
+                                       :name => 'El Body')
+
+              expect(body.translations.size).to eq(2)
+
+              body.translations_attributes = {
+                  'es' => { :id => body.translation_for(:es).id,
+                            :locale => 'es',
+                            :name => 'Renamed' },
+                  'fr' => { :locale => 'fr' }
+              }
+
+              expect(body.translations.size).to eq(2)
+          end
+      end
+  end
 end
 
+
 describe PublicBody, " using tags" do
     before do
         @public_body = PublicBody.new(:name => 'Aardvark Monitoring Service',
@@ -1264,3 +1239,17 @@ describe PublicBody do
 
 end
 
+describe PublicBody::Translation do
+
+  it 'requires a locale' do
+    translation = PublicBody::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 = PublicBody::Translation.new(:locale => I18n.default_locale)
+    expect(translation).to be_valid
+  end
+
+end