diff --git a/app/controllers/organization_transfers_controller.rb b/app/controllers/organization_transfers_controller.rb new file mode 100644 index 00000000..f2a18e84 --- /dev/null +++ b/app/controllers/organization_transfers_controller.rb @@ -0,0 +1,58 @@ +class OrganizationTransfersController < ApplicationController + before_action :authenticate_user! + before_action :check_manager_role + before_action :set_organizations, only: [:new] + before_action :validate_alliance, only: [:new, :create] + + def new + @transfer = Transfer.new + end + + def create + @transfer = Transfer.new(transfer_params) + @transfer.source = current_organization.account + @transfer.destination = destination_organization.account + @transfer.post = nil + persister = ::Persister::TransferPersister.new(@transfer) + + if persister.save + redirect_to organization_path(destination_organization), + notice: t('organizations.transfers.create.success') + else + set_organizations + flash.now[:error] = t('organizations.transfers.create.error', error: @transfer.errors.full_messages.to_sentence) + render :new + end + end + + private + + def transfer_params + params.require(:transfer).permit(:amount, :hours, :minutes, :reason) + end + + def check_manager_role + unless current_user.manages?(current_organization) + redirect_to root_path, alert: t('organization_alliances.not_authorized') + end + end + + def set_organizations + @source_organization = current_organization + @destination_organization = destination_organization + end + + def destination_organization + @destination_organization ||= Organization.find(params[:destination_organization_id]) + rescue ActiveRecord::RecordNotFound + redirect_to organizations_path, alert: t('application.tips.user_not_found') + end + + def validate_alliance + alliance = current_organization.alliance_with(destination_organization) + unless alliance && alliance.accepted? + redirect_to organizations_path, + alert: t('transfers.cross_bank.no_alliance') + end + end +end diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb index 12ae72f9..b10222da 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -72,6 +72,12 @@ def create_cross_bank_transfer(post) destination_organization = transfer_factory.destination_organization + unless current_organization.alliance_with(destination_organization)&.accepted? + redirect_back fallback_location: post, + alert: t('transfers.cross_bank.no_alliance') + return + end + @persisters = [] user_account = current_user.members.find_by(organization: current_organization).account @@ -93,8 +99,7 @@ def create_cross_bank_transfer(post) destination: destination_organization.account, amount: transfer_params[:amount], reason: post.description, - post: post, - is_cross_bank: true + post: post ) @persisters << ::Persister::TransferPersister.new(org_to_org_transfer) @@ -108,6 +113,9 @@ def create_cross_bank_transfer(post) post: post ) @persisters << ::Persister::TransferPersister.new(org_to_user_transfer) + else + redirect_back fallback_location: post, alert: t('transfers.cross_bank.error') + return end if persisters_saved? diff --git a/app/helpers/transfers_helper.rb b/app/helpers/transfers_helper.rb index 7e6cf20f..93d27f0c 100644 --- a/app/helpers/transfers_helper.rb +++ b/app/helpers/transfers_helper.rb @@ -22,4 +22,15 @@ def accounts_from_movements(transfer, with_links: false) end end end + + def is_bank_to_bank_transfer?(transfer) + return false unless transfer + + source_account = transfer.movements.find_by('amount < 0')&.account + destination_account = transfer.movements.find_by('amount > 0')&.account + + source_account&.accountable_type == 'Organization' && + destination_account&.accountable_type == 'Organization' && + transfer.post.nil? + end end diff --git a/app/models/transfer.rb b/app/models/transfer.rb index 8644b326..24c99402 100644 --- a/app/models/transfer.rb +++ b/app/models/transfer.rb @@ -11,7 +11,7 @@ # account, so the total sum of the system is zero # class Transfer < ApplicationRecord - attr_accessor :source, :destination, :amount, :hours, :minutes, :is_cross_bank, :meta + attr_accessor :source, :destination, :amount, :hours, :minutes belongs_to :post, optional: true has_many :movements, dependent: :destroy @@ -19,37 +19,12 @@ class Transfer < ApplicationRecord validates :amount, numericality: { greater_than: 0 } validate :different_source_and_destination - validate :validate_organizations_alliance, if: -> { is_cross_bank && meta.present? } after_create :make_movements def make_movements - if is_cross_bank && meta.present? - make_cross_bank_movements - else - movements.create(account: Account.find(source_id), amount: -amount.to_i, created_at: created_at) - movements.create(account: Account.find(destination_id), amount: amount.to_i, created_at: created_at) - end - end - - def make_cross_bank_movements - source_organization_id = meta[:source_organization_id] - destination_organization_id = meta[:destination_organization_id] - final_destination_user_id = meta[:final_destination_user_id] - - source_organization = Organization.find(source_organization_id) - destination_organization = Organization.find(destination_organization_id) - final_user = User.find(final_destination_user_id) - final_member = final_user.members.find_by(organization: destination_organization) - movements.create(account: Account.find(source_id), amount: -amount.to_i, created_at: created_at) - movements.create(account: source_organization.account, amount: amount.to_i, created_at: created_at) - - movements.create(account: source_organization.account, amount: -amount.to_i, created_at: created_at) - movements.create(account: destination_organization.account, amount: amount.to_i, created_at: created_at) - - movements.create(account: destination_organization.account, amount: -amount.to_i, created_at: created_at) - movements.create(account: final_member.account, amount: amount.to_i, created_at: created_at) + movements.create(account: Account.find(destination_id), amount: amount.to_i, created_at: created_at) end def source_id @@ -60,43 +35,10 @@ def destination_id destination.respond_to?(:id) ? destination.id : destination end + private + def different_source_and_destination return unless source == destination errors.add(:base, :same_account) end - - def cross_bank? - movements.count > 2 - end - - def related_account_for(movement) - return nil unless movement.transfer == self - - movements_in_order = movements.order(:id) - current_index = movements_in_order.index(movement) - return nil unless current_index - - if movement.amount > 0 && current_index > 0 - movements_in_order[current_index - 1].account - elsif movement.amount < 0 && current_index < movements_in_order.length - 1 - movements_in_order[current_index + 1].account - end - end - - private - - def validate_organizations_alliance - return unless meta[:source_organization_id] && meta[:destination_organization_id] - - source_org = Organization.find_by(id: meta[:source_organization_id]) - dest_org = Organization.find_by(id: meta[:destination_organization_id]) - - return unless source_org && dest_org - - alliance = source_org.alliance_with(dest_org) - - unless alliance && alliance.accepted? - errors.add(:base, :no_alliance_between_organizations) - end - end end diff --git a/app/models/transfer_factory.rb b/app/models/transfer_factory.rb index 95019e74..36464c0f 100644 --- a/app/models/transfer_factory.rb +++ b/app/models/transfer_factory.rb @@ -8,22 +8,14 @@ def initialize(current_organization, current_user, offer_id, destination_account end def offer - if offer_id.present? - Offer.find_by_id(offer_id) - end + @offer ||= Offer.find_by_id(offer_id) if offer_id.present? end def build_transfer - transfer = Transfer.new(source: source) + transfer = Transfer.new(source: source, destination: destination_account.id) - if cross_bank && offer && offer.organization != current_organization - transfer.destination = destination_account.id - transfer.post = offer - transfer.is_cross_bank = true - else - transfer.destination = destination_account.id - transfer.post = offer unless for_organization? - end + transfer.post = offer if (cross_bank && offer && offer.organization != current_organization) || + (offer && !for_organization?) transfer end @@ -63,7 +55,7 @@ def source end def for_organization? - destination_account.try(:accountable).class == Organization + destination_account&.accountable.is_a?(Organization) end def admin? diff --git a/app/views/organization_transfers/new.html.erb b/app/views/organization_transfers/new.html.erb new file mode 100644 index 00000000..946a2209 --- /dev/null +++ b/app/views/organization_transfers/new.html.erb @@ -0,0 +1,33 @@ +

+ <%= t 'organizations.transfers.new.title' %> +

+
+ <%= t 'organizations.transfers.new.description', + source_organization: @source_organization.name, + destination_organization: @destination_organization.name %> +
+<%= simple_form_for @transfer, url: organization_to_organization_transfers_path(destination_organization_id: @destination_organization.id) do |f| %> +
+ <%= f.input :hours, + as: :integer, + input_html: { + min: 0, + "data-rule-either-hours-minutes-informed" => "true" + } %> + <%= f.input :minutes, + as: :integer, + input_html: { + min: 0, + max: 59, + step: 15, + "data-rule-either-hours-minutes-informed" => "true", + "data-rule-range" => "[0,59]" + } %> + <%= f.input :amount, as: :hidden %> + <%= f.input :reason %> +
+
+ <%= f.button :submit, t('organizations.transfers.new.submit'), class: "btn btn-primary" %> + +
+<% end %> diff --git a/app/views/organizations/show.html.erb b/app/views/organizations/show.html.erb index 5bfda7a5..9daf1d4d 100644 --- a/app/views/organizations/show.html.erb +++ b/app/views/organizations/show.html.erb @@ -100,8 +100,23 @@ <% end %> <% end %> + <% if current_user&.manages?(current_organization) && + @organization != current_organization && + current_organization.alliance_with(@organization)&.accepted? %> + + <% end %> - <%= render "organizations/petition_button", organization: @organization %> +
+ <%= render "organizations/petition_button", organization: @organization %> +
+ <%= render "organizations/alliance_button", organization: @organization %> +
+
diff --git a/app/views/shared/_movements.html.erb b/app/views/shared/_movements.html.erb index c0be236d..12959fb3 100644 --- a/app/views/shared/_movements.html.erb +++ b/app/views/shared/_movements.html.erb @@ -21,14 +21,7 @@ <% - display_account = nil - - if mv.transfer&.cross_bank? - display_account = mv.transfer.related_account_for(mv) - display_account ||= mv.other_side.account - else - display_account = mv.other_side.account - end + display_account = mv.other_side.account %> <% if display_account.accountable.present? %> @@ -48,6 +41,8 @@ <% if mv.transfer&.post&.active? %> <%= link_to mv.transfer.post, offer_path(mv.transfer.post) %> + <% elsif is_bank_to_bank_transfer?(mv.transfer) %> + <%= t("organizations.transfers.bank_transfer") %> <% else %> <%= mv.transfer.post %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 738372fd..dad7aa7b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -412,7 +412,7 @@ en: contact_request: subject: "Contact request for your %{post}" greeting: "Hello %{name}," - message: "%{requester} from %{organization} time bank is interested in your %{post}." + message: "%{requester} from %{organization} organization is interested in your %{post}." requester_info: "Here is their contact information" closing: "If you are interested, please contact them directly using the provided information." organizations: @@ -427,6 +427,17 @@ en: show: contact_information: Contact information join_timebank: Don't hesitate to contact the time bank to join it or ask any questions. + transfers: + bank_to_bank_transfer: "Transfer time between organizations" + bank_transfer: "Organization to Organization transfer" + new: + title: "Transfer between Organizations" + submit: "Execute transfer" + description: "Transfer time from %{source_organization} to %{destination_organization}" + reason_hint: "Optional: Describe the reason for this bank-to-bank transfer" + create: + success: "Organization to Organization transfer completed successfully" + error: "Error processing the transfer: %{error}" pages: about: app-mobile: Mobile App @@ -569,8 +580,10 @@ en: new: error_amount: Time must be greater than 0 cross_bank: + success: "Cross-organization transfer completed successfully" + error: "Error creating cross-bank transfer" + no_alliance: "Cannot perform cross-bank transfers: no active alliance exists between organizations" info: "This is a time transfer to a member who belongs to %{organization}. The time will be transferred through both organizations." - success: "Cross-organization transfer completed successfully." users: avatar: change_your_image: Change your image @@ -637,4 +650,4 @@ en: last: Last next: Next previous: Previous - truncate: Truncate \ No newline at end of file + truncate: Truncate diff --git a/config/locales/es.yml b/config/locales/es.yml index 69a48217..400f33c9 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -427,6 +427,17 @@ es: show: contact_information: Información de contacto join_timebank: No dudes en contactar con el Banco de Tiempo para unirte o para resolver dudas. + transfers: + bank_to_bank_transfer: "Transferir tiempo entre organizaciones" + bank_transfer: "Transferencia entre organizaciones" + new: + title: "Transferencia entre organizaciones" + submit: "Crear transferencia" + description: "Transferir tiempo desde %{source_organization} a %{destination_organization}" + reason_hint: "Opcional: Describe el motivo de esta transferencia organizaciones" + create: + success: "Transferencia entre organizaciones realizada con éxito" + error: "Error al realizar la transferencia: %{error}" pages: about: app-mobile: App Móvil @@ -572,6 +583,7 @@ es: info: "Esta es una transferencia de tiempo a un miembro perteneciente a %{organization}. El tiempo se transferirá a través de ambas organizaciones." success: "Transferencia entre organizaciones completada con éxito." error: "Ha ocurrido un error al procesar la transferencia entre organizaciones." + no_alliance: "No se pueden realizar transferencias entre organizaciones: no existe una alianza activa entre ellas." users: avatar: diff --git a/config/routes.rb b/config/routes.rb index 54715059..e5c46d69 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -40,6 +40,8 @@ end get :select_organization, to: 'organizations#select_organization' + get 'organization_transfers/new', to: 'organization_transfers#new', as: :new_organization_to_organization_transfer + post 'organization_transfers', to: 'organization_transfers#create', as: :organization_to_organization_transfers resources :organization_alliances, only: [:index, :create, :update, :destroy] resources :users, concerns: :accountable, except: :destroy, :path => "members" do diff --git a/spec/controllers/organization_transfers_controller_spec.rb b/spec/controllers/organization_transfers_controller_spec.rb new file mode 100644 index 00000000..309cad53 --- /dev/null +++ b/spec/controllers/organization_transfers_controller_spec.rb @@ -0,0 +1,161 @@ +RSpec.describe OrganizationTransfersController do + let(:source_organization) { Fabricate(:organization) } + let(:target_organization) { Fabricate(:organization) } + let(:manager) { Fabricate(:member, organization: source_organization, manager: true) } + let(:user) { manager.user } + + let!(:alliance) do + OrganizationAlliance.create!( + source_organization: source_organization, + target_organization: target_organization, + status: "accepted" + ) + end + + before do + login(user) + session[:current_organization_id] = source_organization.id + controller.instance_variable_set(:@current_organization, source_organization) + end + + describe "GET #new" do + it "assigns a new transfer and sets organizations" do + get :new, params: { destination_organization_id: target_organization.id } + + expect(response).to be_successful + expect(assigns(:transfer)).to be_a_new(Transfer) + expect(assigns(:source_organization)).to eq(source_organization) + expect(assigns(:destination_organization)).to eq(target_organization) + end + + context "when user is not a manager" do + let(:regular_member) { Fabricate(:member, organization: source_organization) } + + before do + login(regular_member.user) + end + + it "redirects to root path" do + get :new, params: { destination_organization_id: target_organization.id } + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t('organization_alliances.not_authorized')) + end + end + + context "when destination organization not found" do + it "redirects to organizations path" do + get :new, params: { destination_organization_id: 999 } + + expect(response).to redirect_to(organizations_path) + expect(flash[:alert]).to eq(I18n.t('application.tips.user_not_found')) + end + end + + context "when no alliance exists between organizations" do + let(:other_organization) { Fabricate(:organization) } + + it "redirects to organizations path" do + get :new, params: { destination_organization_id: other_organization.id } + + expect(response).to redirect_to(organizations_path) + expect(flash[:alert]).to eq(I18n.t('activerecord.errors.models.transfer.attributes.base.no_alliance_between_organizations')) + end + end + + context "when alliance is pending" do + let(:pending_organization) { Fabricate(:organization) } + let!(:pending_alliance) do + OrganizationAlliance.create!( + source_organization: source_organization, + target_organization: pending_organization, + status: "pending" + ) + end + + it "redirects to organizations path" do + get :new, params: { destination_organization_id: pending_organization.id } + + expect(response).to redirect_to(organizations_path) + expect(flash[:alert]).to eq(I18n.t('activerecord.errors.models.transfer.attributes.base.no_alliance_between_organizations')) + end + end + end + + describe "POST #create" do + context "with valid parameters" do + it "creates a new transfer and redirects to organization path" do + persister_double = instance_double(::Persister::TransferPersister, save: true) + allow(::Persister::TransferPersister).to receive(:new).and_return(persister_double) + + expect { + post :create, params: { + destination_organization_id: target_organization.id, + transfer: { hours: 2, minutes: 30, reason: "Testing alliance", amount: 150 } + } + }.not_to raise_error + + expect(response).to redirect_to(organization_path(target_organization)) + expect(flash[:notice]).to eq(I18n.t('organizations.transfers.create.success')) + end + end + + context "with invalid parameters" do + it "renders the new template with errors" do + transfer_double = instance_double(Transfer) + persister_double = instance_double(::Persister::TransferPersister, save: false) + + allow(Transfer).to receive(:new).and_return(transfer_double) + allow(transfer_double).to receive(:source=) + allow(transfer_double).to receive(:destination=) + allow(transfer_double).to receive(:post=) + error_messages = ["Amount can't be zero"] + allow(transfer_double).to receive(:errors).and_return( + instance_double("ActiveModel::Errors", full_messages: error_messages) + ) + allow(::Persister::TransferPersister).to receive(:new).and_return(persister_double) + + expect(controller).to receive(:render).with(:new) + + post :create, params: { + destination_organization_id: target_organization.id, + transfer: { hours: 0, minutes: 0, reason: "", amount: 0 } + } + + expect(flash[:error]).to include("Amount can't be zero") + end + end + + context "when user is not a manager" do + let(:regular_member) { Fabricate(:member, organization: source_organization) } + + before do + login(regular_member.user) + end + + it "redirects to root path" do + post :create, params: { + destination_organization_id: target_organization.id, + transfer: { hours: 1, minutes: 0, reason: "Test", amount: 60 } + } + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t('organization_alliances.not_authorized')) + end + end + + context "when no alliance exists between organizations" do + let(:other_organization) { Fabricate(:organization) } + + it "redirects to organizations path" do + post :create, params: { + destination_organization_id: other_organization.id, + transfer: { hours: 1, minutes: 0, reason: "Test", amount: 60 } + } + + expect(response).to redirect_to(organizations_path) + expect(flash[:alert]).to eq(I18n.t('activerecord.errors.models.transfer.attributes.base.no_alliance_between_organizations')) + end + end + end +end diff --git a/spec/controllers/transfers_controller_cross_bank_spec.rb b/spec/controllers/transfers_controller_cross_bank_spec.rb index 24cc99c2..ea1fcd48 100644 --- a/spec/controllers/transfers_controller_cross_bank_spec.rb +++ b/spec/controllers/transfers_controller_cross_bank_spec.rb @@ -48,5 +48,17 @@ expect(response).to redirect_to(offer) expect(flash[:notice]).to eq(I18n.t('transfers.cross_bank.success')) end + + context 'when there is no accepted alliance between organizations' do + before do + alliance.update(status: "pending") + end + + it 'redirects back with an error message about missing alliance' do + request! + expect(response).to redirect_to(request.referer || offer) + expect(flash[:alert]).to eq(I18n.t('transfers.cross_bank.no_alliance')) + end end end +end diff --git a/spec/helpers/transfers_helper_spec.rb b/spec/helpers/transfers_helper_spec.rb index adb2048a..09830d59 100644 --- a/spec/helpers/transfers_helper_spec.rb +++ b/spec/helpers/transfers_helper_spec.rb @@ -15,4 +15,67 @@ expect(helper.accounts_from_movements(transfer, with_links: true)).to include(//) end end + + describe "#is_bank_to_bank_transfer?" do + let(:organization1) { Fabricate(:organization) } + let(:organization2) { Fabricate(:organization) } + let(:user) { Fabricate(:user) } + let(:member) { Fabricate(:member, organization: organization1, user: user) } + + context "when transfer is between two organizations" do + let(:transfer) do + transfer = Transfer.new( + source: organization1.account, + destination: organization2.account, + amount: 60 + ) + ::Persister::TransferPersister.new(transfer).save + transfer + end + + it "returns true" do + expect(helper.is_bank_to_bank_transfer?(transfer)).to be true + end + end + + context "when transfer is from a user to an organization" do + let(:transfer) do + transfer = Transfer.new( + source: member.account, + destination: organization1.account, + amount: 60 + ) + ::Persister::TransferPersister.new(transfer).save + transfer + end + + it "returns false" do + expect(helper.is_bank_to_bank_transfer?(transfer)).to be false + end + end + + context "when transfer has a post associated" do + let(:post) { Fabricate(:post, organization: organization1) } + let(:transfer) do + transfer = Transfer.new( + source: organization1.account, + destination: organization2.account, + amount: 60, + post: post + ) + ::Persister::TransferPersister.new(transfer).save + transfer + end + + it "returns false" do + expect(helper.is_bank_to_bank_transfer?(transfer)).to be false + end + end + + context "when transfer is nil" do + it "returns false" do + expect(helper.is_bank_to_bank_transfer?(nil)).to be false + end + end + end end diff --git a/spec/models/transfer_factory_cross_bank_spec.rb b/spec/models/transfer_factory_cross_bank_spec.rb index 720a69bc..ea35e071 100644 --- a/spec/models/transfer_factory_cross_bank_spec.rb +++ b/spec/models/transfer_factory_cross_bank_spec.rb @@ -30,7 +30,6 @@ before do allow(transfer_factory).to receive(:destination_account).and_return(dest_org.account) - allow_any_instance_of(Transfer).to receive(:is_cross_bank=) end describe '#build_transfer' do diff --git a/spec/views/offers/show.html.erb_spec.rb b/spec/views/offers/show.html.erb_spec.rb index f886b4d3..c0267ad9 100644 --- a/spec/views/offers/show.html.erb_spec.rb +++ b/spec/views/offers/show.html.erb_spec.rb @@ -91,8 +91,6 @@ assign :offer, offer render template: 'offers/show' - # Verificar que la vista muestra el nombre de la organización - # sin depender del formato exacto del mensaje expect(rendered).to include(offer.organization.name) end end @@ -133,8 +131,6 @@ assign :offer, offer render template: 'offers/show' - # Verificar que la vista muestra el nombre de la organización - # sin depender del formato exacto del mensaje expect(rendered).to include(offer.organization.name) end