diff --git a/app/controllers/offers_controller.rb b/app/controllers/offers_controller.rb index 232ba52e1..7dfcad407 100644 --- a/app/controllers/offers_controller.rb +++ b/app/controllers/offers_controller.rb @@ -6,7 +6,7 @@ def model def show super - member = @offer.user.members.find_by(organization: current_organization) + member = @offer.user.members.find_by(organization: @offer.organization) @destination_account = member.account if member end end diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index e65506566..68bf80524 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -5,16 +5,12 @@ def create @source = find_source @account = Account.find(transfer_params[:destination]) - transfer = Transfer.new( - transfer_params.merge(source: @source, destination: @account) - ) - - persister = ::Persister::TransferPersister.new(transfer) + create_persisters - if persister.save + if persisters_saved? redirect_to redirect_target else - redirect_back fallback_location: redirect_target, alert: transfer.errors.full_messages.to_sentence + redirect_back fallback_location: redirect_target, alert: @transfer.errors.full_messages.to_sentence end end @@ -23,7 +19,8 @@ def new current_organization, current_user, params[:offer], - params[:destination_account_id] + params[:destination_account_id], + params[:organization_id] || current_organization.id ) render( @@ -57,12 +54,38 @@ def find_source end end + def create_persisters + source_organization = @source.organization.account + source_type = @source.accountable_type + destination_organization = @account.organization.account + @persisters = Array.new + if source_organization == destination_organization + transfer_persister_between(@source, @account) + else + transfer_persister_between(@source, source_organization) if source_type == "Member" + transfer_persister_between(source_organization, destination_organization) + transfer_persister_between(destination_organization, @account) + end + end + + def transfer_persister_between(source, destination) + @transfer = Transfer.new( + transfer_params.merge(source: source, destination: destination) + ) + + @persisters << ::Persister::TransferPersister.new(@transfer) + end + + def persisters_saved? + @persisters.each { |persister| return false if !persister.save } + end + def redirect_target case accountable = @account.accountable when Organization accountable when Member - accountable.user + accountable.organization == current_organization ? accountable.user : accountable.organization else raise ArgumentError end diff --git a/app/models/organization.rb b/app/models/organization.rb index 90847ee3b..4c3f9d5a0 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -75,6 +75,10 @@ def next_reg_number_seq reg_number_seq end + def global_balance + Account.where(organization_id: id).sum(:balance) + end + def ensure_url return if web.blank? || URI.parse(web).is_a?(URI::HTTP) rescue diff --git a/app/models/transfer_factory.rb b/app/models/transfer_factory.rb index a093299cd..fd4235a89 100644 --- a/app/models/transfer_factory.rb +++ b/app/models/transfer_factory.rb @@ -1,16 +1,22 @@ class TransferFactory - def initialize(current_organization, current_user, offer_id, destination_account_id) + def initialize(current_organization, current_user, offer_id, destination_account_id, + destination_organization_id) @current_organization = current_organization @current_user = current_user @offer_id = offer_id @destination_account_id = destination_account_id + @destination_organization_id = destination_organization_id.to_i + end + + def destination_organization + Organization.find(@destination_organization_id) end # Returns the offer that is the subject of the transfer # # @return [Maybe<Offer>] def offer - current_organization.offers.find_by_id(offer_id) + destination_organization.offers.find_by_id(offer_id) end # Returns a new instance of Transfer with the data provided @@ -73,7 +79,7 @@ def admin? # # @return [Account] def destination_account - @destination_account ||= current_organization + @destination_account ||= destination_organization .all_accounts .find(destination_account_id) end diff --git a/app/views/offers/show.html.erb b/app/views/offers/show.html.erb index 787ff8586..0293fe05d 100644 --- a/app/views/offers/show.html.erb +++ b/app/views/offers/show.html.erb @@ -10,6 +10,16 @@ <%= t ".give_time_for" %> <% end %> <% end %> +<% else %> + <div class="actions text-end"> + <% if current_user and @offer.user != current_user %> + <%= link_to new_transfer_path(id: @offer.user.id, offer: @offer.id, destination_account_id: @destination_account.id, + organization_id: @offer.organization.id), + class: "btn btn-success" do %> + <%= glyph :time %> + <%= t ".give_time_for" %> + <% end %> + <% end %> </div> <% end %> <%= render "shared/post", post: @offer %> diff --git a/app/views/organizations/show.html.erb b/app/views/organizations/show.html.erb index 5bfda7a5b..d0b3ce9da 100644 --- a/app/views/organizations/show.html.erb +++ b/app/views/organizations/show.html.erb @@ -78,6 +78,11 @@ <%= t 'global.balance' %> </strong> <%= seconds_to_hm(@organization.account.try(:balance)) %> + <br/> + <strong> + <%= t 'global.global_balance' %> + </strong> + <%= seconds_to_hm(@organization.try(:global_balance)) %> </p> </div> </div> diff --git a/spec/controllers/transfers_controller_spec.rb b/spec/controllers/transfers_controller_spec.rb index a034f778c..74ec8a535 100644 --- a/spec/controllers/transfers_controller_spec.rb +++ b/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,111 @@ } } 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 + subject(:create_between_src_org) do + post 'create', params: { transfer: { + source: test_organization.account.id, + 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 + 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 - expect do - post_create - member_taker.reload - end.to change { member_taker.account.balance.to_i }.by 5 + context 'the transfer is between different organisations' do + context 'the source is a member' 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 destination organization' do + expect(create_between_orgs).to redirect_to(second_member_taker.organization) + end + end + + context 'the source is a organization' do + it 'creates two news Transfers' do + expect { create_between_src_org }.to change(Transfer, :count).by 2 + end + + it 'creates four Movements' do + expect { create_between_src_org }.to change { Movement.count }.by 4 + end + + it 'updates the balance of both accounts' do + expect do + create_between_src_org + test_organization.reload + end.to change { test_organization.account.balance.to_i }.by -5 + + expect do + create_between_src_org + 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_src_org + + expect(test_organization.global_balance).to equal -5 + expect(second_organization.global_balance).to equal 5 + end + + it 'redirects to destination organization' do + expect(create_between_src_org).to redirect_to(second_member_taker.organization) + end + end end end @@ -173,30 +282,71 @@ } } 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 destination organization' do + expect(create_between_orgs).to redirect_to(second_member_taker.organization) + end end end end @@ -204,17 +354,31 @@ 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 diff --git a/spec/models/transfer_factory_spec.rb b/spec/models/transfer_factory_spec.rb index 50310d35a..2e1529e4c 100644 --- a/spec/models/transfer_factory_spec.rb +++ b/spec/models/transfer_factory_spec.rb @@ -4,7 +4,8 @@ organization, current_user, offer_id, - destination_account_id + destination_account_id, + destination_organization_id ) end @@ -12,6 +13,7 @@ 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) diff --git a/spec/views/offers/show.html.erb_spec.rb b/spec/views/offers/show.html.erb_spec.rb index fce4ba595..8412999e5 100644 --- a/spec/views/offers/show.html.erb_spec.rb +++ b/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(