tests: covered the new use case and other tests have been adapted

the transfer between users from different organisations has been carefully tested and tests like those of transfer_factory which required a parameter. Issue #638.

Author: franpb14

spec/controllers/transfers_controller_spec.rb

@@ -3,6 +3,8 @@
   let (:member_admin) { Fabricate(:member, organization: test_organization, manager: true) }
   let (:member_giver) { Fabricate(:member, organization: test_organization) }
   let (:member_taker) { Fabricate(:member, organization: test_organization) }
+  let (:second_organization) { Fabricate(:organization) }
+  let (:second_member_taker) { Fabricate(:member, organization: second_organization) }
 
   describe '#new' do
     let(:user) { member_giver.user }
@@ -51,6 +53,28 @@
         end
       end
 
+      context 'when the offer is specified and belongs to another organization' do
+        let(:other_offer) { Fabricate(:offer, organization: second_organization) }
+
+        it 'finds the transfer offer' do
+          dest_acc_id = second_member_taker.user.accounts.first.id
+          get :new, params: params.merge(offer: other_offer.id,
+                                         organization_id: other_offer.organization.id,
+                                         destination_account_id: dest_acc_id)
+
+          expect(response.body).to include("<h3>#{other_offer}</h3>")
+        end
+
+        it 'builds a transfer with the offer as post' do
+          dest_acc_id = second_member_taker.user.accounts.first.id
+          get :new, params: params.merge(offer: other_offer.id,
+                                         organization_id: other_offer.organization.id,
+                                         destination_account_id: dest_acc_id)
+
+          expect(response.body).to include("<input class=\"form-control hidden form-control\" type=\"hidden\" value=\"#{other_offer.id}\" name=\"transfer[post_id]\" id=\"transfer_post_id\" />")
+        end
+      end
+
       context 'when the offer is not specified' do
         it 'does not find any offer' do
           get :new, params: params
@@ -142,26 +166,68 @@
           } }
         end
 
-        let(:user) { member_admin.user }
-
-        it 'creates a new Transfer' do
-          expect { post_create }.to change(Transfer, :count).by 1
+        subject(:create_between_orgs) do
+          post 'create', params: { transfer: {
+            source: member_giver.account.id,
+            destination: second_member_taker.account.id,
+            amount: 5
+          } }
         end
 
-        it 'creates two Movements' do
-          expect { post_create }.to change { Movement.count }.by 2
+        let(:user) { member_admin.user }
+        context 'the transfer is within the same organisation' do
+          it 'creates a new Transfer' do
+            expect { post_create }.to change(Transfer, :count).by 1
+          end
+
+          it 'creates two Movements' do
+            expect { post_create }.to change { Movement.count }.by 2
+          end
+
+          it 'updates the balance of both accounts' do
+            expect do
+              post_create
+              member_giver.reload
+            end.to change { member_giver.account.balance.to_i }.by -5
+
+            expect do
+              post_create
+              member_taker.reload
+            end.to change { member_taker.account.balance.to_i }.by 5
+          end
         end
 
-        it 'updates the balance of both accounts' do
-          expect do
-            post_create
-            member_giver.reload
-          end.to change { member_giver.account.balance.to_i }.by -5
-
-          expect do
-            post_create
-            member_taker.reload
-          end.to change { member_taker.account.balance.to_i }.by 5
+        context 'the transfer is between members of different organisations' do
+          it 'creates three news Transfers' do
+            expect { create_between_orgs }.to change(Transfer, :count).by 3
+          end
+
+          it 'creates six Movements' do
+            expect { create_between_orgs }.to change { Movement.count }.by 6
+          end
+
+          it 'updates the balance of both accounts' do
+            expect do
+              create_between_orgs
+              member_giver.reload
+            end.to change { member_giver.account.balance.to_i }.by -5
+
+            expect do
+              create_between_orgs
+              second_member_taker.reload
+            end.to change { second_member_taker.account.balance.to_i }.by 5
+          end
+
+          it 'updates the global balance of both organizations' do
+            create_between_orgs
+
+            expect(test_organization.global_balance).to equal -5
+            expect(second_organization.global_balance).to equal 5
+          end
+
+          it 'redirects to source user' do
+            expect(create_between_orgs).to redirect_to(member_giver.user)
+          end
         end
       end
 
@@ -173,48 +239,103 @@
           } }
         end
 
-        let(:user) { member_giver.user }
-
-        it 'creates a new Transfer' do
-          expect { post_create }.to change(Transfer, :count).by 1
-        end
-
-        it 'creates two Movements' do
-          expect { post_create }.to change { Movement.count }.by 2
+        subject(:create_between_orgs) do
+          post 'create', params: { transfer: {
+            destination: second_member_taker.account.id,
+            amount: 5
+          } }
         end
 
-        it 'updates the balance of both accounts' do
-          expect do
-            post_create
-            member_giver.reload
-          end.to change { member_giver.account.balance.to_i }.by -5
-
-          expect do
-            post_create
-            member_taker.reload
-          end.to change { member_taker.account.balance.to_i }.by 5
+        let(:user) { member_giver.user }
+        context 'the transfer is within the same organisation' do
+          it 'creates a new Transfer' do
+            expect { post_create }.to change(Transfer, :count).by 1
+          end
+
+          it 'creates two Movements' do
+            expect { post_create }.to change { Movement.count }.by 2
+          end
+
+          it 'updates the balance of both accounts' do
+            expect do
+              post_create
+              member_giver.reload
+            end.to change { member_giver.account.balance.to_i }.by -5
+
+            expect do
+              post_create
+              member_taker.reload
+            end.to change { member_taker.account.balance.to_i }.by 5
+          end
+
+          it 'redirects to destination' do
+            expect(post_create).to redirect_to(member_taker.user)
+          end
         end
 
-        it 'redirects to destination' do
-          expect(post_create).to redirect_to(member_taker.user)
+        context 'the transfer is to a member of another organizations' do
+          it 'creates three news Transfers' do
+            expect { create_between_orgs }.to change(Transfer, :count).by 3
+          end
+
+          it 'creates six Movements' do
+            expect { create_between_orgs }.to change { Movement.count }.by 6
+          end
+
+          it 'updates the balance of both accounts' do
+            expect do
+              create_between_orgs
+              member_giver.reload
+            end.to change { member_giver.account.balance.to_i }.by -5
+
+            expect do
+              create_between_orgs
+              second_member_taker.reload
+            end.to change { second_member_taker.account.balance.to_i }.by 5
+          end
+
+          it 'updates the global balance of both organizations' do
+            create_between_orgs
+
+            expect(test_organization.global_balance).to equal -5
+            expect(second_organization.global_balance).to equal 5
+          end
+
+          it 'redirects to source' do
+            expect(create_between_orgs).to redirect_to(member_giver.user)
+          end
         end
       end
     end
 
     context 'with invalid params' do
       let(:user) { member_giver.user }
       let(:referer) { "/transfers/new?destination_account_id=#{member_taker.account.id}" }
+      let(:second_referer) { "/transfers/new?destination_account_id=#{member_taker.account.id}&organization_id=#{second_organization.id}" }
 
       before do
         request.env["HTTP_REFERER"] = referer
       end
 
-      it 'does not create any Transfer and redirects to back if the amount is 0' do
-        expect {
-          post(:create, params: { transfer: { amount: 0, destination: member_taker.account.id } })
-        }.not_to change(Transfer, :count)
+      context 'the transfer is to a member of the same organization' do
+        it 'does not create any Transfer and redirects to back if the amount is 0' do
+          expect {
+            post(:create, params: { transfer: { amount: 0, destination: member_taker.account.id } })
+          }.not_to change(Transfer, :count)
+
+          expect(response).to redirect_to(referer)
+        end
+      end
+
+      context 'the transfer is to a member of another organization' do
+        it 'does not create any Transfer and redirects to back if the amount is 0' do
+          request.env["HTTP_REFERER"] = second_referer
+          expect {
+            post(:create, params: { transfer: { amount: 0, destination: second_member_taker.account.id } })
+          }.not_to change(Transfer, :count)
 
-        expect(response).to redirect_to(referer)
+          expect(response).to redirect_to(second_referer)
+        end
       end
     end
   end

spec/models/transfer_factory_spec.rb

@@ -4,14 +4,16 @@
       organization,
       current_user,
       offer_id,
-      destination_account_id
+      destination_account_id,
+      destination_organization_id
     )
   end
 
   let(:organization) { Fabricate(:organization) }
   let(:current_user) { Fabricate(:user) }
   let(:organization_offer) { Fabricate(:offer, organization: organization) }
   let(:destination_account_id) { nil }
+  let(:destination_organization_id) { organization.id }
 
   describe '#offer' do
     subject { transfer_factory.offer }
@@ -32,6 +34,7 @@
 
     let(:offer_id) { organization_offer.id }
     let(:destination_account_id) { destination_account.id }
+    let(:destination_organization_id) { organization.id }
 
     context 'when the destination account belongs to an organization' do
       let(:organization) { Fabricate(:organization) }
@@ -77,6 +80,7 @@
     subject { transfer_factory.transfer_sources }
 
     let(:offer_id) { organization_offer.id }
+    let(:destination_organization_id) { organization.id }
 
     let!(:active_member) do
       Fabricate(:member, organization: organization, active: true)

spec/views/offers/show.html.erb_spec.rb

@@ -65,16 +65,18 @@
         allow(view).to receive(:current_organization) { another_organization }
       end
 
-      it 'doesn\'t render a link to the transfer page' do
+      it 'render a link to the transfer page with the id of the destination organisation' do
         assign :offer, offer
+        assign :destination_account, destination_account
         render template: 'offers/show'
 
-        expect(rendered).to_not have_link(
+        expect(rendered).to have_link(
           t('offers.show.give_time_for'),
           href: new_transfer_path(
             id: offer.user.id,
             offer: offer.id,
-            destination_account_id: destination_account.id
+            destination_account_id: destination_account.id,
+            organization_id: organization.id
           )
         )
       end
@@ -89,6 +91,7 @@
 
       it 'displays the offer\'s organization' do
         assign :offer, offer
+        assign :destination_account, destination_account
         render template: 'offers/show'
 
         expect(rendered).to include(